[mlir][OpenMP] Fix broken insertion point for charbox with omp task #143112

tblah · 2025-06-06T11:20:28Z

Fixes #142365

Fixes llvm#142365

llvmbot · 2025-06-06T11:21:04Z

@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-mlir-llvm

Author: Tom Eccles (tblah)

Changes

Fixes #142365

Full diff: https://github.com/llvm/llvm-project/pull/143112.diff

2 Files Affected:

(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+1-2)
(added) mlir/test/Target/LLVMIR/openmp-task-charbox.mlir (+87)

diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index ec1a9c909c94a..069a5cbb9112d 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2293,8 +2293,7 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
     if (!privateVarOrErr)
       return handleError(privateVarOrErr, *taskOp.getOperation());
 
-    llvm::IRBuilderBase::InsertPointGuard guard(builder);
-    builder.SetInsertPoint(builder.GetInsertBlock()->getTerminator());
+    setInsertPointForPossiblyEmptyBlock(builder);
 
     // TODO: this is a bit of a hack for Fortran character boxes.
     // Character boxes are passed by value into the init region and then the
diff --git a/mlir/test/Target/LLVMIR/openmp-task-charbox.mlir b/mlir/test/Target/LLVMIR/openmp-task-charbox.mlir
new file mode 100644
index 0000000000000..7a448f74ed648
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/openmp-task-charbox.mlir
@@ -0,0 +1,87 @@
+// RUN: mlir-translate --mlir-to-llvmir %s | FileCheck %s
+
+// Regression test for a compiler crash. Ensure that the insertion point is set
+// correctly when triggering the charbox hack multiple times.
+// Nonsense test code to minimally reproduce the issue.
+
+module {
+  llvm.func @free(!llvm.ptr)
+  llvm.func @malloc(i64) -> !llvm.ptr
+  omp.private {type = private} @_QFEc2_private_box_heap_c8xU : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> init {
+  ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+    %0 = llvm.mlir.constant(24 : i32) : i32
+    %1 = llvm.mlir.constant(0 : i64) : i64
+    %2 = llvm.mlir.constant(1 : i32) : i32
+    %3 = llvm.alloca %2 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {alignment = 8 : i64} : (i32) -> !llvm.ptr
+    "llvm.intr.memcpy"(%3, %arg0, %0) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i32) -> ()
+    %6 = llvm.ptrtoint %arg0 : !llvm.ptr to i64
+    %7 = llvm.icmp "eq" %6, %1 : i64
+    llvm.cond_br %7, ^bb1, ^bb2
+  ^bb1:  // pred: ^bb0
+    llvm.br ^bb3
+  ^bb2:  // pred: ^bb0
+    llvm.br ^bb3
+  ^bb3:  // 2 preds: ^bb1, ^bb2
+    omp.yield(%arg1 : !llvm.ptr)
+  } dealloc {
+  ^bb0(%arg0: !llvm.ptr):
+    omp.yield
+  }
+  omp.private {type = private} @_QFEc1_private_box_ptr_c8xU : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> init {
+  ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+    %0 = llvm.mlir.constant(24 : i32) : i32
+    %1 = llvm.mlir.constant(1 : i32) : i32
+    %2 = llvm.alloca %1 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {alignment = 8 : i64} : (i32) -> !llvm.ptr
+    "llvm.intr.memcpy"(%2, %arg0, %0) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i32) -> ()
+    omp.yield(%arg1 : !llvm.ptr)
+  }
+  llvm.func @_QQmain() {
+    %0 = llvm.mlir.constant(1 : i64) : i64
+    %1 = llvm.alloca %0 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {bindc_name = "c2"} : (i64) -> !llvm.ptr
+    %2 = llvm.alloca %0 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {bindc_name = "c1"} : (i64) -> !llvm.ptr
+    omp.task private(@_QFEc1_private_box_ptr_c8xU %2 -> %arg0, @_QFEc2_private_box_heap_c8xU %1 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+      omp.terminator
+    }
+    llvm.return
+  }
+}
+
+// CHECK-LABEL: @_QQmain() {
+// CHECK:         %[[STRUCTARG:.*]] = alloca { ptr }, align 8
+// CHECK:         %[[VAL_0:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8 }, i64 1, align 8
+// CHECK:         br label %[[VAL_2:.*]]
+// CHECK:       entry:                                            ; preds = %[[VAL_3:.*]]
+// CHECK:         br label %[[VAL_4:.*]]
+// CHECK:       omp.private.init:                                 ; preds = %[[VAL_2]]
+// CHECK:         %[[VAL_5:.*]] = tail call ptr @malloc(i64 ptrtoint (ptr getelementptr ({ { ptr, i64, i32, i8, i8, i8, i8 }, { ptr, i64, i32, i8, i8, i8, i8 } }, ptr null, i32 1) to i64))
+// CHECK:         %[[VAL_6:.*]] = getelementptr { { ptr, i64, i32, i8, i8, i8, i8 }, { ptr, i64, i32, i8, i8, i8, i8 } }, ptr %[[VAL_5]], i32 0, i32 0
+// CHECK:         %[[VAL_7:.*]] = getelementptr { { ptr, i64, i32, i8, i8, i8, i8 }, { ptr, i64, i32, i8, i8, i8, i8 } }, ptr %[[VAL_5]], i32 0, i32 1
+// ...
+// CHECK:         br label %[[VAL_9:.*]]
+// CHECK:       omp.private.init4:                                ; preds = %[[VAL_10:.*]], %[[VAL_11:.*]]
+// CHECK:         br label %[[VAL_12:.*]]
+// CHECK:       omp.private.init3:                                ; preds = %[[VAL_9]]
+// CHECK:         br label %[[VAL_13:.*]]
+// CHECK:       omp.private.init2:                                ; preds = %[[VAL_9]]
+// CHECK:         br label %[[VAL_13]]
+// CHECK:       omp.private.init1:                                ; preds = %[[VAL_4]]
+// CHECK:         %[[VAL_14:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8 }, align 8
+// CHECK:         call void @llvm.memcpy.p0.p0.i32(ptr %[[VAL_14]], ptr %[[VAL_0]], i32 24, i1 false)
+// CHECK:         %[[VAL_15:.*]] = ptrtoint ptr %[[VAL_0]] to i64
+// CHECK:         %[[VAL_16:.*]] = icmp eq i64 %[[VAL_15]], 0
+// CHECK:         br i1 %[[VAL_16]], label %[[VAL_10]], label %[[VAL_11]]
+// CHECK:       omp.region.cont:                                  ; preds = %[[VAL_13]]
+// CHECK:         %[[VAL_17:.*]] = phi ptr [ %[[VAL_7]], %[[VAL_13]] ]
+// CHECK:         br label %[[VAL_18:.*]]
+// CHECK:       omp.private.copy:                                 ; preds = %[[VAL_12]]
+// CHECK:         br label %[[VAL_19:.*]]
+// CHECK:       omp.task.start:                                   ; preds = %[[VAL_18]]
+// CHECK:         br label %[[VAL_20:.*]]
+// CHECK:       codeRepl:                                         ; preds = %[[VAL_19]]
+// CHECK:         %[[VAL_21:.*]] = getelementptr { ptr }, ptr %[[STRUCTARG]], i32 0, i32 0
+// CHECK:         store ptr %[[VAL_5]], ptr %[[VAL_21]], align 8
+// CHECK:         %[[VAL_22:.*]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK:         %[[VAL_23:.*]] = call ptr @__kmpc_omp_task_alloc(ptr @1, i32 %[[VAL_22]], i32 1, i64 40, i64 8, ptr @_QQmain..omp_par)
+// CHECK:         %[[VAL_24:.*]] = load ptr, ptr %[[VAL_23]], align 8
+// CHECK:         call void @llvm.memcpy.p0.p0.i64(ptr align 1 %[[VAL_24]], ptr align 1 %[[STRUCTARG]], i64 8, i1 false)
+// CHECK:         %[[VAL_25:.*]] = call i32 @__kmpc_omp_task(ptr @1, i32 %[[VAL_22]], ptr %[[VAL_23]])

