[flang-commits] [flang] [llvm] [mlir] [mlir][OpenMP] fix crash outlining infinite loop (PR #129872)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Wed Mar 5 03:28:24 PST 2025
https://github.com/tblah created https://github.com/llvm/llvm-project/pull/129872
Previously an extra block was created by splitting the previous exit block. This produced incorrect results when the outlined region statically never terminated because then there wouldn't be a valid exit block for the outlined region, this caused this newly added block to have an incoming edge from outside of the outlining region, which caused outlining to fail.
So far as I can tell this extra block no longer serves any purpose. The comment says it is supposed to collate multiple control flow edges into one place, but the code as it is now does not achieve this. In fact, as can be seen from the changes to lit tests, this block was not actually outlined in the end. This is because there are actually two code extractors: one in the callback for creating a parallel op which is used to find what the input/output variables are (which does have this block added to it), and another one which actually does the outlining (which this block was not added to).
Tested with the gfortran and fujitsu test suites.
Fixes #112884
>From 3055b89b8ab937bfd4ba70d87e1733b05156d0ac Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Tue, 4 Mar 2025 17:19:53 +0000
Subject: [PATCH] [mlir][OpenMP] fix crash outlining infinite loop
Previously an extra block was created by splitting the previous exit
block. This produced incorrect results when the outlined region
statically never terminated because then there wouldn't be a valid exit
block for the outlined region, this caused this newly added block to
have an incoming edge from outside of the outlining region, which caused
outlining to fail.
So far as I can tell this extra block no longer serves any purpose. The
comment says it is supposed to collate multiple control flow edges into
one place, but the code as it is now does not achieve this. In fact, as
can be seen from the changes to lit tests, this block was not actually
outlined in the end. This is because there are actually two code
extractors: one in the callback for creating a parallel op which is used
to find what the input/output variables are (which does have this block
added to it), and another one which actually does the outlining (which
this block was not added to).
Tested with the gfortran and fujitsu test suites.
---
.../parallel-private-reduction-worstcase.f90 | 2 +-
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 8 ----
mlir/test/Target/LLVMIR/openmp-llvm.mlir | 4 +-
.../LLVMIR/openmp-outline-infinite-loop.mlir | 44 +++++++++++++++++++
.../openmp-parallel-reduction-multiblock.mlir | 6 +--
.../openmp-reduction-array-sections.mlir | 2 +-
.../LLVMIR/openmp-reduction-init-arg.mlir | 6 +--
.../LLVMIR/openmp-reduction-sections.mlir | 2 +-
8 files changed, 52 insertions(+), 22 deletions(-)
create mode 100644 mlir/test/Target/LLVMIR/openmp-outline-infinite-loop.mlir
diff --git a/flang/test/Integration/OpenMP/parallel-private-reduction-worstcase.f90 b/flang/test/Integration/OpenMP/parallel-private-reduction-worstcase.f90
index 7e735b6499504..cf77c46346b7f 100644
--- a/flang/test/Integration/OpenMP/parallel-private-reduction-worstcase.f90
+++ b/flang/test/Integration/OpenMP/parallel-private-reduction-worstcase.f90
@@ -219,5 +219,5 @@ subroutine worst_case(a, b, c, d)
! [var extent was non-zero: malloc a private array]
! CHECK: br label %omp.private.init5
-! CHECK: omp.par.outlined.exit.exitStub: ; preds = %omp.region.cont52
+! CHECK: omp.par.exit.exitStub: ; preds = %omp.region.cont52
! CHECK-NEXT: ret void
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index e34e93442ff85..0295319ec1e9d 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1602,14 +1602,6 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createParallel(
SmallVector<BasicBlock *, 32> Blocks;
OI.collectBlocks(ParallelRegionBlockSet, Blocks);
- // Ensure a single exit node for the outlined region by creating one.
- // We might have multiple incoming edges to the exit now due to finalizations,
- // e.g., cancel calls that cause the control flow to leave the region.
- BasicBlock *PRegOutlinedExitBB = PRegExitBB;
- PRegExitBB = SplitBlock(PRegExitBB, &*PRegExitBB->getFirstInsertionPt());
- PRegOutlinedExitBB->setName("omp.par.outlined.exit");
- Blocks.push_back(PRegOutlinedExitBB);
-
CodeExtractorAnalysisCache CEAC(*OuterFn);
CodeExtractor Extractor(Blocks, /* DominatorTree */ nullptr,
/* AggregateArgs */ false,
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index f25ba4aa3c8dc..e4114d491fc85 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -162,14 +162,12 @@ llvm.func @test_omp_parallel_if_1(%arg0: i32) -> () {
// CHECK: %[[I32_IF_COND_VAR_1:.*]] = sext i1 %[[IF_COND_VAR_1]] to i32
// CHECK: call void @__kmpc_fork_call_if(ptr @[[SI_VAR_IF_1]], i32 0, ptr @[[OMP_OUTLINED_FN_IF_1:.*]], i32 %[[I32_IF_COND_VAR_1]], ptr null)
// CHECK: br label %[[OUTLINED_EXIT_IF_1:.*]]
-// CHECK: [[OUTLINED_EXIT_IF_1]]:
-// CHECK: br label %[[RETURN_BLOCK_IF_1:.*]]
omp.parallel if(%1) {
omp.barrier
omp.terminator
}
-// CHECK: [[RETURN_BLOCK_IF_1]]:
+// CHECK: [[OUTLINED_EXIT_IF_1]]:
// CHECK: ret void
llvm.return
}
diff --git a/mlir/test/Target/LLVMIR/openmp-outline-infinite-loop.mlir b/mlir/test/Target/LLVMIR/openmp-outline-infinite-loop.mlir
new file mode 100644
index 0000000000000..faccfc678adfe
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/openmp-outline-infinite-loop.mlir
@@ -0,0 +1,44 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Test that trying to outline an infinite loop doesn't lead to an assertion
+// failure.
+
+llvm.func @parallel_infinite_loop() -> () {
+ omp.parallel {
+ llvm.br ^bb1
+ ^bb1:
+ llvm.br ^bb1
+ }
+ llvm.return
+}
+
+// CHECK-LABEL: define void @parallel_infinite_loop() {
+// CHECK: %[[VAL_2:.*]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK: br label %[[VAL_3:.*]]
+// CHECK: omp_parallel:
+// CHECK: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @1, i32 0, ptr @parallel_infinite_loop..omp_par)
+// CHECK: unreachable
+// CHECK: omp.region.cont: ; No predecessors!
+// CHECK: br label %[[VAL_4:.*]]
+// CHECK: omp.par.pre_finalize: ; preds = %[[VAL_5:.*]]
+// CHECK: br label %[[VAL_6:.*]]
+// CHECK: omp.par.exit: ; preds = %[[VAL_4]]
+// CHECK: ret void
+// CHECK: }
+
+// CHECK-LABEL: define internal void @parallel_infinite_loop..omp_par(
+// CHECK-SAME: ptr noalias %[[TID_ADDR:.*]], ptr noalias %[[ZERO_ADDR:.*]])
+// CHECK: omp.par.entry:
+// CHECK: %[[VAL_7:.*]] = alloca i32, align 4
+// CHECK: %[[VAL_8:.*]] = load i32, ptr %[[VAL_9:.*]], align 4
+// CHECK: store i32 %[[VAL_8]], ptr %[[VAL_7]], align 4
+// CHECK: %[[VAL_10:.*]] = load i32, ptr %[[VAL_7]], align 4
+// CHECK: br label %[[VAL_11:.*]]
+// CHECK: omp.region.after_alloca: ; preds = %[[VAL_12:.*]]
+// CHECK: br label %[[VAL_13:.*]]
+// CHECK: omp.par.region: ; preds = %[[VAL_11]]
+// CHECK: br label %[[VAL_14:.*]]
+// CHECK: omp.par.region1: ; preds = %[[VAL_13]]
+// CHECK: br label %[[VAL_15:.*]]
+// CHECK: omp.par.region2: ; preds = %[[VAL_15]], %[[VAL_14]]
+// CHECK: br label %[[VAL_15]]
diff --git a/mlir/test/Target/LLVMIR/openmp-parallel-reduction-multiblock.mlir b/mlir/test/Target/LLVMIR/openmp-parallel-reduction-multiblock.mlir
index d2e394b2cf6a8..887d2977e45cc 100644
--- a/mlir/test/Target/LLVMIR/openmp-parallel-reduction-multiblock.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-parallel-reduction-multiblock.mlir
@@ -40,9 +40,7 @@ llvm.func @missordered_blocks_(%arg0: !llvm.ptr {fir.bindc_name = "x"}, %arg1: !
// CHECK: store ptr %[[VAL_8:.*]], ptr %[[VAL_7]], align 8
// CHECK: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @1, i32 1, ptr @missordered_blocks_..omp_par, ptr %[[VAL_0]])
// CHECK: br label %[[VAL_9:.*]]
-// CHECK: omp.par.outlined.exit: ; preds = %[[VAL_4]]
-// CHECK: br label %[[VAL_10:.*]]
-// CHECK: omp.par.exit.split: ; preds = %[[VAL_9]]
+// CHECK: omp.par.exit: ; preds = %[[VAL_4]]
// CHECK: ret void
// CHECK: [[PAR_ENTRY:omp.par.entry]]:
// CHECK: %[[VAL_11:.*]] = getelementptr { ptr, ptr }, ptr %[[VAL_12:.*]], i32 0, i32 0
@@ -117,5 +115,5 @@ llvm.func @missordered_blocks_(%arg0: !llvm.ptr {fir.bindc_name = "x"}, %arg1: !
// CHECK: br label %[[VAL_38]]
// CHECK: omp.reduction.neutral1: ; preds = %[[VAL_25]]
// CHECK: br label %[[VAL_30]]
-// CHECK: omp.par.outlined.exit.exitStub: ; preds = %[[VAL_53]]
+// CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_53]]
// CHECK: ret void
diff --git a/mlir/test/Target/LLVMIR/openmp-reduction-array-sections.mlir b/mlir/test/Target/LLVMIR/openmp-reduction-array-sections.mlir
index d6ed3086969fb..b302b4b20edd5 100644
--- a/mlir/test/Target/LLVMIR/openmp-reduction-array-sections.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-reduction-array-sections.mlir
@@ -219,5 +219,5 @@ llvm.func @sectionsreduction_(%arg0: !llvm.ptr {fir.bindc_name = "x"}) attribute
// CHECK: omp_section_loop.inc: ; preds = %[[VAL_69]]
// CHECK: %[[VAL_31]] = add nuw i32 %[[VAL_30]], 1
// CHECK: br label %[[VAL_28]]
-// CHECK: omp.par.outlined.exit.exitStub: ; preds = %[[VAL_64]]
+// CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_64]]
// CHECK: ret void
diff --git a/mlir/test/Target/LLVMIR/openmp-reduction-init-arg.mlir b/mlir/test/Target/LLVMIR/openmp-reduction-init-arg.mlir
index 8d329bd8ff817..a714ca68a1e95 100644
--- a/mlir/test/Target/LLVMIR/openmp-reduction-init-arg.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-reduction-init-arg.mlir
@@ -46,9 +46,7 @@ module {
// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_8]], align 8
// CHECK: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @1, i32 1, ptr @_QFPreduce..omp_par, ptr %[[VAL_0]])
// CHECK: br label %[[VAL_9:.*]]
-// CHECK: omp.par.outlined.exit: ; preds = %[[VAL_6]]
-// CHECK: br label %[[VAL_10:.*]]
-// CHECK: omp.par.exit.split: ; preds = %[[VAL_9]]
+// CHECK: omp.par.exit: ; preds = %[[VAL_6]]
// CHECK: ret void
// CHECK: [[PAR_ENTRY:omp.par.entry]]:
// CHECK: %[[VAL_11:.*]] = getelementptr { ptr, ptr }, ptr %[[VAL_12:.*]], i32 0, i32 0
@@ -99,7 +97,7 @@ module {
// CHECK: br label %[[VAL_38:.*]]
// CHECK: omp.par.pre_finalize: ; preds = %[[VAL_33]]
// CHECK: br label %[[VAL_39:.*]]
-// CHECK: omp.par.outlined.exit.exitStub: ; preds = %[[VAL_38]]
+// CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_38]]
// CHECK: ret void
// CHECK: %[[VAL_40:.*]] = getelementptr inbounds [2 x ptr], ptr %[[VAL_41:.*]], i64 0, i64 0
// CHECK: %[[VAL_42:.*]] = load ptr, ptr %[[VAL_40]], align 8
diff --git a/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir b/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir
index de3b997feb674..19da6f8517fcd 100644
--- a/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir
@@ -144,7 +144,7 @@ llvm.func @sections_(%arg0: !llvm.ptr {fir.bindc_name = "x"}) attributes {fir.in
// CHECK: omp_section_loop.inc: ; preds = %[[VAL_59]]
// CHECK: %[[VAL_35]] = add nuw i32 %[[VAL_34]], 1
// CHECK: br label %[[VAL_32]]
-// CHECK: omp.par.outlined.exit.exitStub: ; preds = %[[VAL_54]]
+// CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_54]]
// CHECK: ret void
// CHECK: %[[VAL_70:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_71:.*]], i64 0, i64 0
// CHECK: %[[VAL_72:.*]] = load ptr, ptr %[[VAL_70]], align 8
More information about the flang-commits
mailing list