From 74af7f49b1b98ae542f8e16e6fb5ef3022af11c9 Mon Sep 17 00:00:00 2001 From: Apochens <52285902006@stu.ecnu.edu.cn> Date: Thu, 4 Jul 2024 02:27:27 +0000 Subject: [PATCH 1/2] fix missing debug location updates --- .../Transforms/Scalar/SimpleLoopUnswitch.cpp | 30 +++++--- ...preserving-dropping-debugloc-nontrivial.ll | 75 +++++++++++++++++++ 2 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 llvm/test/Transforms/SimpleLoopUnswitch/preserving-dropping-debugloc-nontrivial.ll diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp index fdb3211b4a438..caa1f7a156a3f 100644 --- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp +++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp @@ -1245,9 +1245,12 @@ static BasicBlock *buildClonedLoopBlocks( if (SE && isa(I)) SE->forgetValue(&I); + Instruction *InsertPt = &*MergeBB->getFirstInsertionPt(); + auto *MergePN = PHINode::Create(I.getType(), /*NumReservedValues*/ 2, ".us-phi"); - MergePN->insertBefore(MergeBB->getFirstInsertionPt()); + MergePN->insertBefore(InsertPt); + MergePN->setDebugLoc(InsertPt->getDebugLoc()); I.replaceAllUsesWith(MergePN); MergePN->addIncoming(&I, ExitBB); MergePN->addIncoming(&ClonedI, ClonedExitBB); @@ -1306,8 +1309,9 @@ static BasicBlock *buildClonedLoopBlocks( else if (auto *SI = dyn_cast(ClonedTerminator)) ClonedConditionToErase = SI->getCondition(); + Instruction *BI = BranchInst::Create(ClonedSuccBB, ClonedParentBB); + BI->setDebugLoc(ClonedTerminator->getDebugLoc()); ClonedTerminator->eraseFromParent(); - BranchInst::Create(ClonedSuccBB, ClonedParentBB); if (ClonedConditionToErase) RecursivelyDeleteTriviallyDeadInstructions(ClonedConditionToErase, nullptr, @@ -2334,14 +2338,15 @@ static void unswitchNontrivialInvariants( // nuke the initial terminator placed in the split block. SplitBB->getTerminator()->eraseFromParent(); if (FullUnswitch) { - // Splice the terminator from the original loop and rewrite its - // successors. - TI.moveBefore(*SplitBB, SplitBB->end()); - // Keep a clone of the terminator for MSSA updates. Instruction *NewTI = TI.clone(); NewTI->insertInto(ParentBB, ParentBB->end()); + // Splice the terminator from the original loop and rewrite its + // successors. + TI.moveBefore(*SplitBB, SplitBB->end()); + TI.dropLocation(); + // First wire up the moved terminator to the preheaders. if (BI) { BasicBlock *ClonedPH = ClonedPHs.begin()->second; @@ -2349,6 +2354,9 @@ static void unswitchNontrivialInvariants( BI->setSuccessor(1 - ClonedSucc, LoopPH); Value *Cond = skipTrivialSelect(BI->getCondition()); if (InsertFreeze) + // We don't give any debug location to the new freeze, because the + // BI (`dyn_cast(TI)`) is an in-loop instruction hoisted + // out of the loop. Cond = new FreezeInst(Cond, Cond->getName() + ".fr", BI->getIterator()); BI->setCondition(Cond); DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH}); @@ -2432,12 +2440,13 @@ static void unswitchNontrivialInvariants( DTUpdates.push_back({DominatorTree::Delete, ParentBB, SuccBB}); } - // After MSSAU update, remove the cloned terminator instruction NewTI. - ParentBB->getTerminator()->eraseFromParent(); - // Create a new unconditional branch to the continuing block (as opposed to // the one cloned). - BranchInst::Create(RetainedSuccBB, ParentBB); + Instruction *NewBI = BranchInst::Create(RetainedSuccBB, ParentBB); + NewBI->setDebugLoc(NewTI->getDebugLoc()); + + // After MSSAU update, remove the cloned terminator instruction NewTI. + NewTI->eraseFromParent(); } else { assert(BI && "Only branches have partial unswitching."); assert(UnswitchedSuccBBs.size() == 1 && @@ -2710,6 +2719,7 @@ static BranchInst *turnSelectIntoBranch(SelectInst *SI, DominatorTree &DT, PHINode::Create(SI->getType(), 2, "unswitched.select", SI->getIterator()); Phi->addIncoming(SI->getTrueValue(), ThenBB); Phi->addIncoming(SI->getFalseValue(), HeadBB); + Phi->setDebugLoc(SI->getDebugLoc()); SI->replaceAllUsesWith(Phi); SI->eraseFromParent(); diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/preserving-dropping-debugloc-nontrivial.ll b/llvm/test/Transforms/SimpleLoopUnswitch/preserving-dropping-debugloc-nontrivial.ll new file mode 100644 index 0000000000000..bdb97ff220608 --- /dev/null +++ b/llvm/test/Transforms/SimpleLoopUnswitch/preserving-dropping-debugloc-nontrivial.ll @@ -0,0 +1,75 @@ +; RUN: opt -passes='simple-loop-unswitch' -S < %s | FileCheck %s + +define i32 @basic(i32 %N, i1 %cond, i32 %select_input) !dbg !5 { +; CHECK-LABEL: define i32 @basic( + +; Check that SimpleLoopUnswitch's unswitchNontrivialInvariants() drops the +; debug location of the hoisted terminator and doesn't give any debug location +; to the new freeze, since it's inserted in a hoist block. +; Also check that in unswitchNontrivialInvariants(), the new br instruction +; inherits the debug location of the old terminator in the same block. + +; CHECK: entry: +; CHECK-NEXT: [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]{{$}} +; CHECK-NEXT: br i1 [[COND_FR]], label %[[ENTRY_SPLIT_US:.*]], label %[[ENTRY_SPLIT:.*]]{{$}} +; CHECK: for.body.us: +; CHECK-NEXT: br label %0, !dbg [[DBG13:![0-9]+]] + +; Check that in turnSelectIntoBranch(), the new phi inherits the debug +; location of the old select instruction replaced. + +; CHECK: 1: +; CHECK-NEXT: [[UNSWITCHED_SELECT_US:%.*]] = phi i32 [ [[SELECT_INPUT:%.*]], %0 ], !dbg [[DBG13]] + +; Check that in BuildClonedLoopBlocks(), the new phi inherits the debug +; location of the instruction at the insertion point and the new br +; instruction inherits the debug location of the old terminator. + +; CHECK: for.body: +; CHECK-NEXT: br label %2, !dbg [[DBG13]] +; CHECK: for.cond.cleanup: +; CHECK: [[DOTUS_PHI:%.*]] = phi i32 [ [[RES_LCSSA:%.*]], %[[FOR_COND_CLEANUP_SPLIT:.*]] ], [ [[RES_LCSSA_US:%.*]], %[[FOR_COND_CLEANUP_SPLIT_US:.*]] ], !dbg [[DBG17:![0-9]+]] +entry: + br label %for.cond, !dbg !8 + +for.cond: ; preds = %for.body, %entry + %res = phi i32 [ 0, %entry ], [ %add, %for.body ], !dbg !9 + %i = phi i32 [ 0, %entry ], [ %inc, %for.body ], !dbg !10 + %cmp = icmp slt i32 %i, %N, !dbg !11 + br i1 %cmp, label %for.body, label %for.cond.cleanup, !dbg !12 + +for.body: ; preds = %for.cond + %cond1 = select i1 %cond, i32 %select_input, i32 42, !dbg !13 + %add = add nuw nsw i32 %cond1, %res, !dbg !14 + %inc = add nuw nsw i32 %i, 1, !dbg !15 + br label %for.cond, !dbg !16 + +for.cond.cleanup: ; preds = %for.cond + ret i32 %res, !dbg !17 +} + +!llvm.dbg.cu = !{!0} +!llvm.debugify = !{!2, !3} +!llvm.module.flags = !{!4} + +; CHECK: [[DBG13]] = !DILocation(line: 6, +; CHECK: [[DBG17]] = !DILocation(line: 10, + +!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug) +!1 = !DIFile(filename: "main.ll", directory: "/") +!2 = !{i32 10} +!3 = !{i32 0} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = distinct !DISubprogram(name: "basic", linkageName: "basic", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0) +!6 = !DISubroutineType(types: !7) +!7 = !{} +!8 = !DILocation(line: 1, column: 1, scope: !5) +!9 = !DILocation(line: 2, column: 1, scope: !5) +!10 = !DILocation(line: 3, column: 1, scope: !5) +!11 = !DILocation(line: 4, column: 1, scope: !5) +!12 = !DILocation(line: 5, column: 1, scope: !5) +!13 = !DILocation(line: 6, column: 1, scope: !5) +!14 = !DILocation(line: 7, column: 1, scope: !5) +!15 = !DILocation(line: 8, column: 1, scope: !5) +!16 = !DILocation(line: 9, column: 1, scope: !5) +!17 = !DILocation(line: 10, column: 1, scope: !5) From b0f6c23893e2bba779ede399ac99b86d202638ba Mon Sep 17 00:00:00 2001 From: Apochens <52285902006@stu.ecnu.edu.cn> Date: Thu, 11 Jul 2024 01:50:29 +0000 Subject: [PATCH 2/2] nit --- llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp index caa1f7a156a3f..6a3b0e1b43e21 100644 --- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp +++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp @@ -1245,7 +1245,7 @@ static BasicBlock *buildClonedLoopBlocks( if (SE && isa(I)) SE->forgetValue(&I); - Instruction *InsertPt = &*MergeBB->getFirstInsertionPt(); + BasicBlock::iterator InsertPt = MergeBB->getFirstInsertionPt(); auto *MergePN = PHINode::Create(I.getType(), /*NumReservedValues*/ 2, ".us-phi"); @@ -2353,11 +2353,12 @@ static void unswitchNontrivialInvariants( BI->setSuccessor(ClonedSucc, ClonedPH); BI->setSuccessor(1 - ClonedSucc, LoopPH); Value *Cond = skipTrivialSelect(BI->getCondition()); - if (InsertFreeze) + if (InsertFreeze) { // We don't give any debug location to the new freeze, because the // BI (`dyn_cast(TI)`) is an in-loop instruction hoisted // out of the loop. Cond = new FreezeInst(Cond, Cond->getName() + ".fr", BI->getIterator()); + } BI->setCondition(Cond); DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH}); } else {