[llvm] 5f7acf7 - [flang][OMPIRbuilder] Set debug loc on terminator created by splitBB. (#125897)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 5 14:35:48 PST 2025
Author: Abid Qadeer
Date: 2025-02-05T22:35:43Z
New Revision: 5f7acf7259ec693cf03d6dcc75d9b0ef1a4b4e81
URL: https://github.com/llvm/llvm-project/commit/5f7acf7259ec693cf03d6dcc75d9b0ef1a4b4e81
DIFF: https://github.com/llvm/llvm-project/commit/5f7acf7259ec693cf03d6dcc75d9b0ef1a4b4e81.diff
LOG: [flang][OMPIRbuilder] Set debug loc on terminator created by splitBB. (#125897)
Fixes #125088.
When splitBB is called with createBranch=true, it creates a branch
instruction in the old block. But no debug loc is set on that branch
instruction. If that is used as InsertPoint in the restoreIP, it has the
potential to set the current debug location to null and subsequent
instruction will come out without a debug location. This caused the
verification check to fail as shown in the bug report.
This PR changes splitBB and spliceBB function to also take a debugLoc
parameter which can be used to set the debug location of the branch
instruction.
Added:
mlir/test/Target/LLVMIR/omptarget-debug-nowait.mlir
Modified:
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 9802cbe8b7b9438..d25077cae63e42b 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -40,9 +40,10 @@ class OpenMPIRBuilder;
/// not have any PHINodes. If \p CreateBranch is true, a branch instruction to
/// \p New will be added such that there is no semantic change. Otherwise, the
/// \p IP insert block remains degenerate and it is up to the caller to insert a
-/// terminator.
-void spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New,
- bool CreateBranch);
+/// terminator. \p DL is used as the debug location for the branch instruction
+/// if one is created.
+void spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New, bool CreateBranch,
+ DebugLoc DL);
/// Splice a BasicBlock at an IRBuilder's current insertion point. Its new
/// insert location will stick to after the instruction before the insertion
@@ -58,9 +59,10 @@ void spliceBB(IRBuilder<> &Builder, BasicBlock *New, bool CreateBranch);
/// is true, a branch to the new successor will new created such that
/// semantically there is no change; otherwise the block of the insertion point
/// remains degenerate and it is the caller's responsibility to insert a
-/// terminator. Returns the new successor block.
+/// terminator. \p DL is used as the debug location for the branch instruction
+/// if one is created. Returns the new successor block.
BasicBlock *splitBB(IRBuilderBase::InsertPoint IP, bool CreateBranch,
- llvm::Twine Name = {});
+ DebugLoc DL, llvm::Twine Name = {});
/// Split a BasicBlock at \p Builder's insertion point, even if the block is
/// degenerate (missing the terminator). Its new insert location will stick to
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 695b15ac31f380e..490a012f696034a 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -313,7 +313,7 @@ static void redirectTo(BasicBlock *Source, BasicBlock *Target, DebugLoc DL) {
}
void llvm::spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New,
- bool CreateBranch) {
+ bool CreateBranch, DebugLoc DL) {
assert(New->getFirstInsertionPt() == New->begin() &&
"Target BB must not have PHI nodes");
@@ -321,15 +321,17 @@ void llvm::spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New,
BasicBlock *Old = IP.getBlock();
New->splice(New->begin(), Old, IP.getPoint(), Old->end());
- if (CreateBranch)
- BranchInst::Create(New, Old);
+ if (CreateBranch) {
+ auto *NewBr = BranchInst::Create(New, Old);
+ NewBr->setDebugLoc(DL);
+ }
}
void llvm::spliceBB(IRBuilder<> &Builder, BasicBlock *New, bool CreateBranch) {
DebugLoc DebugLoc = Builder.getCurrentDebugLocation();
BasicBlock *Old = Builder.GetInsertBlock();
- spliceBB(Builder.saveIP(), New, CreateBranch);
+ spliceBB(Builder.saveIP(), New, CreateBranch, DebugLoc);
if (CreateBranch)
Builder.SetInsertPoint(Old->getTerminator());
else
@@ -341,12 +343,12 @@ void llvm::spliceBB(IRBuilder<> &Builder, BasicBlock *New, bool CreateBranch) {
}
BasicBlock *llvm::splitBB(IRBuilderBase::InsertPoint IP, bool CreateBranch,
- llvm::Twine Name) {
+ DebugLoc DL, llvm::Twine Name) {
BasicBlock *Old = IP.getBlock();
BasicBlock *New = BasicBlock::Create(
Old->getContext(), Name.isTriviallyEmpty() ? Old->getName() : Name,
Old->getParent(), Old->getNextNode());
- spliceBB(IP, New, CreateBranch);
+ spliceBB(IP, New, CreateBranch, DL);
New->replaceSuccessorsPhiUsesWith(Old, New);
return New;
}
@@ -354,7 +356,7 @@ BasicBlock *llvm::splitBB(IRBuilderBase::InsertPoint IP, bool CreateBranch,
BasicBlock *llvm::splitBB(IRBuilderBase &Builder, bool CreateBranch,
llvm::Twine Name) {
DebugLoc DebugLoc = Builder.getCurrentDebugLocation();
- BasicBlock *New = splitBB(Builder.saveIP(), CreateBranch, Name);
+ BasicBlock *New = splitBB(Builder.saveIP(), CreateBranch, DebugLoc, Name);
if (CreateBranch)
Builder.SetInsertPoint(Builder.GetInsertBlock()->getTerminator());
else
@@ -368,7 +370,7 @@ BasicBlock *llvm::splitBB(IRBuilderBase &Builder, bool CreateBranch,
BasicBlock *llvm::splitBB(IRBuilder<> &Builder, bool CreateBranch,
llvm::Twine Name) {
DebugLoc DebugLoc = Builder.getCurrentDebugLocation();
- BasicBlock *New = splitBB(Builder.saveIP(), CreateBranch, Name);
+ BasicBlock *New = splitBB(Builder.saveIP(), CreateBranch, DebugLoc, Name);
if (CreateBranch)
Builder.SetInsertPoint(Builder.GetInsertBlock()->getTerminator());
else
@@ -5303,7 +5305,7 @@ void OpenMPIRBuilder::createIfVersion(CanonicalLoopInfo *CanonicalLoop,
Builder.CreateCondBr(IfCond, ThenBlock, /*ifFalse*/ ElseBlock);
InsertPointTy IP{BrInstr->getParent(), ++BrInstr->getIterator()};
// Then block contains branch to omp loop which needs to be vectorized
- spliceBB(IP, ThenBlock, false);
+ spliceBB(IP, ThenBlock, false, Builder.getCurrentDebugLocation());
ThenBlock->replaceSuccessorsPhiUsesWith(Head, ThenBlock);
Builder.SetInsertPoint(ElseBlock);
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 9e0a338843d6603..83c8f7e932b2b60 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -7653,4 +7653,20 @@ TEST_F(OpenMPIRBuilderTest, createGPUOffloadEntry) {
EXPECT_TRUE(Fn->hasFnAttribute(Attribute::MustProgress));
}
+TEST_F(OpenMPIRBuilderTest, splitBB) {
+ OpenMPIRBuilder OMPBuilder(*M);
+ OMPBuilder.Config.IsTargetDevice = false;
+ OMPBuilder.initialize();
+ F->setName("func");
+ IRBuilder<> Builder(BB);
+
+ Builder.SetCurrentDebugLocation(DL);
+ AllocaInst *alloc = Builder.CreateAlloca(Builder.getInt32Ty());
+ EXPECT_TRUE(DL == alloc->getStableDebugLoc());
+ BasicBlock *AllocaBB = Builder.GetInsertBlock();
+ splitBB(Builder, /*CreateBranch=*/true, "test");
+ if (AllocaBB->getTerminator())
+ EXPECT_TRUE(DL == AllocaBB->getTerminator()->getStableDebugLoc());
+}
+
} // namespace
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 9c16388e3e348ff..48e4077ec2af64b 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1392,7 +1392,8 @@ static llvm::Expected<llvm::BasicBlock *> allocateAndInitPrivateVars(
llvm::cast<llvm::BranchInst>(allocaIP.getBlock()->getTerminator());
splitBB(llvm::OpenMPIRBuilder::InsertPointTy(allocaIP.getBlock(),
allocaTerminator->getIterator()),
- true, "omp.region.after_alloca");
+ true, allocaTerminator->getStableDebugLoc(),
+ "omp.region.after_alloca");
llvm::IRBuilderBase::InsertPointGuard guard(builder);
// Update the allocaTerminator in case the alloca block was split above.
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug-nowait.mlir b/mlir/test/Target/LLVMIR/omptarget-debug-nowait.mlir
new file mode 100644
index 000000000000000..eaa88d9dd60535e
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug-nowait.mlir
@@ -0,0 +1,40 @@
+// RUN: mlir-translate -mlir-to-llvmir %s
+
+module attributes {omp.is_target_device = false} {
+ llvm.func @main() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x f32 : (i64) -> !llvm.ptr
+ %3 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr
+ %6 = omp.map.info var_ptr(%1 : !llvm.ptr, f32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr
+ %7 = omp.map.info var_ptr(%3 : !llvm.ptr, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !llvm.ptr
+ omp.target nowait map_entries(%6 -> %arg0, %7 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+ %8 = llvm.mlir.constant(0 : i64) : i64
+ %9 = llvm.mlir.constant(100 : i32) : i32
+ llvm.br ^bb1(%9, %8 : i32, i64)
+ ^bb1(%13: i32, %14: i64): // 2 preds: ^bb0, ^bb2
+ %15 = llvm.icmp "sgt" %14, %8 : i64
+ llvm.cond_br %15, ^bb2, ^bb3
+ ^bb2: // pred: ^bb1
+ llvm.store %13, %arg1 : i32, !llvm.ptr
+ llvm.br ^bb1(%13, %14 : i32, i64)
+ ^bb3: // pred: ^bb1
+ llvm.store %13, %arg1 : i32, !llvm.ptr
+ omp.terminator
+ }
+ llvm.return
+ } loc(#loc2)
+}
+
+#file = #llvm.di_file<"test.f90" in "">
+#di_null_type = #llvm.di_null_type
+#cu = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+ emissionKind = Full>
+#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_program,
+ types = #di_null_type>
+#sp = #llvm.di_subprogram<compileUnit = #cu, name = "main", file=#file,
+ subprogramFlags = "Definition", type = #sp_ty>
+
+#loc1 = loc("test.f90":6:7)
+#loc2 = loc(fused<#sp>[#loc1])
+
More information about the llvm-commits
mailing list