Skip to content

Commit 6b3ba66

Browse files
ergawyergawy
authored
[flang][OpenMP] Unconditionally create after_alloca block in allocatePrivateVars (#123168)
While #122866 fixed some issues, it introduced a regression in worksharing loops. The new bug comes from the fact that we now conditionally created the `after_alloca` block based on the number of sucessors of the alloca insertion point. This is unneccessary, we can just alway create the block. If we do this, we respect the post condtions expected after calling `allocatePrivateVars` (i.e. that the `afterAlloca` block has a single predecessor.
1 parent 8965dd4 commit 6b3ba66

10 files changed

+89
-13
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,12 @@ subroutine worst_case(a, b, c, d)
9696

9797
! CHECK: omp.region.cont13: ; preds = %omp.private.copy16
9898
! CHECK-NEXT: %{{.*}} = phi ptr
99+
! CHECK-NEXT: br label %omp.region.after_alloca
100+
101+
! CHECK: omp.region.after_alloca:
99102
! CHECK-NEXT: br label %omp.par.region
100103

101-
! CHECK: omp.par.region: ; preds = %omp.region.cont13
104+
! CHECK: omp.par.region: ; preds = %omp.region.after_alloca
102105
! CHECK-NEXT: br label %omp.reduction.init
103106

104107
! CHECK: omp.reduction.init: ; preds = %omp.par.region

‎mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,11 +1350,9 @@ allocatePrivateVars(llvm::IRBuilderBase &builder,
13501350
// Allocate private vars
13511351
llvm::BranchInst *allocaTerminator =
13521352
llvm::cast<llvm::BranchInst>(allocaIP.getBlock()->getTerminator());
1353-
if (allocaTerminator->getNumSuccessors() != 1) {
1354-
splitBB(llvm::OpenMPIRBuilder::InsertPointTy(
1355-
allocaIP.getBlock(), allocaTerminator->getIterator()),
1356-
true, "omp.region.after_alloca");
1357-
}
1353+
splitBB(llvm::OpenMPIRBuilder::InsertPointTy(allocaIP.getBlock(),
1354+
allocaTerminator->getIterator()),
1355+
true, "omp.region.after_alloca");
13581356

13591357
llvm::IRBuilderBase::InsertPointGuard guard(builder);
13601358
// Update the allocaTerminator in case the alloca block was split above.

‎mlir/test/Target/LLVMIR/openmp-llvm.mlir‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2766,7 +2766,9 @@ llvm.func @task(%arg0 : !llvm.ptr) {
27662766
// CHECK: %[[VAL_19:.*]] = load i32, ptr %[[VAL_14]], align 4
27672767
// CHECK: store i32 %[[VAL_19]], ptr %[[VAL_15]], align 4
27682768
// CHECK: br label %[[VAL_20:.*]]
2769-
// CHECK: task.body: ; preds = %omp.private.copy
2769+
// CHECK: [[VAL_20]]:
2770+
// CHECK: br label %task.body
2771+
// CHECK: task.body: ; preds = %[[VAL_20]]
27702772
// CHECK: br label %omp.task.region
27712773
// CHECK: omp.task.region: ; preds = %task.body
27722774
// CHECK: call void @foo(ptr %[[VAL_15]])

‎mlir/test/Target/LLVMIR/openmp-parallel-reduction-multiblock.mlir‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@ llvm.func @missordered_blocks_(%arg0: !llvm.ptr {fir.bindc_name = "x"}, %arg1: !
5656
// CHECK: %[[VAL_20:.*]] = alloca ptr, align 8
5757
// CHECK: %[[VAL_21:.*]] = alloca ptr, align 8
5858
// CHECK: %[[VAL_22:.*]] = alloca [2 x ptr], align 8
59+
// CHECK: br label %[[AFTER_ALLOC:omp.region.after_alloca]]
60+
// CHECK: [[AFTER_ALLOC]]: ; preds = %[[PAR_ENTRY]]
5961
// CHECK: br label %[[VAL_23:omp.par.region]]
60-
// CHECK: [[VAL_23]]: ; preds = %[[PAR_ENTRY]]
62+
// CHECK: [[VAL_23]]: ; preds = %[[AFTER_ALLOC]]
6163
// CHECK: br label %[[VAL_42:.*]]
6264
// CHECK: [[RED_INIT:omp.reduction.init]]:
6365
// CHECK: br label %[[VAL_25:omp.reduction.neutral]]

‎mlir/test/Target/LLVMIR/openmp-reduction-array-sections.mlir‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,12 @@ llvm.func @sectionsreduction_(%arg0: !llvm.ptr {fir.bindc_name = "x"}) attribute
9191
// CHECK: %[[VAL_14:.*]] = alloca [1 x ptr], align 8
9292
// CHECK: br label %[[VAL_15:.*]]
9393

94-
// CHECK: omp.par.region: ; preds = %[[PAR_ENTRY]]
94+
// CHECK: [[VAL_15]]:
95+
// CHECK: br label %[[PAR_REG:omp.par.region]]
96+
97+
// CHECK: [[PAR_REG]]: ; preds = %[[VAL_15]]
9598
// CHECK: br label %[[VAL_18:.*]]
96-
// CHECK: omp.par.region1: ; preds = %[[VAL_15]]
99+
// CHECK: omp.par.region1: ; preds = %[[PAR_REG]]
97100
// CHECK: %[[VAL_19:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, i64 1, align 8
98101
// CHECK: br label %[[VAL_22:.*]]
99102

‎mlir/test/Target/LLVMIR/openmp-reduction-init-arg.mlir‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ module {
6363
// CHECK: %[[VAL_23:.*]] = alloca ptr, align 8
6464
// CHECK: %[[VAL_24:.*]] = alloca [2 x ptr], align 8
6565
// CHECK: br label %[[VAL_25:.*]]
66-
// CHECK: omp.par.region: ; preds = %[[PAR_ENTRY]]
66+
67+
// CHECK: [[VAL_25]]:
68+
// CHECK: br label %[[PAR_REG:omp.par.region]]
69+
70+
// CHECK: [[PAR_REG]]: ; preds = %[[VAL_25]]
6771
// CHECK: br label %[[INIT_LABEL:.*]]
6872
// CHECK: [[INIT_LABEL]]:
6973
// CHECK: %[[VAL_20:.*]] = load { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr %[[VAL_13]], align 8

‎mlir/test/Target/LLVMIR/openmp-reduction-sections.mlir‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,13 @@ llvm.func @sections_(%arg0: !llvm.ptr {fir.bindc_name = "x"}) attributes {fir.in
5050
// CHECK: %[[VAL_20:.*]] = alloca float, align 4
5151
// CHECK: %[[VAL_21:.*]] = alloca [1 x ptr], align 8
5252
// CHECK: br label %[[VAL_22:.*]]
53-
// CHECK: omp.par.region: ; preds = %[[PAR_ENTRY]]
53+
54+
// CHECK: [[VAL_22]]:
55+
// CHECK: br label %[[PAR_REG:omp.par.region]]
56+
57+
// CHECK: [[PAR_REG]]: ; preds = %[[VAL_22]]
5458
// CHECK: br label %[[VAL_25:.*]]
55-
// CHECK: omp.par.region1: ; preds = %[[VAL_22]]
59+
// CHECK: omp.par.region1: ; preds = %[[PAR_REG]]
5660
// CHECK: br label %[[VAL_26:.*]]
5761

5862
// CHECK: [[RED_INIT:omp.reduction.init]]:

‎mlir/test/Target/LLVMIR/openmp-simd-private.mlir‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ omp.private {type = private} @i_privatizer : !llvm.ptr alloc {
1212
// CHECK: %{{.*}} = alloca i32, i64 1, align 4
1313
// CHECK: %[[DUMMY:.*]] = alloca float, i64 1, align 4
1414
// CHECK: %[[PRIV_I:.*]] = alloca i32, i64 1, align 4
15+
// CHECK: br label %[[LATE_ALLOC:.*]]
16+
17+
// CHECK: [[LATE_ALLOC]]:
1518
// CHECK: br label %[[AFTER_ALLOC:.*]]
1619

1720
// CHECK: [[AFTER_ALLOC]]:

‎mlir/test/Target/LLVMIR/openmp-target-use-device-nested.mlir‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
// CHECK-NEXT: br i1 %[[VAL_7]], label %[[VAL_8:.*]], label %[[VAL_9:.*]]
1313
// CHECK: user_code.entry: ; preds = %[[VAL_10:.*]]
1414
// CHECK-NEXT: %[[VAL_11:.*]] = load ptr, ptr %[[VAL_3]], align 8
15+
// CHECK-NEXT: br label %[[AFTER_ALLOC:.*]]
16+
17+
// CHECK: [[AFTER_ALLOC]]:
1518
// CHECK-NEXT: br label %[[VAL_12:.*]]
1619

1720
// CHECK: [[VAL_12]]:

‎mlir/test/Target/LLVMIR/openmp-wsloop-test-block-structure.mlir‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
2+
3+
// Tests regression uncovered by "1009/1009_0029.f90" (from the Fujitsu test
4+
// suite). This test replicates a simplified version of the block structure
5+
// produced by the Fujitsu test.
6+
7+
llvm.func @test_block_structure() {
8+
%i1 = llvm.mlir.constant(1 : index) : i1
9+
%i64 = llvm.mlir.constant(1 : index) : i64
10+
llvm.br ^bb1(%i64, %i64 : i64, i64)
11+
12+
^bb1(%20: i64, %21: i64): // 2 preds: ^bb0, ^bb5
13+
llvm.cond_br %i1, ^bb2, ^bb6
14+
15+
^bb2: // pred: ^bb1
16+
llvm.br ^bb3(%i64, %i64 : i64, i64)
17+
18+
^bb3(%25: i64, %26: i64): // 2 preds: ^bb2, ^bb4
19+
llvm.cond_br %i1, ^bb4, ^bb5
20+
21+
^bb4: // pred: ^bb3
22+
omp.wsloop {
23+
omp.loop_nest (%arg0) : i64 = (%i64) to (%i64) inclusive step (%i64) {
24+
omp.yield
25+
}
26+
}
27+
llvm.br ^bb1(%i64, %i64 : i64, i64)
28+
29+
^bb5: // pred: ^bb3
30+
llvm.br ^bb1(%i64, %i64 : i64, i64)
31+
32+
^bb6: // pred: ^bb1
33+
llvm.return
34+
}
35+
36+
// CHECK: define void @test_block_structure
37+
// CHECK: br label %[[AFTER_ALLOCA:.*]]
38+
39+
// CHECK: [[AFTER_ALLOCA:]]:
40+
// CHECK: br label %[[BB1:.*]]
41+
42+
// CHECK: [[BB1:]]:
43+
// CHECK: %{{.*}} = phi i64
44+
// CHECK: br i1 true, label %[[BB2:.*]], label %{{.*}}
45+
46+
// CHECK: [[BB2]]:
47+
// CHECK: br label %[[BB3:.*]]
48+
49+
// CHECK: [[BB3]]:
50+
// CHECK: %{{.*}} = phi i64
51+
// CHECK: br i1 true, label %[[BB4:.*]], label %{{.*}}
52+
53+
// CHECK: [[BB4]]:
54+
// CHECK: br label %omp_loop.preheader

0 commit comments

Comments
 (0)