Skip to content

Commit f7daa9d

Browse files
tblahtblah
authored
[mlir][OpenMP] fix crash outlining infinite loop (#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
1 parent de9cee1 commit f7daa9d

15 files changed

+182
-226
lines changed

‎clang/test/OpenMP/cancel_codegen.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -728,9 +728,7 @@ for (int i = 0; i < argc; ++i) {
728728
// CHECK3-NEXT: store ptr [[ARGV_ADDR]], ptr [[GEP_ARGV_ADDR]], align 8
729729
// CHECK3-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @main..omp_par, ptr [[STRUCTARG]])
730730
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
731-
// CHECK3: omp.par.outlined.exit:
732-
// CHECK3-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
733-
// CHECK3: omp.par.exit.split:
731+
// CHECK3: omp.par.exit:
734732
// CHECK3-NEXT: br label [[OMP_SECTION_LOOP_PREHEADER:%.*]]
735733
// CHECK3: omp_section_loop.preheader:
736734
// CHECK3-NEXT: store i32 0, ptr [[P_LOWERBOUND]], align 4
@@ -998,7 +996,7 @@ for (int i = 0; i < argc; ++i) {
998996
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB]]
999997
// CHECK3: .split:
1000998
// CHECK3-NEXT: br label [[TMP4]]
1001-
// CHECK3: omp.par.outlined.exit.exitStub:
999+
// CHECK3: omp.par.exit.exitStub:
10021000
// CHECK3-NEXT: ret void
10031001
//
10041002
//

‎clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
// ALL-NEXT: br label [[OMP_PARALLEL:%.*]]
1717
// ALL: omp_parallel:
1818
// ALL-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 0, ptr @_Z17nested_parallel_0v..omp_par.1)
19-
// ALL-NEXT: br label [[OMP_PAR_OUTLINED_EXIT12:%.*]]
20-
// ALL: omp.par.outlined.exit12:
19+
// ALL-NEXT: br label [[OMP_PAR_EXIT:%.*]]
20+
// ALL: omp.par.exit7:
2121
// ALL-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
22-
// ALL: omp.par.exit.split:
22+
// ALL: omp.par.exit.exitStub:
2323
// ALL-NEXT: ret void
2424
//
2525
void nested_parallel_0(void) {
@@ -50,10 +50,8 @@ void nested_parallel_0(void) {
5050
// ALL-NEXT: [[GEP_R_ADDR17:%.*]] = getelementptr { ptr, ptr, ptr }, ptr [[STRUCTARG14]], i32 0, i32 2
5151
// ALL-NEXT: store ptr [[R_ADDR]], ptr [[GEP_R_ADDR17]], align 8
5252
// ALL-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @_Z17nested_parallel_1Pfid..omp_par.2, ptr [[STRUCTARG14]])
53-
// ALL-NEXT: br label [[OMP_PAR_OUTLINED_EXIT13:%.*]]
54-
// ALL: omp.par.outlined.exit13:
55-
// ALL-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
56-
// ALL: omp.par.exit.split:
53+
// ALL-NEXT: br label [[OMP_PAR_EXIT:%.*]]
54+
// ALL: omp.par.exit:
5755
// ALL-NEXT: ret void
5856
//
5957
void nested_parallel_1(float *r, int a, double b) {
@@ -85,10 +83,8 @@ void nested_parallel_1(float *r, int a, double b) {
8583
// ALL-NEXT: [[GEP_R_ADDR:%.*]] = getelementptr { ptr, ptr, ptr }, ptr [[STRUCTARG]], i32 0, i32 2
8684
// ALL-NEXT: store ptr [[R_ADDR]], ptr [[GEP_R_ADDR]], align 8
8785
// ALL-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @_Z17nested_parallel_2Pfid..omp_par.5, ptr [[STRUCTARG]])
88-
// ALL-NEXT: br label [[OMP_PAR_OUTLINED_EXIT55:%.*]]
89-
// ALL: omp.par.outlined.exit55:
90-
// ALL-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
91-
// ALL: omp.par.exit.split:
86+
// ALL-NEXT: br label [[OMP_PAR_EXIT:%.*]]
87+
// ALL: omp.par.exit:
9288
// ALL-NEXT: [[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
9389
// ALL-NEXT: [[CONV56:%.*]] = sitofp i32 [[TMP0]] to double
9490
// ALL-NEXT: [[TMP1:%.*]] = load double, ptr [[B_ADDR]], align 8

‎clang/test/OpenMP/irbuilder_nested_parallel_for.c

Lines changed: 80 additions & 108 deletions
Large diffs are not rendered by default.

‎clang/test/OpenMP/nested_loop_codegen.cpp

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -530,10 +530,8 @@ int inline_decl() {
530530
// CHECK3-NEXT: [[GEP_K:%.*]] = getelementptr { ptr, ptr }, ptr [[STRUCTARG]], i32 0, i32 1
531531
// CHECK3-NEXT: store ptr [[K]], ptr [[GEP_K]], align 8
532532
// CHECK3-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @_Z12outline_declv..omp_par, ptr [[STRUCTARG]])
533-
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
534-
// CHECK3: omp.par.outlined.exit:
535-
// CHECK3-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
536-
// CHECK3: omp.par.exit.split:
533+
// CHECK3-NEXT: br label [[OMP_PAR_EXIT:%.*]]
534+
// CHECK3: omp.par.exit:
537535
// CHECK3-NEXT: [[TMP0:%.*]] = load i32, ptr [[K]], align 4
538536
// CHECK3-NEXT: ret i32 [[TMP0]]
539537
//
@@ -620,7 +618,7 @@ int inline_decl() {
620618
// CHECK3: omp_loop.inc:
621619
// CHECK3-NEXT: [[OMP_LOOP_NEXT]] = add nuw i32 [[OMP_LOOP_IV]], 1
622620
// CHECK3-NEXT: br label [[OMP_LOOP_HEADER]]
623-
// CHECK3: omp.par.outlined.exit.exitStub:
621+
// CHECK3: omp.par.exit.exitStub:
624622
// CHECK3-NEXT: ret void
625623
//
626624
//
@@ -699,9 +697,7 @@ int inline_decl() {
699697
// CHECK3-NEXT: store ptr [[RES]], ptr [[GEP_RES]], align 8
700698
// CHECK3-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @_Z11inline_declv..omp_par, ptr [[STRUCTARG]])
701699
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
702-
// CHECK3: omp.par.outlined.exit:
703-
// CHECK3-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
704-
// CHECK3: omp.par.exit.split:
700+
// CHECK3: omp.par.exit:
705701
// CHECK3-NEXT: [[TMP0:%.*]] = load i32, ptr [[RES]], align 4
706702
// CHECK3-NEXT: ret i32 [[TMP0]]
707703
//
@@ -789,7 +785,7 @@ int inline_decl() {
789785
// CHECK3: omp_loop.inc:
790786
// CHECK3-NEXT: [[OMP_LOOP_NEXT]] = add nuw i32 [[OMP_LOOP_IV]], 1
791787
// CHECK3-NEXT: br label [[OMP_LOOP_HEADER]]
792-
// CHECK3: omp.par.outlined.exit.exitStub:
788+
// CHECK3: omp.par.exit.exitStub:
793789
// CHECK3-NEXT: ret void
794790
//
795791
//
@@ -870,9 +866,7 @@ int inline_decl() {
870866
// CHECK4-NEXT: store ptr [[K]], ptr [[GEP_K]], align 8
871867
// CHECK4-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @_Z12outline_declv..omp_par, ptr [[STRUCTARG]]), !dbg [[DBG18:![0-9]+]]
872868
// CHECK4-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
873-
// CHECK4: omp.par.outlined.exit:
874-
// CHECK4-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
875-
// CHECK4: omp.par.exit.split:
869+
// CHECK4: omp.par.exit:
876870
// CHECK4-NEXT: [[TMP0:%.*]] = load i32, ptr [[K]], align 4, !dbg [[DBG20:![0-9]+]]
877871
// CHECK4-NEXT: ret i32 [[TMP0]], !dbg [[DBG20]]
878872
//
@@ -959,7 +953,7 @@ int inline_decl() {
959953
// CHECK4: omp_loop.inc:
960954
// CHECK4-NEXT: [[OMP_LOOP_NEXT]] = add nuw i32 [[OMP_LOOP_IV]], 1, !dbg [[DBG28]]
961955
// CHECK4-NEXT: br label [[OMP_LOOP_HEADER]], !dbg [[DBG28]]
962-
// CHECK4: omp.par.outlined.exit.exitStub:
956+
// CHECK4: omp.par.exit.exitStub:
963957
// CHECK4-NEXT: ret void
964958
//
965959
//
@@ -1048,9 +1042,7 @@ int inline_decl() {
10481042
// CHECK4-NEXT: store ptr [[RES]], ptr [[GEP_RES]], align 8
10491043
// CHECK4-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB6]], i32 1, ptr @_Z11inline_declv..omp_par, ptr [[STRUCTARG]]), !dbg [[DBG82:![0-9]+]]
10501044
// CHECK4-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
1051-
// CHECK4: omp.par.outlined.exit:
1052-
// CHECK4-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
1053-
// CHECK4: omp.par.exit.split:
1045+
// CHECK4: omp.par.exit:
10541046
// CHECK4-NEXT: [[TMP0:%.*]] = load i32, ptr [[RES]], align 4, !dbg [[DBG84:![0-9]+]]
10551047
// CHECK4-NEXT: ret i32 [[TMP0]], !dbg [[DBG84]]
10561048
//
@@ -1139,7 +1131,7 @@ int inline_decl() {
11391131
// CHECK4: omp_loop.inc:
11401132
// CHECK4-NEXT: [[OMP_LOOP_NEXT]] = add nuw i32 [[OMP_LOOP_IV]], 1, !dbg [[META95]]
11411133
// CHECK4-NEXT: br label [[OMP_LOOP_HEADER]], !dbg [[META95]]
1142-
// CHECK4: omp.par.outlined.exit.exitStub:
1134+
// CHECK4: omp.par.exit.exitStub:
11431135
// CHECK4-NEXT: ret void
11441136
//
11451137
//

‎clang/test/OpenMP/parallel_codegen.cpp

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -736,9 +736,7 @@ int main (int argc, char **argv) {
736736
// CHECK3-NEXT: store ptr [[VLA]], ptr [[GEP_VLA]], align 8
737737
// CHECK3-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @main..omp_par, ptr [[STRUCTARG]])
738738
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
739-
// CHECK3: omp.par.outlined.exit:
740-
// CHECK3-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
741-
// CHECK3: omp.par.exit.split:
739+
// CHECK3: omp.par.exit:
742740
// CHECK3-NEXT: [[TMP3:%.*]] = load ptr, ptr [[ARGV_ADDR]], align 8
743741
// CHECK3-NEXT: [[CALL:%.*]] = call noundef i32 @_Z5tmainIPPcEiT_(ptr noundef [[TMP3]])
744742
// CHECK3-NEXT: store i32 [[CALL]], ptr [[RETVAL]], align 4
@@ -770,7 +768,7 @@ int main (int argc, char **argv) {
770768
// CHECK3-NEXT: br label [[OMP_PAR_PRE_FINALIZE:%.*]]
771769
// CHECK3: omp.par.pre_finalize:
772770
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB:%.*]]
773-
// CHECK3: omp.par.outlined.exit.exitStub:
771+
// CHECK3: omp.par.exit.exitStub:
774772
// CHECK3-NEXT: ret void
775773
//
776774
//
@@ -805,9 +803,7 @@ int main (int argc, char **argv) {
805803
// CHECK3-NEXT: store ptr [[ARGC_ADDR]], ptr [[GEP_ARGC_ADDR]], align 8
806804
// CHECK3-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @_Z5tmainIPPcEiT_..omp_par, ptr [[STRUCTARG]])
807805
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
808-
// CHECK3: omp.par.outlined.exit:
809-
// CHECK3-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
810-
// CHECK3: omp.par.exit.split:
806+
// CHECK3: omp.par.exit:
811807
// CHECK3-NEXT: ret i32 0
812808
//
813809
//
@@ -837,7 +833,7 @@ int main (int argc, char **argv) {
837833
// CHECK3-NEXT: br label [[OMP_PAR_PRE_FINALIZE:%.*]]
838834
// CHECK3: omp.par.pre_finalize:
839835
// CHECK3-NEXT: br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB:%.*]]
840-
// CHECK3: omp.par.outlined.exit.exitStub:
836+
// CHECK3: omp.par.exit.exitStub:
841837
// CHECK3-NEXT: ret void
842838
//
843839
//
@@ -878,9 +874,7 @@ int main (int argc, char **argv) {
878874
// CHECK4-NEXT: store ptr [[VLA]], ptr [[GEP_VLA]], align 8
879875
// CHECK4-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 1, ptr @main..omp_par, ptr [[STRUCTARG]]), !dbg [[DBG30:![0-9]+]]
880876
// CHECK4-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
881-
// CHECK4: omp.par.outlined.exit:
882-
// CHECK4-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
883-
// CHECK4: omp.par.exit.split:
877+
// CHECK4: omp.par.exit:
884878
// CHECK4-NEXT: [[TMP3:%.*]] = load ptr, ptr [[ARGV_ADDR]], align 8, !dbg [[DBG31:![0-9]+]]
885879
// CHECK4-NEXT: [[CALL:%.*]] = call noundef i32 @_Z5tmainIPPcEiT_(ptr noundef [[TMP3]]), !dbg [[DBG31]]
886880
// CHECK4-NEXT: store i32 [[CALL]], ptr [[RETVAL]], align 4, !dbg [[DBG31]]
@@ -912,7 +906,7 @@ int main (int argc, char **argv) {
912906
// CHECK4-NEXT: br label [[OMP_PAR_PRE_FINALIZE:%.*]]
913907
// CHECK4: omp.par.pre_finalize:
914908
// CHECK4-NEXT: br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB:%.*]], !dbg [[DBG35]]
915-
// CHECK4: omp.par.outlined.exit.exitStub:
909+
// CHECK4: omp.par.exit.exitStub:
916910
// CHECK4-NEXT: ret void
917911
//
918912
//
@@ -949,9 +943,7 @@ int main (int argc, char **argv) {
949943
// CHECK4-NEXT: store ptr [[ARGC_ADDR]], ptr [[GEP_ARGC_ADDR]], align 8
950944
// CHECK4-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB3]], i32 1, ptr @_Z5tmainIPPcEiT_..omp_par, ptr [[STRUCTARG]]), !dbg [[DBG52:![0-9]+]]
951945
// CHECK4-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
952-
// CHECK4: omp.par.outlined.exit:
953-
// CHECK4-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
954-
// CHECK4: omp.par.exit.split:
946+
// CHECK4: omp.par.exit:
955947
// CHECK4-NEXT: ret i32 0, !dbg [[DBG54:![0-9]+]]
956948
//
957949
//
@@ -982,7 +974,7 @@ int main (int argc, char **argv) {
982974
// CHECK4-NEXT: br label [[OMP_PAR_PRE_FINALIZE:%.*]]
983975
// CHECK4: omp.par.pre_finalize:
984976
// CHECK4-NEXT: br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB:%.*]], !dbg [[DBG66]]
985-
// CHECK4: omp.par.outlined.exit.exitStub:
977+
// CHECK4: omp.par.exit.exitStub:
986978
// CHECK4-NEXT: ret void
987979
//
988980
//

‎clang/test/OpenMP/taskgroup_codegen.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,7 @@ void parallel_taskgroup() {
224224
// CHECK2: omp_parallel:
225225
// CHECK2-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB1]], i32 0, ptr @_Z18parallel_taskgroupv..omp_par)
226226
// CHECK2-NEXT: br label [[OMP_PAR_OUTLINED_EXIT:%.*]]
227-
// CHECK2: omp.par.outlined.exit:
228-
// CHECK2-NEXT: br label [[OMP_PAR_EXIT_SPLIT:%.*]]
229-
// CHECK2: omp.par.exit.split:
227+
// CHECK2: omp.par.exit:
230228
// CHECK2-NEXT: ret void
231229
//
232230
//
@@ -250,6 +248,6 @@ void parallel_taskgroup() {
250248
// CHECK2-NEXT: br label [[OMP_PAR_PRE_FINALIZE:%.*]]
251249
// CHECK2: omp.par.pre_finalize:
252250
// CHECK2-NEXT: br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB:%.*]]
253-
// CHECK2: omp.par.outlined.exit.exitStub:
251+
// CHECK2: omp.par.exit.exitStub:
254252
// CHECK2-NEXT: ret void
255253
//

‎flang/test/Integration/OpenMP/parallel-private-reduction-worstcase.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,5 +219,5 @@ subroutine worst_case(a, b, c, d)
219219
! [var extent was non-zero: malloc a private array]
220220
! CHECK: br label %omp.private.init5
221221

222-
! CHECK: omp.par.outlined.exit.exitStub: ; preds = %omp.region.cont52
222+
! CHECK: omp.par.exit.exitStub: ; preds = %omp.region.cont52
223223
! CHECK-NEXT: ret void

‎llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1602,14 +1602,6 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createParallel(
16021602
SmallVector<BasicBlock *, 32> Blocks;
16031603
OI.collectBlocks(ParallelRegionBlockSet, Blocks);
16041604

1605-
// Ensure a single exit node for the outlined region by creating one.
1606-
// We might have multiple incoming edges to the exit now due to finalizations,
1607-
// e.g., cancel calls that cause the control flow to leave the region.
1608-
BasicBlock *PRegOutlinedExitBB = PRegExitBB;
1609-
PRegExitBB = SplitBlock(PRegExitBB, &*PRegExitBB->getFirstInsertionPt());
1610-
PRegOutlinedExitBB->setName("omp.par.outlined.exit");
1611-
Blocks.push_back(PRegOutlinedExitBB);
1612-
16131605
CodeExtractorAnalysisCache CEAC(*OuterFn);
16141606
CodeExtractor Extractor(Blocks, /* DominatorTree */ nullptr,
16151607
/* AggregateArgs */ false,

0 commit comments

Comments
 (0)