Skip to content

Commit 6fe8471

Browse files
Krish GuptaKrish Gupta
Krish Gupta
authored and
Krish Gupta
committed
[flang][OpenMP] Fix firstprivate in do simd composite constructs
Remove duplicate privatization in do simd composites. Only the wsloop should create private allocations; simd reuses them per OpenMP spec. Fixes #168306
1 parent af0fcf8 commit 6fe8471

File tree

3 files changed

+141
-12
lines changed

3 files changed

+141
-12
lines changed

‎flang/lib/Lower/OpenMP/OpenMP.cpp‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3284,17 +3284,12 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
32843284
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps,
32853285
simdReductionSyms);
32863286

3287-
DataSharingProcessor wsloopItemDSP(
3288-
converter, semaCtx, doItem->clauses, eval,
3289-
/*shouldCollectPreDeterminedSymbols=*/false,
3290-
/*useDelayedPrivatization=*/true, symTable);
3287+
DataSharingProcessor wsloopItemDSP(converter, semaCtx, doItem->clauses, eval,
3288+
/*shouldCollectPreDeterminedSymbols=*/true,
3289+
/*useDelayedPrivatization=*/true,
3290+
symTable);
32913291
wsloopItemDSP.processStep1(&wsloopClauseOps);
32923292

3293-
DataSharingProcessor simdItemDSP(converter, semaCtx, simdItem->clauses, eval,
3294-
/*shouldCollectPreDeterminedSymbols=*/true,
3295-
/*useDelayedPrivatization=*/true, symTable);
3296-
simdItemDSP.processStep1(&simdClauseOps, simdItem->id);
3297-
32983293
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
32993294
// is placed by construct decomposition.
33003295
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -3313,8 +3308,9 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
33133308
wsloopOp.setComposite(/*val=*/true);
33143309

33153310
EntryBlockArgs simdArgs;
3316-
simdArgs.priv.syms = simdItemDSP.getDelayedPrivSymbols();
3317-
simdArgs.priv.vars = simdClauseOps.privateVars;
3311+
// For composite 'do simd', privatization is handled by the wsloop.
3312+
// The simd does not create separate private storage for variables already
3313+
// privatized by the worksharing construct.
33183314
simdArgs.reduction.syms = simdReductionSyms;
33193315
simdArgs.reduction.vars = simdClauseOps.reductionVars;
33203316
auto simdOp =
@@ -3324,7 +3320,7 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
33243320
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, simdItem,
33253321
loopNestClauseOps, iv,
33263322
{{wsloopOp, wsloopArgs}, {simdOp, simdArgs}},
3327-
llvm::omp::Directive::OMPD_do_simd, simdItemDSP);
3323+
llvm::omp::Directive::OMPD_do_simd, wsloopItemDSP);
33283324
return wsloopOp;
33293325
}
33303326

‎flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
! Test runtime behavior of DO SIMD with firstprivate and lastprivate on same variable
2+
! This is the reproducer from issue #168306
3+
4+
! REQUIRES: openmp-runtime
5+
6+
! RUN: %flang_fc1 -fopenmp -emit-llvm %s -o - | FileCheck %s --check-prefix=LLVM
7+
! RUN: %flang -fopenmp %s -o %t && %t | FileCheck %s
8+
9+
! LLVM-LABEL: define {{.*}} @_QQmain
10+
program main
11+
integer :: a
12+
integer :: i
13+
14+
a = 10
15+
!$omp do simd lastprivate(a) firstprivate(a)
16+
do i = 1, 1
17+
! Inside loop: a should be 10 (from firstprivate initialization)
18+
! CHECK: main1 : a = 10
19+
print *, "main1 : a = ", a
20+
a = 20
21+
end do
22+
!$omp end do simd
23+
! After loop: a should be 20 (from lastprivate copy-out)
24+
! CHECK: main2 : a = 20
25+
print *, "main2 : a = ", a
26+
27+
call sub
28+
! CHECK: pass
29+
print *, 'pass'
30+
end program main
31+
32+
subroutine sub
33+
integer :: a
34+
integer :: i
35+
36+
a = 10
37+
!$omp do simd lastprivate(a) firstprivate(a)
38+
do i = 1, 1
39+
! Inside loop: a should be 10 (from firstprivate initialization)
40+
! CHECK: sub1 : a = 10
41+
print *, "sub1 : a = ", a
42+
a = 20
43+
end do
44+
!$omp end do simd
45+
! After loop: a should be 20 (from lastprivate copy-out)
46+
! CHECK: sub2 : a = 20
47+
print *, "sub2 : a = ", a
48+
end subroutine sub