llvmbot · 2025-06-06T11:21:04Z

@llvm/pr-subscribers-mlir

Author: Tom Eccles (tblah)

Changes

Fixes #142365

Full diff: https://github.com/llvm/llvm-project/pull/143112.diff

2 Files Affected:

(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+1-2)
(added) mlir/test/Target/LLVMIR/openmp-task-charbox.mlir (+87)

diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index ec1a9c909c94a..069a5cbb9112d 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2293,8 +2293,7 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
     if (!privateVarOrErr)
       return handleError(privateVarOrErr, *taskOp.getOperation());
 
-    llvm::IRBuilderBase::InsertPointGuard guard(builder);
-    builder.SetInsertPoint(builder.GetInsertBlock()->getTerminator());
+    setInsertPointForPossiblyEmptyBlock(builder);
 
     // TODO: this is a bit of a hack for Fortran character boxes.
     // Character boxes are passed by value into the init region and then the
diff --git a/mlir/test/Target/LLVMIR/openmp-task-charbox.mlir b/mlir/test/Target/LLVMIR/openmp-task-charbox.mlir
new file mode 100644
index 0000000000000..7a448f74ed648
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/openmp-task-charbox.mlir
@@ -0,0 +1,87 @@
+// RUN: mlir-translate --mlir-to-llvmir %s | FileCheck %s
+
+// Regression test for a compiler crash. Ensure that the insertion point is set
+// correctly when triggering the charbox hack multiple times.
+// Nonsense test code to minimally reproduce the issue.
+
+module {
+  llvm.func @free(!llvm.ptr)
+  llvm.func @malloc(i64) -> !llvm.ptr
+  omp.private {type = private} @_QFEc2_private_box_heap_c8xU : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> init {
+  ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+    %0 = llvm.mlir.constant(24 : i32) : i32
+    %1 = llvm.mlir.constant(0 : i64) : i64
+    %2 = llvm.mlir.constant(1 : i32) : i32
+    %3 = llvm.alloca %2 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {alignment = 8 : i64} : (i32) -> !llvm.ptr
+    "llvm.intr.memcpy"(%3, %arg0, %0) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i32) -> ()
+    %6 = llvm.ptrtoint %arg0 : !llvm.ptr to i64
+    %7 = llvm.icmp "eq" %6, %1 : i64
+    llvm.cond_br %7, ^bb1, ^bb2
+  ^bb1:  // pred: ^bb0
+    llvm.br ^bb3
+  ^bb2:  // pred: ^bb0
+    llvm.br ^bb3
+  ^bb3:  // 2 preds: ^bb1, ^bb2
+    omp.yield(%arg1 : !llvm.ptr)
+  } dealloc {
+  ^bb0(%arg0: !llvm.ptr):
+    omp.yield
+  }
+  omp.private {type = private} @_QFEc1_private_box_ptr_c8xU : !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> init {
+  ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+    %0 = llvm.mlir.constant(24 : i32) : i32
+    %1 = llvm.mlir.constant(1 : i32) : i32
+    %2 = llvm.alloca %1 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {alignment = 8 : i64} : (i32) -> !llvm.ptr
+    "llvm.intr.memcpy"(%2, %arg0, %0) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i32) -> ()
+    omp.yield(%arg1 : !llvm.ptr)
+  }
+  llvm.func @_QQmain() {
+    %0 = llvm.mlir.constant(1 : i64) : i64
+    %1 = llvm.alloca %0 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {bindc_name = "c2"} : (i64) -> !llvm.ptr
+    %2 = llvm.alloca %0 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)> {bindc_name = "c1"} : (i64) -> !llvm.ptr
+    omp.task private(@_QFEc1_private_box_ptr_c8xU %2 -> %arg0, @_QFEc2_private_box_heap_c8xU %1 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+      omp.terminator
+    }
+    llvm.return
+  }
+}
+
+// CHECK-LABEL: @_QQmain() {
+// CHECK:         %[[STRUCTARG:.*]] = alloca { ptr }, align 8
+// CHECK:         %[[VAL_0:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8 }, i64 1, align 8
+// CHECK:         br label %[[VAL_2:.*]]
+// CHECK:       entry:                                            ; preds = %[[VAL_3:.*]]
+// CHECK:         br label %[[VAL_4:.*]]
+// CHECK:       omp.private.init:                                 ; preds = %[[VAL_2]]
+// CHECK:         %[[VAL_5:.*]] = tail call ptr @malloc(i64 ptrtoint (ptr getelementptr ({ { ptr, i64, i32, i8, i8, i8, i8 }, { ptr, i64, i32, i8, i8, i8, i8 } }, ptr null, i32 1) to i64))
+// CHECK:         %[[VAL_6:.*]] = getelementptr { { ptr, i64, i32, i8, i8, i8, i8 }, { ptr, i64, i32, i8, i8, i8, i8 } }, ptr %[[VAL_5]], i32 0, i32 0
+// CHECK:         %[[VAL_7:.*]] = getelementptr { { ptr, i64, i32, i8, i8, i8, i8 }, { ptr, i64, i32, i8, i8, i8, i8 } }, ptr %[[VAL_5]], i32 0, i32 1
+// ...
+// CHECK:         br label %[[VAL_9:.*]]
+// CHECK:       omp.private.init4:                                ; preds = %[[VAL_10:.*]], %[[VAL_11:.*]]
+// CHECK:         br label %[[VAL_12:.*]]
+// CHECK:       omp.private.init3:                                ; preds = %[[VAL_9]]
+// CHECK:         br label %[[VAL_13:.*]]
+// CHECK:       omp.private.init2:                                ; preds = %[[VAL_9]]
+// CHECK:         br label %[[VAL_13]]
+// CHECK:       omp.private.init1:                                ; preds = %[[VAL_4]]
+// CHECK:         %[[VAL_14:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8 }, align 8
+// CHECK:         call void @llvm.memcpy.p0.p0.i32(ptr %[[VAL_14]], ptr %[[VAL_0]], i32 24, i1 false)
+// CHECK:         %[[VAL_15:.*]] = ptrtoint ptr %[[VAL_0]] to i64
+// CHECK:         %[[VAL_16:.*]] = icmp eq i64 %[[VAL_15]], 0
+// CHECK:         br i1 %[[VAL_16]], label %[[VAL_10]], label %[[VAL_11]]
+// CHECK:       omp.region.cont:                                  ; preds = %[[VAL_13]]
+// CHECK:         %[[VAL_17:.*]] = phi ptr [ %[[VAL_7]], %[[VAL_13]] ]
+// CHECK:         br label %[[VAL_18:.*]]
+// CHECK:       omp.private.copy:                                 ; preds = %[[VAL_12]]
+// CHECK:         br label %[[VAL_19:.*]]
+// CHECK:       omp.task.start:                                   ; preds = %[[VAL_18]]
+// CHECK:         br label %[[VAL_20:.*]]
+// CHECK:       codeRepl:                                         ; preds = %[[VAL_19]]
+// CHECK:         %[[VAL_21:.*]] = getelementptr { ptr }, ptr %[[STRUCTARG]], i32 0, i32 0
+// CHECK:         store ptr %[[VAL_5]], ptr %[[VAL_21]], align 8
+// CHECK:         %[[VAL_22:.*]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK:         %[[VAL_23:.*]] = call ptr @__kmpc_omp_task_alloc(ptr @1, i32 %[[VAL_22]], i32 1, i64 40, i64 8, ptr @_QQmain..omp_par)
+// CHECK:         %[[VAL_24:.*]] = load ptr, ptr %[[VAL_23]], align 8
+// CHECK:         call void @llvm.memcpy.p0.p0.i64(ptr align 1 %[[VAL_24]], ptr align 1 %[[STRUCTARG]], i64 8, i1 false)
+// CHECK:         %[[VAL_25:.*]] = call i32 @__kmpc_omp_task(ptr @1, i32 %[[VAL_22]], ptr %[[VAL_23]])

