Skip to content

Commit 8cb07ce

Browse files
epilkDavid Salinas
epilk
authored and
David Salinas
committed
[StructurizeCFG] Stop setting DebugLocs in flow blocks (llvm#139088)
Flow blocks are generated code that don't really correspond to any location in the source, so principally they should have empty DebugLocs. Practically, setting these debug locs leads to redundant is_stmts being generated after llvm#108251, causing stepping test failures in the ROCm GDB test suite. Fixes SWDEV-502134
1 parent 160ae96 commit 8cb07ce

File tree

3 files changed

+39
-46
lines changed

3 files changed

+39
-46
lines changed

‎llvm/lib/Transforms/Scalar/StructurizeCFG.cpp‎

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ using BBPredicates = DenseMap<BasicBlock *, PredInfo>;
129129
using PredMap = DenseMap<BasicBlock *, BBPredicates>;
130130
using BB2BBMap = DenseMap<BasicBlock *, BasicBlock *>;
131131

132-
using BranchDebugLocMap = DenseMap<BasicBlock *, DebugLoc>;
133-
134132
// A traits type that is intended to be used in graph algorithms. The graph
135133
// traits starts at an entry node, and traverses the RegionNodes that are in
136134
// the Nodes set.
@@ -303,8 +301,6 @@ class StructurizeCFG {
303301
PredMap LoopPreds;
304302
BranchVector LoopConds;
305303

306-
BranchDebugLocMap TermDL;
307-
308304
RegionNode *PrevNode;
309305

310306
void orderNodes();
@@ -336,14 +332,14 @@ class StructurizeCFG {
336332

337333
void simplifyAffectedPhis();
338334

339-
void killTerminator(BasicBlock *BB);
335+
DebugLoc killTerminator(BasicBlock *BB);
340336

341337
void changeExit(RegionNode *Node, BasicBlock *NewExit,
342338
bool IncludeDominator);
343339

344340
BasicBlock *getNextFlow(BasicBlock *Dominator);
345341

346-
BasicBlock *needPrefix(bool NeedEmpty);
342+
std::pair<BasicBlock *, DebugLoc> needPrefix(bool NeedEmpty);
347343

348344
BasicBlock *needPostfix(BasicBlock *Flow, bool ExitUseAllowed);
349345

@@ -595,14 +591,14 @@ void StructurizeCFG::collectInfos() {
595591
// Find the last back edges
596592
analyzeLoops(RN);
597593
}
598-
594+
/*
599595
// Reset the collected term debug locations
600596
TermDL.clear();
601597
602598
for (BasicBlock &BB : *Func) {
603599
if (const DebugLoc &DL = BB.getTerminator()->getDebugLoc())
604600
TermDL[&BB] = DL;
605-
}
601+
} */
606602
}
607603

608604
/// Insert the missing branch conditions
@@ -924,15 +920,17 @@ void StructurizeCFG::simplifyAffectedPhis() {
924920
}
925921

926922
/// Remove phi values from all successors and then remove the terminator.
927-
void StructurizeCFG::killTerminator(BasicBlock *BB) {
923+
DebugLoc StructurizeCFG::killTerminator(BasicBlock *BB) {
928924
Instruction *Term = BB->getTerminator();
929925
if (!Term)
930-
return;
926+
return DebugLoc();
931927

932928
for (BasicBlock *Succ : successors(BB))
933929
delPhiValues(BB, Succ);
934930

931+
DebugLoc DL = Term->getDebugLoc();
935932
Term->eraseFromParent();
933+
return DL;
936934
}
937935

938936
/// Let node exit(s) point to NewExit
@@ -971,9 +969,9 @@ void StructurizeCFG::changeExit(RegionNode *Node, BasicBlock *NewExit,
971969
SubRegion->replaceExit(NewExit);
972970
} else {
973971
BasicBlock *BB = Node->getNodeAs<BasicBlock>();
974-
killTerminator(BB);
972+
DebugLoc DL = killTerminator(BB);
975973
BranchInst *Br = BranchInst::Create(NewExit, BB);
976-
Br->setDebugLoc(TermDL[BB]);
974+
Br->setDebugLoc(DL);
977975
addPhiValues(BB, NewExit);
978976
if (IncludeDominator)
979977
DT->changeImmediateDominator(NewExit, BB);
@@ -988,25 +986,20 @@ BasicBlock *StructurizeCFG::getNextFlow(BasicBlock *Dominator) {
988986
BasicBlock *Flow = BasicBlock::Create(Context, FlowBlockName,
989987
Func, Insert);
990988
FlowSet.insert(Flow);
991-
992-
// use a temporary variable to avoid a use-after-free if the map's storage is
993-
// reallocated
994-
DebugLoc DL = TermDL[Dominator];
995-
TermDL[Flow] = std::move(DL);
996-
997989
DT->addNewBlock(Flow, Dominator);
998990
ParentRegion->getRegionInfo()->setRegionFor(Flow, ParentRegion);
999991
return Flow;
1000992
}
1001993

1002-
/// Create a new or reuse the previous node as flow node
1003-
BasicBlock *StructurizeCFG::needPrefix(bool NeedEmpty) {
994+
/// Create a new or reuse the previous node as flow node. Returns a block and a
995+
/// debug location to be used for new instructions in that block.
996+
std::pair<BasicBlock *, DebugLoc> StructurizeCFG::needPrefix(bool NeedEmpty) {
1004997
BasicBlock *Entry = PrevNode->getEntry();
1005998

1006999
if (!PrevNode->isSubRegion()) {
1007-
killTerminator(Entry);
1000+
DebugLoc DL = killTerminator(Entry);
10081001
if (!NeedEmpty || Entry->getFirstInsertionPt() == Entry->end())
1009-
return Entry;
1002+
return {Entry, DL};
10101003
}
10111004

10121005
// create a new flow node
@@ -1015,7 +1008,7 @@ BasicBlock *StructurizeCFG::needPrefix(bool NeedEmpty) {
10151008
// and wire it up
10161009
changeExit(PrevNode, Flow, true);
10171010
PrevNode = ParentRegion->getBBNode(Flow);
1018-
return Flow;
1011+
return {Flow, DebugLoc()};
10191012
}
10201013

10211014
/// Returns the region exit if possible, otherwise just a new flow node
@@ -1079,15 +1072,15 @@ void StructurizeCFG::wireFlow(bool ExitUseAllowed,
10791072
PrevNode = Node;
10801073
} else {
10811074
// Insert extra prefix node (or reuse last one)
1082-
BasicBlock *Flow = needPrefix(false);
1075+
auto [Flow, DL] = needPrefix(false);
10831076

10841077
// Insert extra postfix node (or use exit instead)
10851078
BasicBlock *Entry = Node->getEntry();
10861079
BasicBlock *Next = needPostfix(Flow, ExitUseAllowed);
10871080

10881081
// let it point to entry and next block
10891082
BranchInst *Br = BranchInst::Create(Entry, Next, BoolPoison, Flow);
1090-
Br->setDebugLoc(TermDL[Flow]);
1083+
Br->setDebugLoc(DL);
10911084
Conditions.push_back(Br);
10921085
addPhiValues(Flow, Entry);
10931086
DT->changeImmediateDominator(Entry, Flow);
@@ -1114,7 +1107,7 @@ void StructurizeCFG::handleLoops(bool ExitUseAllowed,
11141107
}
11151108

11161109
if (!isPredictableTrue(Node))
1117-
LoopStart = needPrefix(true);
1110+
LoopStart = needPrefix(true).first;
11181111

11191112
LoopEnd = Loops[Node->getEntry()];
11201113
wireFlow(false, LoopEnd);
@@ -1125,10 +1118,11 @@ void StructurizeCFG::handleLoops(bool ExitUseAllowed,
11251118
assert(LoopStart != &LoopStart->getParent()->getEntryBlock());
11261119

11271120
// Create an extra loop end node
1128-
LoopEnd = needPrefix(false);
1121+
DebugLoc DL;
1122+
std::tie(LoopEnd, DL) = needPrefix(false);
11291123
BasicBlock *Next = needPostfix(LoopEnd, ExitUseAllowed);
11301124
BranchInst *Br = BranchInst::Create(Next, LoopStart, BoolPoison, LoopEnd);
1131-
Br->setDebugLoc(TermDL[LoopEnd]);
1125+
Br->setDebugLoc(DL);
11321126
LoopConds.push_back(Br);
11331127
addPhiValues(LoopEnd, LoopStart);
11341128
setPrevNode(Next);
@@ -1328,7 +1322,6 @@ bool StructurizeCFG::run(Region *R, DominatorTree *DT) {
13281322
LoopPreds.clear();
13291323
LoopConds.clear();
13301324
FlowSet.clear();
1331-
TermDL.clear();
13321325

13331326
return true;
13341327
}

‎llvm/test/CodeGen/AMDGPU/si-annotate-dbg-info.ll‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@ define amdgpu_ps i32 @if_else(i32 %0) !dbg !5 {
1212
; OPT-NEXT: [[TMP4:%.*]] = extractvalue { i1, i64 } [[TMP2]], 1, !dbg [[DBG14]]
1313
; OPT-NEXT: br i1 [[TMP3]], label [[FALSE:%.*]], label [[FLOW:%.*]], !dbg [[DBG14]]
1414
; OPT: Flow:
15-
; OPT-NEXT: [[TMP5:%.*]] = phi i32 [ 33, [[FALSE]] ], [ undef, [[TMP1:%.*]] ]
16-
; OPT-NEXT: [[TMP6:%.*]] = call { i1, i64 } @llvm.amdgcn.else.i64.i64(i64 [[TMP4]]), !dbg [[DBG14]]
17-
; OPT-NEXT: [[TMP7:%.*]] = extractvalue { i1, i64 } [[TMP6]], 0, !dbg [[DBG14]]
18-
; OPT-NEXT: [[TMP8:%.*]] = extractvalue { i1, i64 } [[TMP6]], 1, !dbg [[DBG14]]
19-
; OPT-NEXT: br i1 [[TMP7]], label [[TRUE:%.*]], label [[EXIT:%.*]], !dbg [[DBG14]]
15+
; OPT-NEXT: [[TMP7:%.*]] = phi i32 [ 33, [[FALSE]] ], [ undef, [[TMP1:%.*]] ]
16+
; OPT-NEXT: [[TMP5:%.*]] = call { i1, i64 } @llvm.amdgcn.else.i64.i64(i64 [[TMP4]])
17+
; OPT-NEXT: [[TMP6:%.*]] = extractvalue { i1, i64 } [[TMP5]], 0
18+
; OPT-NEXT: [[TMP8:%.*]] = extractvalue { i1, i64 } [[TMP5]], 1
19+
; OPT-NEXT: br i1 [[TMP6]], label [[TRUE:%.*]], label [[EXIT:%.*]]
2020
; OPT: true:
2121
; OPT-NEXT: br label [[EXIT]], !dbg [[DBG15:![0-9]+]]
2222
; OPT: false:
2323
; OPT-NEXT: br label [[FLOW]], !dbg [[DBG16:![0-9]+]]
2424
; OPT: exit:
25-
; OPT-NEXT: [[RET:%.*]] = phi i32 [ [[TMP5]], [[FLOW]] ], [ 42, [[TRUE]] ], !dbg [[DBG17:![0-9]+]]
25+
; OPT-NEXT: [[RET:%.*]] = phi i32 [ [[TMP7]], [[FLOW]] ], [ 42, [[TRUE]] ], !dbg [[DBG17:![0-9]+]]
2626
; OPT-NEXT: call void @llvm.amdgcn.end.cf.i64(i64 [[TMP8]])
2727
; OPT-NEXT: #dbg_value(i32 [[RET]], [[META11:![0-9]+]], !DIExpression(), [[DBG17]])
2828
; OPT-NEXT: ret i32 [[RET]], !dbg [[DBG18:![0-9]+]]
@@ -66,9 +66,9 @@ define amdgpu_ps void @loop_if_break(i32 %n) !dbg !19 {
6666
; OPT-NEXT: [[TMP3]] = phi i32 [ [[I_NEXT]], [[LOOP_BODY]] ], [ undef, [[LOOP]] ]
6767
; OPT-NEXT: [[TMP4:%.*]] = phi i1 [ false, [[LOOP_BODY]] ], [ true, [[LOOP]] ]
6868
; OPT-NEXT: call void @llvm.amdgcn.end.cf.i64(i64 [[TMP2]])
69-
; OPT-NEXT: [[TMP5]] = call i64 @llvm.amdgcn.if.break.i64(i1 [[TMP4]], i64 [[PHI_BROKEN]]), !dbg [[DBG27]]
70-
; OPT-NEXT: [[TMP6:%.*]] = call i1 @llvm.amdgcn.loop.i64(i64 [[TMP5]]), !dbg [[DBG27]]
71-
; OPT-NEXT: br i1 [[TMP6]], label [[EXIT:%.*]], label [[LOOP]], !dbg [[DBG27]]
69+
; OPT-NEXT: [[TMP5]] = call i64 @llvm.amdgcn.if.break.i64(i1 [[TMP4]], i64 [[PHI_BROKEN]])
70+
; OPT-NEXT: [[TMP6:%.*]] = call i1 @llvm.amdgcn.loop.i64(i64 [[TMP5]])
71+
; OPT-NEXT: br i1 [[TMP6]], label [[EXIT:%.*]], label [[LOOP]]
7272
; OPT: exit:
7373
; OPT-NEXT: call void @llvm.amdgcn.end.cf.i64(i64 [[TMP5]])
7474
; OPT-NEXT: ret void, !dbg [[DBG30:![0-9]+]]

‎llvm/test/Transforms/StructurizeCFG/structurizecfg-debug-loc.ll‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ define void @if_then_else(ptr addrspace(1) %out, i1 %arg) !dbg !7 {
55
; CHECK: entry:
66
; CHECK: br i1 {{.*}}, label %if.else, label %Flow, !dbg [[ITE_ENTRY_DL:![0-9]+]]
77
; CHECK: Flow:
8-
; CHECK: br i1 {{.*}}, label %if.then, label %exit, !dbg [[ITE_ENTRY_DL]]
8+
; CHECK: br i1 {{.*}}, label %if.then, label %exit
99
; CHECK: if.then:
1010
; CHECK: br label %exit, !dbg [[ITE_IFTHEN_DL:![0-9]+]]
1111
; CHECK: if.else:
@@ -36,7 +36,7 @@ define void @while_loop(ptr addrspace(1) %out) !dbg !14 {
3636
; CHECK: while.body:
3737
; CHECK: br label %Flow, !dbg [[WHILE_BODY_DL:![0-9]+]]
3838
; CHECK: Flow:
39-
; CHECK: br i1 {{.*}}, label %exit, label %while.header, !dbg [[WHILE_HEADER_DL]]
39+
; CHECK: br i1 {{.*}}, label %exit, label %while.header
4040
; CHECK: exit:
4141
;
4242
entry:
@@ -63,7 +63,7 @@ define void @while_multiple_exits(ptr addrspace(1) %out) !dbg !21 {
6363
; CHECK: while.exiting:
6464
; CHECK: br label %Flow, !dbg [[WHILEME_EXITING_DL:![0-9]+]]
6565
; CHECK: Flow:
66-
; CHECK: br i1 {{.*}}, label %exit, label %while.header, !dbg [[WHILEME_HEADER_DL]]
66+
; CHECK: br i1 {{.*}}, label %exit, label %while.header
6767
; CHECK: exit:
6868
;
6969
entry:
@@ -86,27 +86,27 @@ define void @nested_if_then_else(ptr addrspace(1) %out, i1 %a, i1 %b) !dbg !28 {
8686
; CHECK: entry:
8787
; CHECK: br i1 {{.*}}, label %if.else, label %Flow4, !dbg [[NESTED_ENTRY_DL:![0-9]+]]
8888
; CHECK: Flow4:
89-
; CHECK: br i1 {{.*}}, label %if.then, label %exit, !dbg [[NESTED_ENTRY_DL]]
89+
; CHECK: br i1 {{.*}}, label %if.then, label %exit
9090
; CHECK: if.then:
9191
; CHECK: br i1 {{.*}}, label %if.then.else, label %Flow2, !dbg [[NESTED_IFTHEN_DL:![0-9]+]]
9292
; CHECK: Flow2:
93-
; CHECK: br i1 {{.*}}, label %if.then.then, label %Flow3, !dbg [[NESTED_IFTHEN_DL]]
93+
; CHECK: br i1 {{.*}}, label %if.then.then, label %Flow3
9494
; CHECK: if.then.then:
9595
; CHECK: br label %Flow3, !dbg [[NESTED_IFTHENTHEN_DL:![0-9]+]]
9696
; CHECK: if.then.else:
9797
; CHECK: br label %Flow2, !dbg [[NESTED_IFTHENELSE_DL:![0-9]+]]
9898
; CHECK: if.else:
9999
; CHECK: br i1 {{.*}}, label %if.else.else, label %Flow, !dbg [[NESTED_IFELSE_DL:![0-9]+]]
100100
; CHECK: Flow:
101-
; CHECK: br i1 {{.*}}, label %if.else.then, label %Flow1, !dbg [[NESTED_IFELSE_DL]]
101+
; CHECK: br i1 {{.*}}, label %if.else.then, label %Flow1
102102
; CHECK: if.else.then:
103103
; CHECK: br label %Flow1, !dbg [[NESTED_IFELSETHEN_DL:![0-9]+]]
104104
; CHECK: if.else.else:
105105
; CHECK: br label %Flow, !dbg [[NESTED_IFELSEELSE_DL:![0-9]+]]
106106
; CHECK: Flow1:
107-
; CHECK: br label %Flow4, !dbg [[NESTED_IFELSE_DL]]
107+
; CHECK: br label %Flow4
108108
; CHECK: Flow3:
109-
; CHECK: br label %exit, !dbg [[NESTED_IFTHEN_DL]]
109+
; CHECK: br label %exit
110110
; CHECK: exit:
111111
;
112112
entry:

0 commit comments

Comments
 (0)