llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Abid Qadeer (abidh) <details> <summary>Changes</summary> 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](https://github.com/llvm/llvm-project/blob/b0473c599b0418c71d15150e0ea19d57df3b98e5/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L1869). If the `OldIP` is pointing to the end of the `BasicBlock`, `SetInsertPoint` will not call `SetCurrentDebugLocation` and we have a mismatch between InsertPoint and the DebugLoc. This causes many subtle bugs like [147063](https://github.com/llvm/llvm-project/issues/147063). 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. --- Full diff: https://github.com/llvm/llvm-project/pull/147091.diff 6 Files Affected: - (modified) clang/test/OpenMP/parallel_codegen.cpp (+5-5) - (modified) clang/test/OpenMP/parallel_for_codegen.cpp (+2-2) - (modified) clang/test/OpenMP/scope_codegen.cpp (+2-2) - (modified) clang/test/OpenMP/taskgroup_codegen.cpp (+4-4) - (modified) llvm/include/llvm/IR/IRBuilder.h (+2) - (modified) llvm/unittests/IR/IRBuilderTest.cpp (+33) ``````````diff 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); `````````` </details> https://github.com/llvm/llvm-project/pull/147091 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