tblah · 2025-06-11T09:20:06Z

Ping for review

ergawy

LGTM! Thanks for the fix.

…lvm#143112) Fixes llvm#142365

[mlir][OpenMP] Fix broken insertion point for charbox with omp task

0e17826

Fixes llvm#142365

tblah requested review from ergawy, Meinersbur, skatrak and bhandarkar-pranav June 6, 2025 11:20

llvmbot added mlir:llvm mlir mlir:openmp flang:openmp labels Jun 6, 2025

tblah mentioned this pull request Jun 6, 2025

[Flang][OpenMP] Important issues to be fixed prior to removing experimental status of OpenMP #110008

Open

92 tasks

ergawy approved these changes Jun 17, 2025

View reviewed changes

tblah merged commit aa01e8e into llvm:main Jun 17, 2025
12 checks passed

ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 17, 2025

[mlir][OpenMP] Fix broken insertion point for charbox with omp task (l…

05fd4c5

…lvm#143112) Fixes llvm#142365

fschlimb pushed a commit to fschlimb/llvm-project that referenced this pull request Jun 18, 2025

[mlir][OpenMP] Fix broken insertion point for charbox with omp task (l…

a27ca83

…lvm#143112) Fixes llvm#142365

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir][OpenMP] Fix broken insertion point for charbox with omp task #143112

[mlir][OpenMP] Fix broken insertion point for charbox with omp task #143112

Uh oh!

tblah commented Jun 6, 2025

Uh oh!

llvmbot commented Jun 6, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jun 6, 2025

Uh oh!

tblah commented Jun 11, 2025

Uh oh!

ergawy left a comment

Uh oh!

Uh oh!

Uh oh!

[mlir][OpenMP] Fix broken insertion point for charbox with omp task #143112

[mlir][OpenMP] Fix broken insertion point for charbox with omp task #143112

Uh oh!

Conversation

tblah commented Jun 6, 2025

Uh oh!

llvmbot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 6, 2025

Uh oh!

tblah commented Jun 11, 2025

Uh oh!

ergawy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jun 6, 2025 •

edited

Loading