[IRBuilder] Improve setting of DebugLoc in SetInsertPoint. #147091
+48
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 llvm#147063.
abidh
requested review from
nikic,
skatrak,
tblah,
SLTozer,
alexey-bataev,
jdoerfert and
ddpagan
llvmbot
added
clang
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-clang Author: Abid Qadeer (abidh) ChangesCurrently, in In An example can be seen here. If the I fixed it by by using the debug location of the last instruction in the Fixes #147063. Full diff: https://github.com/llvm/llvm-project/pull/147091.diff 6 Files Affected:
diff --git a/clang/test/OpenMP/parallel_codegen.cpp b/clang/test/OpenMP/parallel_codegen.cpp
index e8e57aedaa164..6be8ff5149f2a 100644
--- a/clang/test/OpenMP/parallel_codegen.cpp
+++ b/clang/test/OpenMP/parallel_codegen.cpp
@@ -354,7 +354,7 @@ int main (int argc, char **argv) {
// CHECK2-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG54:![0-9]+]]
// CHECK2-NEXT: [[TMP2:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !dbg [[DBG54]]
// CHECK2-NEXT: invoke void @_Z3fooIiEvT_(i32 noundef [[TMP2]])
-// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG53]]
+// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG54]]
// CHECK2: invoke.cont:
// CHECK2-NEXT: [[TMP3:%.*]] = load i32, ptr @global, align 4, !dbg [[DBG55:![0-9]+]]
// CHECK2-NEXT: [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG56:![0-9]+]]
@@ -480,7 +480,7 @@ int main (int argc, char **argv) {
// CHECK2-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG107:![0-9]+]]
// CHECK2-NEXT: [[TMP3:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !dbg [[DBG107]]
// CHECK2-NEXT: invoke void @_Z3fooIiEvT_(i32 noundef [[TMP3]])
-// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG106]]
+// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG107]]
// CHECK2: invoke.cont:
// CHECK2-NEXT: [[TMP4:%.*]] = load i32, ptr [[TMP2]], align 4, !dbg [[DBG108:![0-9]+]]
// CHECK2-NEXT: [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG109:![0-9]+]]
@@ -588,7 +588,7 @@ int main (int argc, char **argv) {
// CHECK2-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG143:![0-9]+]]
// CHECK2-NEXT: [[TMP2:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !dbg [[DBG143]]
// CHECK2-NEXT: invoke void @_Z3fooIiEvT_(i32 noundef [[TMP2]])
-// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG142]]
+// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG143]]
// CHECK2: invoke.cont:
// CHECK2-NEXT: [[TMP3:%.*]] = load i32, ptr @global, align 4, !dbg [[DBG144:![0-9]+]]
// CHECK2-NEXT: [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 1, !dbg [[DBG145:![0-9]+]]
@@ -662,7 +662,7 @@ int main (int argc, char **argv) {
// CHECK2-NEXT: [[TMP1:%.*]] = load i64, ptr [[VLA_ADDR]], align 8, !dbg [[DBG175]]
// CHECK2-NEXT: [[TMP2:%.*]] = load ptr, ptr [[TMP0]], align 8, !dbg [[DBG176:![0-9]+]]
// CHECK2-NEXT: invoke void @_Z3fooIPPcEvT_(ptr noundef [[TMP2]])
-// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG178:![0-9]+]]
+// CHECK2-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG176]]
// CHECK2: invoke.cont:
// CHECK2-NEXT: #dbg_declare(ptr [[VAR]], [[META179:![0-9]+]], !DIExpression(), [[META186:![0-9]+]])
// CHECK2-NEXT: [[TMP3:%.*]] = load ptr, ptr [[VAR]], align 8, !dbg [[DBG187:![0-9]+]]
@@ -672,7 +672,7 @@ int main (int argc, char **argv) {
// CHECK2-NEXT: ret void, !dbg [[DBG188:![0-9]+]]
// CHECK2: terminate.lpad:
// CHECK2-NEXT: [[TMP5:%.*]] = landingpad { ptr, i32 }
-// CHECK2-NEXT: catch ptr null, !dbg [[DBG178]]
+// CHECK2-NEXT: catch ptr null, !dbg [[DBG178:![0-9]+]]
// CHECK2-NEXT: [[TMP6:%.*]] = extractvalue { ptr, i32 } [[TMP5]], 0, !dbg [[DBG178]]
// CHECK2-NEXT: call void @__clang_call_terminate(ptr [[TMP6]]) #[[ATTR6]], !dbg [[DBG178]]
// CHECK2-NEXT: unreachable, !dbg [[DBG178]]
diff --git a/clang/test/OpenMP/parallel_for_codegen.cpp b/clang/test/OpenMP/parallel_for_codegen.cpp
index c7afae419509b..ed60a1d1febbb 100644
--- a/clang/test/OpenMP/parallel_for_codegen.cpp
+++ b/clang/test/OpenMP/parallel_for_codegen.cpp
@@ -3530,9 +3530,9 @@ void range_for_collapsed() {
// CHECK5-NEXT: [[ADD:%.*]] = add i32 131071, [[MUL]], !dbg [[DBG110]]
// CHECK5-NEXT: store i32 [[ADD]], ptr [[I]], align 4, !dbg [[DBG110]]
// CHECK5-NEXT: [[CALL:%.*]] = invoke noundef i32 @_Z3foov()
-// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG111:![0-9]+]]
+// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG110]]
// CHECK5: invoke.cont:
-// CHECK5-NEXT: [[CONV:%.*]] = sitofp i32 [[CALL]] to float, !dbg [[DBG111]]
+// CHECK5-NEXT: [[CONV:%.*]] = sitofp i32 [[CALL]] to float, !dbg [[DBG111:![0-9]+]]
// CHECK5-NEXT: [[TMP13:%.*]] = load i32, ptr [[I]], align 4, !dbg [[DBG111]]
// CHECK5-NEXT: [[IDXPROM:%.*]] = zext i32 [[TMP13]] to i64, !dbg [[DBG111]]
// CHECK5-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds nuw float, ptr [[VLA1]], i64 [[IDXPROM]], !dbg [[DBG111]]
diff --git a/clang/test/OpenMP/scope_codegen.cpp b/clang/test/OpenMP/scope_codegen.cpp
index ef69b8302fa2d..13b369d9c2989 100644
--- a/clang/test/OpenMP/scope_codegen.cpp
+++ b/clang/test/OpenMP/scope_codegen.cpp
@@ -1584,7 +1584,7 @@ int main() {
// CHECK5-NEXT: [[TMP1:%.*]] = load i8, ptr [[A]], align 1, !dbg [[DBG42:![0-9]+]]
// CHECK5-NEXT: store i8 [[TMP1]], ptr [[A1]], align 1, !dbg [[DBG42]]
// CHECK5-NEXT: invoke void @_ZN9TestClassC1ERKS_(ptr noundef nonnull align 4 dereferenceable(4) [[C2]], ptr noundef nonnull align 4 dereferenceable(4) @tc)
-// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG43:![0-9]+]]
+// CHECK5-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG42]]
// CHECK5: invoke.cont:
// CHECK5-NEXT: store ptr [[C2]], ptr [[TMP]], align 8, !dbg [[DBG44:![0-9]+]]
// 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() {
// CHECK5-NEXT: ret i32 [[CONV]], !dbg [[DBG50:![0-9]+]]
// CHECK5: terminate.lpad:
// CHECK5-NEXT: [[TMP4:%.*]] = landingpad { ptr, i32 }
-// CHECK5-NEXT: catch ptr null, !dbg [[DBG43]]
+// CHECK5-NEXT: catch ptr null, !dbg [[DBG43:![0-9]+]]
// CHECK5-NEXT: [[TMP5:%.*]] = extractvalue { ptr, i32 } [[TMP4]], 0, !dbg [[DBG43]]
// CHECK5-NEXT: call void @__clang_call_terminate(ptr [[TMP5]]) #[[ATTR10:[0-9]+]], !dbg [[DBG43]]
// CHECK5-NEXT: unreachable, !dbg [[DBG43]]
diff --git a/clang/test/OpenMP/taskgroup_codegen.cpp b/clang/test/OpenMP/taskgroup_codegen.cpp
index 72653144d08dd..3055f1855c3a9 100644
--- a/clang/test/OpenMP/taskgroup_codegen.cpp
+++ b/clang/test/OpenMP/taskgroup_codegen.cpp
@@ -135,9 +135,9 @@ void parallel_taskgroup() {
// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB1]], i32 [[TMP0]]), !dbg [[DBG15:![0-9]+]]
// DEBUG1-NEXT: call void @__kmpc_taskgroup(ptr @[[GLOB3:[0-9]+]], i32 [[TMP0]]), !dbg [[DBG16:![0-9]+]]
// DEBUG1-NEXT: invoke void @_Z3foov()
-// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG17:![0-9]+]]
+// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG16:![0-9]+]]
// DEBUG1: invoke.cont:
-// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB3]], i32 [[TMP0]]), !dbg [[DBG17]]
+// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB3]], i32 [[TMP0]]), !dbg [[DBG17:![0-9]+]]
// DEBUG1-NEXT: [[TMP1:%.*]] = load i8, ptr [[A]], align 1, !dbg [[DBG18:![0-9]+]]
// DEBUG1-NEXT: [[CONV:%.*]] = sext i8 [[TMP1]] to i32, !dbg [[DBG18]]
// DEBUG1-NEXT: ret i32 [[CONV]], !dbg [[DBG19:![0-9]+]]
@@ -174,9 +174,9 @@ void parallel_taskgroup() {
// DEBUG1-NEXT: [[TMP1:%.*]] = load i32, ptr [[TMP0]], align 4, !dbg [[DBG24]]
// DEBUG1-NEXT: call void @__kmpc_taskgroup(ptr @[[GLOB5:[0-9]+]], i32 [[TMP1]]), !dbg [[DBG24]]
// DEBUG1-NEXT: invoke void @_Z3foov()
-// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG25:![0-9]+]]
+// DEBUG1-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[TERMINATE_LPAD:%.*]], !dbg [[DBG24]]
// DEBUG1: invoke.cont:
-// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB5]], i32 [[TMP1]]), !dbg [[DBG25]]
+// DEBUG1-NEXT: call void @__kmpc_end_taskgroup(ptr @[[GLOB5]], i32 [[TMP1]]), !dbg [[DBG25:![0-9]+]]
// DEBUG1-NEXT: ret void, !dbg [[DBG26:![0-9]+]]
// DEBUG1: terminate.lpad:
// DEBUG1-NEXT: [[TMP2:%.*]] = landingpad { ptr, i32 }
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 66ab2fa5610f5..d732117a36a95 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -225,6 +225,8 @@ class IRBuilderBase {
InsertPt = IP;
if (IP != TheBB->end())
SetCurrentDebugLocation(IP->getStableDebugLoc());
+ else if (!BB->empty() && BB->back().getStableDebugLoc())
+ SetCurrentDebugLocation(BB->back().getStableDebugLoc());
}
/// This specifies that created instructions should be inserted at
diff --git a/llvm/unittests/IR/IRBuilderTest.cpp b/llvm/unittests/IR/IRBuilderTest.cpp
index 520735dfc3268..1922ca85b380e 100644
--- a/llvm/unittests/IR/IRBuilderTest.cpp
+++ b/llvm/unittests/IR/IRBuilderTest.cpp
@@ -1189,6 +1189,39 @@ TEST_F(IRBuilderTest, DebugLoc) {
DIB.finalize();
}
+TEST_F(IRBuilderTest, RestoreDebugLocation) {
+ DIBuilder DIB(*M);
+ auto File = DIB.createFile("tmp.cpp", "/");
+ auto CU =
+ DIB.createCompileUnit(dwarf::DW_LANG_C_plus_plus_11,
+ DIB.createFile("tmp.cpp", "/"), "", true, "", 0);
+ auto SPType = DIB.createSubroutineType(DIB.getOrCreateTypeArray({}));
+ auto SP =
+ DIB.createFunction(CU, "foo", "foo", File, 1, SPType, 1, DINode::FlagZero,
+ DISubprogram::SPFlagDefinition);
+ DebugLoc DL1 = DILocation::get(Ctx, 2, 0, SP);
+ DebugLoc DL2 = DILocation::get(Ctx, 3, 0, SP);
+
+ IRBuilder<> Builder(Ctx);
+ auto BB1 = BasicBlock::Create(Ctx, "bb1", F);
+ Builder.SetInsertPoint(BB1);
+ Builder.SetCurrentDebugLocation(DL1);
+ Builder.CreateAlloca(Builder.getInt8Ty());
+ Builder.CreateAlloca(Builder.getInt32Ty());
+ EXPECT_EQ(DL1, Builder.getCurrentDebugLocation());
+ auto IP = Builder.saveIP();
+
+ auto BB2 = BasicBlock::Create(Ctx, "bb2", F);
+ Builder.SetInsertPoint(BB2);
+ Builder.SetCurrentDebugLocation(DL2);
+ Builder.CreateAlloca(Builder.getInt32Ty());
+ EXPECT_EQ(DL2, Builder.getCurrentDebugLocation());
+
+ Builder.restoreIP(IP);
+ EXPECT_EQ(DL1, Builder.getCurrentDebugLocation());
+ DIB.finalize();
+}
+
TEST_F(IRBuilderTest, DIImportedEntity) {
IRBuilder<> Builder(BB);
DIBuilder DIB(*M);
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, in
SetInsertPoint(BasicBlock *TheBB, BasicBlock::iterator IP)if theIPis pointing to the end of theTheBB, the debugLoc is not changed although the insertion point now points to a different location. This has been source of many problems in theOMPIRBuilder.In
OMPIRBuilder, the following code pattern is quite common.An example can be seen here. If the
OldIPis pointing to the end of theBasicBlock,SetInsertPointwill not callSetCurrentDebugLocationand we have a mismatch between InsertPoint and the DebugLoc. This causes many subtle bugs like 147063.I fixed it by by using the debug location of the last instruction in the
BasicBlockif theInsertPointis at the end. Some clang OpenMP tests needed minor adjustments to work with this change.Fixes #147063.