Skip to content

Commit 33bdb87

Browse files
authored
[DebugInfo][SimpleLoopUnswitch] Fix missing debug location updates (#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).
1 parent d83d09f commit 33bdb87

File tree

2 files changed

+97
-11
lines changed

2 files changed

+97
-11
lines changed

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,9 +1245,12 @@ static BasicBlock *buildClonedLoopBlocks(
12451245
if (SE && isa<PHINode>(I))
12461246
SE->forgetValue(&I);
12471247

1248+
BasicBlock::iterator InsertPt = MergeBB->getFirstInsertionPt();
1249+
12481250
auto *MergePN =
12491251
PHINode::Create(I.getType(), /*NumReservedValues*/ 2, ".us-phi");
1250-
MergePN->insertBefore(MergeBB->getFirstInsertionPt());
1252+
MergePN->insertBefore(InsertPt);
1253+
MergePN->setDebugLoc(InsertPt->getDebugLoc());
12511254
I.replaceAllUsesWith(MergePN);
12521255
MergePN->addIncoming(&I, ExitBB);
12531256
MergePN->addIncoming(&ClonedI, ClonedExitBB);
@@ -1306,8 +1309,9 @@ static BasicBlock *buildClonedLoopBlocks(
13061309
else if (auto *SI = dyn_cast<SwitchInst>(ClonedTerminator))
13071310
ClonedConditionToErase = SI->getCondition();
13081311

1312+
Instruction *BI = BranchInst::Create(ClonedSuccBB, ClonedParentBB);
1313+
BI->setDebugLoc(ClonedTerminator->getDebugLoc());
13091314
ClonedTerminator->eraseFromParent();
1310-
BranchInst::Create(ClonedSuccBB, ClonedParentBB);
13111315

13121316
if (ClonedConditionToErase)
13131317
RecursivelyDeleteTriviallyDeadInstructions(ClonedConditionToErase, nullptr,
@@ -2334,22 +2338,27 @@ static void unswitchNontrivialInvariants(
23342338
// nuke the initial terminator placed in the split block.
23352339
SplitBB->getTerminator()->eraseFromParent();
23362340
if (FullUnswitch) {
2337-
// Splice the terminator from the original loop and rewrite its
2338-
// successors.
2339-
TI.moveBefore(*SplitBB, SplitBB->end());
2340-
23412341
// Keep a clone of the terminator for MSSA updates.
23422342
Instruction *NewTI = TI.clone();
23432343
NewTI->insertInto(ParentBB, ParentBB->end());
23442344

2345+
// Splice the terminator from the original loop and rewrite its
2346+
// successors.
2347+
TI.moveBefore(*SplitBB, SplitBB->end());
2348+
TI.dropLocation();
2349+
23452350
// First wire up the moved terminator to the preheaders.
23462351
if (BI) {
23472352
BasicBlock *ClonedPH = ClonedPHs.begin()->second;
23482353
BI->setSuccessor(ClonedSucc, ClonedPH);
23492354
BI->setSuccessor(1 - ClonedSucc, LoopPH);
23502355
Value *Cond = skipTrivialSelect(BI->getCondition());
2351-
if (InsertFreeze)
2356+
if (InsertFreeze) {
2357+
// We don't give any debug location to the new freeze, because the
2358+
// BI (`dyn_cast<BranchInst>(TI)`) is an in-loop instruction hoisted
2359+
// out of the loop.
23522360
Cond = new FreezeInst(Cond, Cond->getName() + ".fr", BI->getIterator());
2361+
}
23532362
BI->setCondition(Cond);
23542363
DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH});
23552364
} else {
@@ -2432,12 +2441,13 @@ static void unswitchNontrivialInvariants(
24322441
DTUpdates.push_back({DominatorTree::Delete, ParentBB, SuccBB});
24332442
}
24342443

2435-
// After MSSAU update, remove the cloned terminator instruction NewTI.
2436-
ParentBB->getTerminator()->eraseFromParent();
2437-
24382444
// Create a new unconditional branch to the continuing block (as opposed to
24392445
// the one cloned).
2440-
BranchInst::Create(RetainedSuccBB, ParentBB);
2446+
Instruction *NewBI = BranchInst::Create(RetainedSuccBB, ParentBB);
2447+
NewBI->setDebugLoc(NewTI->getDebugLoc());
2448+
2449+
// After MSSAU update, remove the cloned terminator instruction NewTI.
2450+
NewTI->eraseFromParent();
24412451
} else {
24422452
assert(BI && "Only branches have partial unswitching.");
24432453
assert(UnswitchedSuccBBs.size() == 1 &&
@@ -2710,6 +2720,7 @@ static BranchInst *turnSelectIntoBranch(SelectInst *SI, DominatorTree &DT,
27102720
PHINode::Create(SI->getType(), 2, "unswitched.select", SI->getIterator());
27112721
Phi->addIncoming(SI->getTrueValue(), ThenBB);
27122722
Phi->addIncoming(SI->getFalseValue(), HeadBB);
2723+
Phi->setDebugLoc(SI->getDebugLoc());
27132724
SI->replaceAllUsesWith(Phi);
27142725
SI->eraseFromParent();
27152726

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
; RUN: opt -passes='simple-loop-unswitch<nontrivial>' -S < %s | FileCheck %s
2+
3+
define i32 @basic(i32 %N, i1 %cond, i32 %select_input) !dbg !5 {
4+
; CHECK-LABEL: define i32 @basic(
5+
6+
; Check that SimpleLoopUnswitch's unswitchNontrivialInvariants() drops the
7+
; debug location of the hoisted terminator and doesn't give any debug location
8+
; to the new freeze, since it's inserted in a hoist block.
9+
; Also check that in unswitchNontrivialInvariants(), the new br instruction
10+
; inherits the debug location of the old terminator in the same block.
11+
12+
; CHECK: entry:
13+
; CHECK-NEXT: [[COND_FR:%.*]] = freeze i1 [[COND:%.*]]{{$}}
14+
; CHECK-NEXT: br i1 [[COND_FR]], label %[[ENTRY_SPLIT_US:.*]], label %[[ENTRY_SPLIT:.*]]{{$}}
15+
; CHECK: for.body.us:
16+
; CHECK-NEXT: br label %0, !dbg [[DBG13:![0-9]+]]
17+
18+
; Check that in turnSelectIntoBranch(), the new phi inherits the debug
19+
; location of the old select instruction replaced.
20+
21+
; CHECK: 1:
22+
; CHECK-NEXT: [[UNSWITCHED_SELECT_US:%.*]] = phi i32 [ [[SELECT_INPUT:%.*]], %0 ], !dbg [[DBG13]]
23+
24+
; Check that in BuildClonedLoopBlocks(), the new phi inherits the debug
25+
; location of the instruction at the insertion point and the new br
26+
; instruction inherits the debug location of the old terminator.
27+
28+
; CHECK: for.body:
29+
; CHECK-NEXT: br label %2, !dbg [[DBG13]]
30+
; CHECK: for.cond.cleanup:
31+
; CHECK: [[DOTUS_PHI:%.*]] = phi i32 [ [[RES_LCSSA:%.*]], %[[FOR_COND_CLEANUP_SPLIT:.*]] ], [ [[RES_LCSSA_US:%.*]], %[[FOR_COND_CLEANUP_SPLIT_US:.*]] ], !dbg [[DBG17:![0-9]+]]
32+
entry:
33+
br label %for.cond, !dbg !8
34+
35+
for.cond: ; preds = %for.body, %entry
36+
%res = phi i32 [ 0, %entry ], [ %add, %for.body ], !dbg !9
37+
%i = phi i32 [ 0, %entry ], [ %inc, %for.body ], !dbg !10
38+
%cmp = icmp slt i32 %i, %N, !dbg !11
39+
br i1 %cmp, label %for.body, label %for.cond.cleanup, !dbg !12
40+
41+
for.body: ; preds = %for.cond
42+
%cond1 = select i1 %cond, i32 %select_input, i32 42, !dbg !13
43+
%add = add nuw nsw i32 %cond1, %res, !dbg !14
44+
%inc = add nuw nsw i32 %i, 1, !dbg !15
45+
br label %for.cond, !dbg !16
46+
47+
for.cond.cleanup: ; preds = %for.cond
48+
ret i32 %res, !dbg !17
49+
}
50+
51+
!llvm.dbg.cu = !{!0}
52+
!llvm.debugify = !{!2, !3}
53+
!llvm.module.flags = !{!4}
54+
55+
; CHECK: [[DBG13]] = !DILocation(line: 6,
56+
; CHECK: [[DBG17]] = !DILocation(line: 10,
57+
58+
!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
59+
!1 = !DIFile(filename: "main.ll", directory: "/")
60+
!2 = !{i32 10}
61+
!3 = !{i32 0}
62+
!4 = !{i32 2, !"Debug Info Version", i32 3}
63+
!5 = distinct !DISubprogram(name: "basic", linkageName: "basic", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
64+
!6 = !DISubroutineType(types: !7)
65+
!7 = !{}
66+
!8 = !DILocation(line: 1, column: 1, scope: !5)
67+
!9 = !DILocation(line: 2, column: 1, scope: !5)
68+
!10 = !DILocation(line: 3, column: 1, scope: !5)
69+
!11 = !DILocation(line: 4, column: 1, scope: !5)
70+
!12 = !DILocation(line: 5, column: 1, scope: !5)
71+
!13 = !DILocation(line: 6, column: 1, scope: !5)
72+
!14 = !DILocation(line: 7, column: 1, scope: !5)
73+
!15 = !DILocation(line: 8, column: 1, scope: !5)
74+
!16 = !DILocation(line: 9, column: 1, scope: !5)
75+
!17 = !DILocation(line: 10, column: 1, scope: !5)

0 commit comments

Comments
 (0)