‎flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
! Test for DO SIMD with the same variable in both firstprivate and lastprivate clauses
2+
! This tests the fix for issue #168306
3+
4+
! RUN: %flang_fc1 -fopenmp -mmlir --enable-delayed-privatization-staging=true -emit-hlfir %s -o - | FileCheck %s
5+
6+
! Test case 1: Basic test with firstprivate + lastprivate on same variable
7+
! CHECK-LABEL: func.func @_QPdo_simd_first_last_same_var
8+
subroutine do_simd_first_last_same_var()
9+
integer :: a
10+
integer :: i
11+
a = 10
12+
13+
! CHECK: omp.wsloop
14+
! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %[[FIRSTPRIV_A:.*]], @{{.*}}private{{.*}} %{{.*}} -> %[[PRIV_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
15+
! CHECK-NEXT: omp.simd
16+
! CHECK-NEXT: omp.loop_nest (%[[IV:.*]]) : i32
17+
!$omp do simd firstprivate(a) lastprivate(a)
18+
do i = 1, 1
19+
! CHECK: %[[FIRSTPRIV_A_DECL:.*]]:2 = hlfir.declare %[[FIRSTPRIV_A]]
20+
! CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[PRIV_I]]
21+
! The private copy should be initialized from firstprivate (value 10)
22+
! and then modified to 20
23+
a = 20
24+
end do
25+
!$omp end do simd
26+
! After the loop, 'a' should be 20 due to lastprivate
27+
end subroutine do_simd_first_last_same_var
28+
29+
! Test case 2: Test with lastprivate and firstprivate in reverse order
30+
! CHECK-LABEL: func.func @_QPdo_simd_last_first_reverse
31+
subroutine do_simd_last_first_reverse()
32+
integer :: a
33+
integer :: i
34+
a = 10
35+
36+
! CHECK: omp.wsloop
37+
! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %[[FIRSTPRIV_A:.*]], @{{.*}}private{{.*}} %{{.*}} -> %[[PRIV_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
38+
! CHECK-NEXT: omp.simd
39+
!$omp do simd lastprivate(a) firstprivate(a)
40+
do i = 1, 1
41+
a = 20
42+
end do
43+
!$omp end do simd
44+
end subroutine do_simd_last_first_reverse
45+
46+
! Test case 3: Multiple variables with mixed privatization
47+
! CHECK-LABEL: func.func @_QPdo_simd_multiple_vars
48+
subroutine do_simd_multiple_vars()
49+
integer :: a, b, c
50+
integer :: i
51+
a = 10
52+
b = 20
53+
c = 30
54+
55+
! CHECK: omp.wsloop
56+
! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %{{.*}}, @{{.*}}firstprivate{{.*}} %{{.*}} -> %{{.*}}, @{{.*}}private{{.*}} %{{.*}} -> %{{.*}} : !fir.ref<i32>, !fir.ref<i32>, !fir.ref<i32>)
57+
! CHECK-NEXT: omp.simd
58+
!$omp do simd firstprivate(a, b) lastprivate(a) private(c)
59+
do i = 1, 5
60+
a = a + 1
61+
b = b + 1
62+
c = i
63+
end do
64+
!$omp end do simd
65+
end subroutine do_simd_multiple_vars
66+
67+
! Test case 4: Reproducer from issue #168306
68+
! CHECK-LABEL: func.func @_QPissue_168306_reproducer
69+
subroutine issue_168306_reproducer()
70+
integer :: a
71+
integer :: i
72+
a = 10
73+
74+
! CHECK: omp.wsloop
75+
! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %[[FIRSTPRIV_A:.*]], @{{.*}}private{{.*}} %{{.*}} -> %[[PRIV_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
76+
! CHECK-NEXT: omp.simd
77+
!$omp do simd lastprivate(a) firstprivate(a)
78+
do i = 1, 1
79+
! Inside the loop, 'a' should start at 10 (from firstprivate)
80+
! This is the key behavior that was broken
81+
a = 20
82+
end do
83+
!$omp end do simd
84+
! After the loop, 'a' should be 20 (from lastprivate)
85+
end subroutine issue_168306_reproducer

0 commit comments

Comments
 (0)