[llvm] [DebugInfo][SimpleLoopUnswitch] Fix missing debug location updates (PR #97662)
Shan Huang via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 3 19:41:40 PDT 2024
https://github.com/Apochens created https://github.com/llvm/llvm-project/pull/97662
Fix #97559 .
For the change at line 1253, I propagate the debug location of the terminator (i.e., the insertion point) to the new phi. because `MergeBB` is generated by splitting `ExitBB` several lines above, it only has the terminator, which could provide a reasonable debug location.
For the change at line 2348, I switch the order of moving and cloning `TI`. Because `NewTI` cloned from `TI` is inserted into the original place where `TI` is, `NewTI` should preserve the origianl debug location. At the same time, doing this allows us to propagate the debug location to the new branch instruction replacing `NewTI` (the change at line 2446).
>From 74af7f49b1b98ae542f8e16e6fb5ef3022af11c9 Mon Sep 17 00:00:00 2001
From: Apochens <52285902006 at stu.ecnu.edu.cn>
Date: Thu, 4 Jul 2024 02:27:27 +0000
Subject: [PATCH] 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<PHINode>(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<SwitchInst>(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<BranchInst>(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<nontrivial>' -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)
More information about the llvm-commits
mailing list