Skip to content

Commit 22d9b3a

Browse files
abidhabidh
committed
[IRBuilder] Improve setting of DebugLoc in SetInsertPoint.
Currently, in SetInsertPoint(BasicBlock *TheBB, BasicBlock::iterator IP) if the IP is pointing to the end of the TheBB, the debugLoc is not changed although the insertion point now points to a different location. This has been source of many problems in the OMPIRBuilder. In OMPIRBuilder, the following code pattern is quite common. auto OldIP = Builder.saveIP(); Builder.SetInsertPoint(/* some new location */); // Generate some code. Builder.restoreIP(OldIP); An example can be seen here. If the OldIP is pointing to the end of the BasicBlock, SetInsertPoint will not call setCurrentDebugLocation(). This causes many subtle bugs like this. I fixed it by by using the debug location of the last instruction in the BasicBlock if the InsertPoint is at the end. Some clang OpenMP tests needed minor adjustments to work with this change. Fixes #147063.
1 parent ec25a05 commit 22d9b3a

File tree

6 files changed

+48
-13
lines changed

6 files changed

+48
-13
lines changed

‎clang/test/OpenMP/parallel_codegen.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ int main (int argc, char **argv) {
354354
// CHECK2-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG54:![0-9]+]]
355355
// CHECK2-NEXT: [[TMP2:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !dbg [[DBG54]]
356356
// CHECK2-NEXT: invoke void @_Z3fooIiEvT_(i32 noundef [[TMP2]])
357-
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG53]]
357+
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG54]]
358358
// CHECK2: invoke.cont:
359359
// CHECK2-NEXT: [[TMP3:%.*]] = load i32, ptr @global, align 4, !dbg [[DBG55:![0-9]+]]
360360
// CHECK2-NEXT: [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG56:![0-9]+]]
@@ -480,7 +480,7 @@ int main (int argc, char **argv) {
480480
// CHECK2-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG107:![0-9]+]]
481481
// CHECK2-NEXT: [[TMP3:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !dbg [[DBG107]]
482482
// CHECK2-NEXT: invoke void @_Z3fooIiEvT_(i32 noundef [[TMP3]])
483-
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG106]]
483+
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG107]]
484484
// CHECK2: invoke.cont:
485485
// CHECK2-NEXT: [[TMP4:%.*]] = load i32, ptr [[TMP2]], align 4, !dbg [[DBG108:![0-9]+]]
486486
// CHECK2-NEXT: [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG109:![0-9]+]]
@@ -588,7 +588,7 @@ int main (int argc, char **argv) {
588588
// CHECK2-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG143:![0-9]+]]
589589
// CHECK2-NEXT: [[TMP2:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !dbg [[DBG143]]
590590
// CHECK2-NEXT: invoke void @_Z3fooIiEvT_(i32 noundef [[TMP2]])
591-
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG142]]
591+
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG143]]
592592
// CHECK2: invoke.cont:
593593
// CHECK2-NEXT: [[TMP3:%.*]] = load i32, ptr @global, align 4, !dbg [[DBG144:![0-9]+]]
594594
// CHECK2-NEXT: [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG145:![0-9]+]]
@@ -662,7 +662,7 @@ int main (int argc, char **argv) {
662662
// CHECK2-NEXT: [[TMP1:%.*]] = load i64, ptr [[VLA_ADDR]], align 8, !dbg [[DBG175]]
663663
// CHECK2-NEXT: [[TMP2:%.*]] = load ptr, ptr [[TMP0]], align 8, !dbg [[DBG176:![0-9]+]]
664664
// CHECK2-NEXT: invoke void @_Z3fooIPPcEvT_(ptr noundef [[TMP2]])
665-
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG178:![0-9]+]]
665+
// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG176]]
666666
// CHECK2: invoke.cont:
667667
// CHECK2-NEXT: #dbg_declare(ptr [[VAR]], [[META179:![0-9]+]], !DIExpression(), [[META186:![0-9]+]])
668668
// CHECK2-NEXT: [[TMP3:%.*]] = load ptr, ptr [[VAR]], align 8, !dbg [[DBG187:![0-9]+]]
@@ -672,7 +672,7 @@ int main (int argc, char **argv) {
672672
// CHECK2-NEXT: ret void, !dbg [[DBG188:![0-9]+]]
673673
// CHECK2: terminate.lpad:
674674
// CHECK2-NEXT: [[TMP5:%.*]] = landingpad { ptr, i32 }
675-
// CHECK2-NEXT: catch ptr null, !dbg [[DBG178]]
675+
// CHECK2-NEXT: catch ptr null, !dbg [[DBG178:![0-9]+]]
676676
// CHECK2-NEXT: [[TMP6:%.*]] = extractvalue { ptr, i32 } [[TMP5]], 0, !dbg [[DBG178]]
677677
// CHECK2-NEXT: call void @__clang_call_terminate(ptr [[TMP6]]) #[[ATTR6]], !dbg [[DBG178]]
678678
// CHECK2-NEXT: unreachable, !dbg [[DBG178]]

‎clang/test/OpenMP/parallel_for_codegen.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3530,9 +3530,9 @@ void range_for_collapsed() {
35303530
// CHECK5-NEXT: [[ADD:%.*]] = add i32 131071, [[MUL]], !dbg [[DBG110]]
35313531
// CHECK5-NEXT: store i32 [[ADD]], ptr [[I]], align 4, !dbg [[DBG110]]
35323532
// CHECK5-NEXT: [[CALL:%.*]] = invoke noundef i32 @_Z3foov()
3533-
// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG111:![0-9]+]]
3533+
// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG110]]
35343534
// CHECK5: invoke.cont:
3535-
// CHECK5-NEXT: [[CONV:%.*]] = sitofp i32 [[CALL]] to float, !dbg [[DBG111]]
3535+
// CHECK5-NEXT: [[CONV:%.*]] = sitofp i32 [[CALL]] to float, !dbg [[DBG111:![0-9]+]]
35363536
// CHECK5-NEXT: [[TMP13:%.*]] = load i32, ptr [[I]], align 4, !dbg [[DBG111]]
35373537
// CHECK5-NEXT: [[IDXPROM:%.*]] = zext i32 [[TMP13]] to i64, !dbg [[DBG111]]
35383538
// CHECK5-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds nuw float, ptr [[VLA1]], i64 [[IDXPROM]], !dbg [[DBG111]]

‎clang/test/OpenMP/scope_codegen.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,7 +1584,7 @@ int main() {
15841584
// CHECK5-NEXT: [[TMP1:%.*]] = load i8, ptr [[A]], align 1, !dbg [[DBG42:![0-9]+]]
15851585
// CHECK5-NEXT: store i8 [[TMP1]], ptr [[A1]], align 1, !dbg [[DBG42]]
15861586
// CHECK5-NEXT: invoke void @_ZN9TestClassC1ERKS_(ptr noundef nonnull align 4 dereferenceable(4) [[C2]], ptr noundef nonnull align 4 dereferenceable(4) @tc)
1587-
// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG43:![0-9]+]]
1587+
// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG42]]
15881588
// CHECK5: invoke.cont:
15891589
// CHECK5-NEXT: store ptr [[C2]], ptr [[TMP]], align 8, !dbg [[DBG44:![0-9]+]]
15901590
// CHECK5-NEXT: invoke void @_ZN9TestClassC1ERKS_(ptr noundef nonnull align 4 dereferenceable(4) [[TC]], ptr noundef nonnull align 4 dereferenceable(4) @tc)
@@ -1623,7 +1623,7 @@ int main() {
16231623
// CHECK5-NEXT: ret i32 [[CONV]], !dbg [[DBG50:![0-9]+]]
16241624
// CHECK5: terminate.lpad:
16251625
// CHECK5-NEXT: [[TMP4:%.*]] = landingpad { ptr, i32 }
1626-
// CHECK5-NEXT: catch ptr null, !dbg [[DBG43]]
1626+
// CHECK5-NEXT: catch ptr null, !dbg [[DBG43:![0-9]+]]
16271627
// CHECK5-NEXT: [[TMP5:%.*]] = extractvalue { ptr, i32 } [[TMP4]], 0, !dbg [[DBG43]]
16281628
// CHECK5-NEXT: call void @__clang_call_terminate(ptr [[TMP5]]) #[[ATTR10:[0-9]+]], !dbg [[DBG43]]
16291629
// CHECK5-NEXT: unreachable, !dbg [[DBG43]]

‎clang/test/OpenMP/taskgroup_codegen.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,9 @@ void parallel_taskgroup() {
135135
// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB1]], i32 [[TMP0]]), !dbg [[DBG15:![0-9]+]]
136136
// DEBUG1-NEXT: call void @__kmpc_taskgroup(ptr @[[GLOB3:[0-9]+]], i32 [[TMP0]]), !dbg [[DBG16:![0-9]+]]
137137
// DEBUG1-NEXT: invoke void @_Z3foov()
138-
// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG17:![0-9]+]]
138+
// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG16:![0-9]+]]
139139
// DEBUG1: invoke.cont:
140-
// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB3]], i32 [[TMP0]]), !dbg [[DBG17]]
140+
// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB3]], i32 [[TMP0]]), !dbg [[DBG17:![0-9]+]]
141141
// DEBUG1-NEXT: [[TMP1:%.*]] = load i8, ptr [[A]], align 1, !dbg [[DBG18:![0-9]+]]
142142
// DEBUG1-NEXT: [[CONV:%.*]] = sext i8 [[TMP1]] to i32, !dbg [[DBG18]]
143143
// DEBUG1-NEXT: ret i32 [[CONV]], !dbg [[DBG19:![0-9]+]]
@@ -174,9 +174,9 @@ void parallel_taskgroup() {
174174
// DEBUG1-NEXT: [[TMP1:%.*]] = load i32, ptr [[TMP0]], align 4, !dbg [[DBG24]]
175175
// DEBUG1-NEXT: call void @__kmpc_taskgroup(ptr @[[GLOB5:[0-9]+]], i32 [[TMP1]]), !dbg [[DBG24]]
176176
// DEBUG1-NEXT: invoke void @_Z3foov()
177-
// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG25:![0-9]+]]
177+
// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG24]]
178178
// DEBUG1: invoke.cont:
179-
// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB5]], i32 [[TMP1]]), !dbg [[DBG25]]
179+
// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB5]], i32 [[TMP1]]), !dbg [[DBG25:![0-9]+]]
180180
// DEBUG1-NEXT: ret void, !dbg [[DBG26:![0-9]+]]
181181
// DEBUG1: terminate.lpad:
182182
// DEBUG1-NEXT: [[TMP2:%.*]] = landingpad { ptr, i32 }

‎llvm/include/llvm/IR/IRBuilder.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ class IRBuilderBase {
225225
InsertPt = IP;
226226
if (IP != TheBB->end())
227227
SetCurrentDebugLocation(IP->getStableDebugLoc());
228+
else if (!BB->empty() && BB->back().getStableDebugLoc())
229+
SetCurrentDebugLocation(BB->back().getStableDebugLoc());
228230
}
229231

230232
/// This specifies that created instructions should be inserted at

‎llvm/unittests/IR/IRBuilderTest.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,39 @@ TEST_F(IRBuilderTest, DebugLoc) {
11891189
DIB.finalize();
11901190
}
11911191

1192+
TEST_F(IRBuilderTest, RestoreDebugLocation) {
1193+
DIBuilder DIB(*M);
1194+
auto File = DIB.createFile("tmp.cpp", "/");
1195+
auto CU =
1196+
DIB.createCompileUnit(dwarf::DW_LANG_C_plus_plus_11,
1197+
DIB.createFile("tmp.cpp", "/"), "", true, "", 0);
1198+
auto SPType = DIB.createSubroutineType(DIB.getOrCreateTypeArray({}));
1199+
auto SP =
1200+
DIB.createFunction(CU, "foo", "foo", File, 1, SPType, 1, DINode::FlagZero,
1201+
DISubprogram::SPFlagDefinition);
1202+
DebugLoc DL1 = DILocation::get(Ctx, 2, 0, SP);
1203+
DebugLoc DL2 = DILocation::get(Ctx, 3, 0, SP);
1204+
1205+
IRBuilder<> Builder(Ctx);
1206+
auto BB1 = BasicBlock::Create(Ctx, "bb1", F);
1207+
Builder.SetInsertPoint(BB1);
1208+
Builder.SetCurrentDebugLocation(DL1);
1209+
Builder.CreateAlloca(Builder.getInt8Ty());
1210+
Builder.CreateAlloca(Builder.getInt32Ty());
1211+
EXPECT_EQ(DL1, Builder.getCurrentDebugLocation());
1212+
auto IP = Builder.saveIP();
1213+
1214+
auto BB2 = BasicBlock::Create(Ctx, "bb2", F);
1215+
Builder.SetInsertPoint(BB2);
1216+
Builder.SetCurrentDebugLocation(DL2);
1217+
Builder.CreateAlloca(Builder.getInt32Ty());
1218+
EXPECT_EQ(DL2, Builder.getCurrentDebugLocation());
1219+
1220+
Builder.restoreIP(IP);
1221+
EXPECT_EQ(DL1, Builder.getCurrentDebugLocation());
1222+
DIB.finalize();
1223+
}
1224+
11921225
TEST_F(IRBuilderTest, DIImportedEntity) {
11931226
IRBuilder<> Builder(BB);
11941227
DIBuilder DIB(*M);

0 commit comments

Comments
 (0)