Skip to content

Commit 7babf22

Browse files
epilkepilk
authored
[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 49c22e3 commit 7babf22

File tree

3 files changed

+36
-61
lines changed

3 files changed

+36
-61
lines changed

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

Lines changed: 20 additions & 45 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

@@ -361,8 +357,6 @@ class StructurizeCFG {
361357

362358
void rebuildSSA();
363359

364-
void setDebugLoc(BranchInst *Br, BasicBlock *BB);
365-
366360
public:
367361
void init(Region *R);
368362
bool run(Region *R, DominatorTree *DT);
@@ -918,28 +912,18 @@ void StructurizeCFG::simplifyAffectedPhis() {
918912
} while (Changed);
919913
}
920914

921-
void StructurizeCFG::setDebugLoc(BranchInst *Br, BasicBlock *BB) {
922-
auto I = TermDL.find(BB);
923-
if (I == TermDL.end())
924-
return;
925-
926-
Br->setDebugLoc(I->second);
927-
TermDL.erase(I);
928-
}
929-
930915
/// Remove phi values from all successors and then remove the terminator.
931-
void StructurizeCFG::killTerminator(BasicBlock *BB) {
916+
DebugLoc StructurizeCFG::killTerminator(BasicBlock *BB) {
932917
Instruction *Term = BB->getTerminator();
933918
if (!Term)
934-
return;
935-
936-
if (const DebugLoc &DL = Term->getDebugLoc())
937-
TermDL[BB] = DL;
919+
return DebugLoc();
938920

939921
for (BasicBlock *Succ : successors(BB))
940922
delPhiValues(BB, Succ);
941923

924+
DebugLoc DL = Term->getDebugLoc();
942925
Term->eraseFromParent();
926+
return DL;
943927
}
944928

945929
/// Let node exit(s) point to NewExit
@@ -978,9 +962,9 @@ void StructurizeCFG::changeExit(RegionNode *Node, BasicBlock *NewExit,
978962
SubRegion->replaceExit(NewExit);
979963
} else {
980964
BasicBlock *BB = Node->getNodeAs<BasicBlock>();
981-
killTerminator(BB);
965+
DebugLoc DL = killTerminator(BB);
982966
BranchInst *Br = BranchInst::Create(NewExit, BB);
983-
setDebugLoc(Br, BB);
967+
Br->setDebugLoc(DL);
984968
addPhiValues(BB, NewExit);
985969
if (IncludeDominator)
986970
DT->changeImmediateDominator(NewExit, BB);
@@ -995,29 +979,20 @@ BasicBlock *StructurizeCFG::getNextFlow(BasicBlock *Dominator) {
995979
BasicBlock *Flow = BasicBlock::Create(Context, FlowBlockName,
996980
Func, Insert);
997981
FlowSet.insert(Flow);
998-
999-
if (auto *Term = Dominator->getTerminator()) {
1000-
if (const DebugLoc &DL = Term->getDebugLoc())
1001-
TermDL[Flow] = DL;
1002-
} else if (DebugLoc DLTemp = TermDL.lookup(Dominator)) {
1003-
// Use a temporary copy to avoid a use-after-free if the map's storage
1004-
// is reallocated.
1005-
TermDL[Flow] = DLTemp;
1006-
}
1007-
1008982
DT->addNewBlock(Flow, Dominator);
1009983
ParentRegion->getRegionInfo()->setRegionFor(Flow, ParentRegion);
1010984
return Flow;
1011985
}
1012986

1013-
/// Create a new or reuse the previous node as flow node
1014-
BasicBlock *StructurizeCFG::needPrefix(bool NeedEmpty) {
987+
/// Create a new or reuse the previous node as flow node. Returns a block and a
988+
/// debug location to be used for new instructions in that block.
989+
std::pair<BasicBlock *, DebugLoc> StructurizeCFG::needPrefix(bool NeedEmpty) {
1015990
BasicBlock *Entry = PrevNode->getEntry();
1016991

1017992
if (!PrevNode->isSubRegion()) {
1018-
killTerminator(Entry);
993+
DebugLoc DL = killTerminator(Entry);
1019994
if (!NeedEmpty || Entry->getFirstInsertionPt() == Entry->end())
1020-
return Entry;
995+
return {Entry, DL};
1021996
}
1022997

1023998
// create a new flow node
@@ -1026,7 +1001,7 @@ BasicBlock *StructurizeCFG::needPrefix(bool NeedEmpty) {
10261001
// and wire it up
10271002
changeExit(PrevNode, Flow, true);
10281003
PrevNode = ParentRegion->getBBNode(Flow);
1029-
return Flow;
1004+
return {Flow, DebugLoc()};
10301005
}
10311006

10321007
/// Returns the region exit if possible, otherwise just a new flow node
@@ -1090,15 +1065,15 @@ void StructurizeCFG::wireFlow(bool ExitUseAllowed,
10901065
PrevNode = Node;
10911066
} else {
10921067
// Insert extra prefix node (or reuse last one)
1093-
BasicBlock *Flow = needPrefix(false);
1068+
auto [Flow, DL] = needPrefix(false);
10941069

10951070
// Insert extra postfix node (or use exit instead)
10961071
BasicBlock *Entry = Node->getEntry();
10971072
BasicBlock *Next = needPostfix(Flow, ExitUseAllowed);
10981073

10991074
// let it point to entry and next block
11001075
BranchInst *Br = BranchInst::Create(Entry, Next, BoolPoison, Flow);
1101-
setDebugLoc(Br, Flow);
1076+
Br->setDebugLoc(DL);
11021077
Conditions.push_back(Br);
11031078
addPhiValues(Flow, Entry);
11041079
DT->changeImmediateDominator(Entry, Flow);
@@ -1125,7 +1100,7 @@ void StructurizeCFG::handleLoops(bool ExitUseAllowed,
11251100
}
11261101

11271102
if (!isPredictableTrue(Node))
1128-
LoopStart = needPrefix(true);
1103+
LoopStart = needPrefix(true).first;
11291104

11301105
LoopEnd = Loops[Node->getEntry()];
11311106
wireFlow(false, LoopEnd);
@@ -1136,10 +1111,11 @@ void StructurizeCFG::handleLoops(bool ExitUseAllowed,
11361111
assert(LoopStart != &LoopStart->getParent()->getEntryBlock());
11371112

11381113
// Create an extra loop end node
1139-
LoopEnd = needPrefix(false);
1114+
DebugLoc DL;
1115+
std::tie(LoopEnd, DL) = needPrefix(false);
11401116
BasicBlock *Next = needPostfix(LoopEnd, ExitUseAllowed);
11411117
BranchInst *Br = BranchInst::Create(Next, LoopStart, BoolPoison, LoopEnd);
1142-
setDebugLoc(Br, LoopEnd);
1118+
Br->setDebugLoc(DL);
11431119
LoopConds.push_back(Br);
11441120
addPhiValues(LoopEnd, LoopStart);
11451121
setPrevNode(Next);
@@ -1339,7 +1315,6 @@ bool StructurizeCFG::run(Region *R, DominatorTree *DT) {
13391315
LoopPreds.clear();
13401316
LoopConds.clear();
13411317
FlowSet.clear();
1342-
TermDL.clear();
13431318

13441319
return true;
13451320
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ define amdgpu_ps i32 @if_else(i32 %0) !dbg !5 {
1111
; OPT-NEXT: [[TMP4:%.*]] = extractvalue { i1, i64 } [[TMP2]], 1, !dbg [[DBG14]]
1212
; OPT-NEXT: br i1 [[TMP3]], label [[FALSE:%.*]], label [[FLOW:%.*]], !dbg [[DBG14]]
1313
; OPT: Flow:
14-
; OPT-NEXT: [[TMP5:%.*]] = call { i1, i64 } @llvm.amdgcn.else.i64.i64(i64 [[TMP4]]), !dbg [[DBG14]]
15-
; OPT-NEXT: [[TMP6:%.*]] = extractvalue { i1, i64 } [[TMP5]], 0, !dbg [[DBG14]]
16-
; OPT-NEXT: [[TMP8:%.*]] = extractvalue { i1, i64 } [[TMP5]], 1, !dbg [[DBG14]]
17-
; OPT-NEXT: br i1 [[TMP6]], label [[TRUE:%.*]], label [[EXIT:%.*]], !dbg [[DBG14]]
14+
; OPT-NEXT: [[TMP5:%.*]] = call { i1, i64 } @llvm.amdgcn.else.i64.i64(i64 [[TMP4]])
15+
; OPT-NEXT: [[TMP6:%.*]] = extractvalue { i1, i64 } [[TMP5]], 0
16+
; OPT-NEXT: [[TMP8:%.*]] = extractvalue { i1, i64 } [[TMP5]], 1
17+
; OPT-NEXT: br i1 [[TMP6]], label [[TRUE:%.*]], label [[EXIT:%.*]]
1818
; OPT: true:
1919
; OPT-NEXT: br label [[EXIT]], !dbg [[DBG15:![0-9]+]]
2020
; OPT: false:
@@ -64,9 +64,9 @@ define amdgpu_ps void @loop_if_break(i32 %n) !dbg !19 {
6464
; OPT-NEXT: [[TMP3]] = phi i32 [ [[I_NEXT]], [[LOOP_BODY]] ], [ poison, [[LOOP]] ]
6565
; OPT-NEXT: [[TMP4:%.*]] = phi i1 [ false, [[LOOP_BODY]] ], [ true, [[LOOP]] ]
6666
; OPT-NEXT: call void @llvm.amdgcn.end.cf.i64(i64 [[TMP2]])
67-
; OPT-NEXT: [[TMP5]] = call i64 @llvm.amdgcn.if.break.i64(i1 [[TMP4]], i64 [[PHI_BROKEN]]), !dbg [[DBG27]]
68-
; OPT-NEXT: [[TMP6:%.*]] = call i1 @llvm.amdgcn.loop.i64(i64 [[TMP5]]), !dbg [[DBG27]]
69-
; OPT-NEXT: br i1 [[TMP6]], label [[EXIT:%.*]], label [[LOOP]], !dbg [[DBG27]]
67+
; OPT-NEXT: [[TMP5]] = call i64 @llvm.amdgcn.if.break.i64(i1 [[TMP4]], i64 [[PHI_BROKEN]])
68+
; OPT-NEXT: [[TMP6:%.*]] = call i1 @llvm.amdgcn.loop.i64(i64 [[TMP5]])
69+
; OPT-NEXT: br i1 [[TMP6]], label [[EXIT:%.*]], label [[LOOP]]
7070
; OPT: exit:
7171
; OPT-NEXT: call void @llvm.amdgcn.end.cf.i64(i64 [[TMP5]])
7272
; OPT-NEXT: ret void, !dbg [[DBG30:![0-9]+]]
@@ -132,7 +132,7 @@ attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memo
132132
!30 = !DILocation(line: 13, column: 1, scope: !19)
133133
;.
134134
; OPT: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C, file: [[META1:![0-9]+]], producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
135-
; OPT: [[META1]] = !DIFile(filename: "../../../test/CodeGen/AMDGPU/si-annotate-dbg-info.ll", directory: {{.*}})
135+
; OPT: [[META1]] = !DIFile(filename: "{{.*}}si-annotate-dbg-info.ll", directory: {{.*}})
136136
; OPT: [[DBG5]] = distinct !DISubprogram(name: "if_else", linkageName: "if_else", scope: null, file: [[META1]], line: 1, type: [[META6:![0-9]+]], scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]], retainedNodes: [[META8:![0-9]+]])
137137
; OPT: [[META6]] = !DISubroutineType(types: [[META7:![0-9]+]])
138138
; OPT: [[META7]] = !{}

‎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)