[flang-commits] [flang] 748e7af - [flang][OpenMP] Fix firstprivate not working with lastprivate in DO SIMD (#170163)
via flang-commits
flang-commits at lists.llvm.org
Tue Dec 9 07:10:25 PST 2025
Author: Krish Gupta
Date: 2025-12-09T15:10:21Z
New Revision: 748e7af8dd6e9b4683a6402a0ca6598fe23a9c1e
URL: https://github.com/llvm/llvm-project/commit/748e7af8dd6e9b4683a6402a0ca6598fe23a9c1e
DIFF: https://github.com/llvm/llvm-project/commit/748e7af8dd6e9b4683a6402a0ca6598fe23a9c1e.diff
LOG: [flang][OpenMP] Fix firstprivate not working with lastprivate in DO SIMD (#170163)
This fixes a bug where firstprivate was ignored when the same variable
had both firstprivate and lastprivate clauses in a do simd construct.
What was broken:
```
integer :: a
a = 10
!$omp do simd firstprivate(a) lastprivate(a)
do i = 1, 1
print *, a ! Should print 10, but printed garbage/0
a = 20
end do
!$omp end do simd
print *, a ! Correctly prints 20
```
Inside the loop, [a] wasn't being initialized from the firstprivate
clause—it just had whatever uninitialized value was there.
The fix:
In genCompositeDoSimd(), we were using simdItemDSP to handle
privatization for the whole loop nest. This only looked at SIMD clauses
and missed the firstprivate from the DO part. Changed it to use
wsloopItemDSP instead, which handles both DO clauses (firstprivate,
lastprivate) correctly.
One line change in OpenMP.cpp
Tests added:
Lowering test to check MLIR generation
Runtime test to verify the actual values are correct
<img width="740" height="440" alt="image"
src="https://github.com/user-attachments/assets/fa911ea8-2024-4edf-b710-52c10659742e"
/>
Fixes #168306
---------
Co-authored-by: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Added:
flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90
flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90
Modified:
flang/lib/Lower/OpenMP/OpenMP.cpp
flang/test/Lower/OpenMP/order-clause.f90
flang/test/Lower/OpenMP/wsloop-simd.f90
Removed:
################################################################################
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 582e684442dfc..9c25c1955cb78 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3314,17 +3314,12 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps,
simdReductionSyms);
- DataSharingProcessor wsloopItemDSP(
- converter, semaCtx, doItem->clauses, eval,
- /*shouldCollectPreDeterminedSymbols=*/false,
- /*useDelayedPrivatization=*/true, symTable);
+ DataSharingProcessor wsloopItemDSP(converter, semaCtx, doItem->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/true,
+ /*useDelayedPrivatization=*/true,
+ symTable);
wsloopItemDSP.processStep1(&wsloopClauseOps);
- DataSharingProcessor simdItemDSP(converter, semaCtx, simdItem->clauses, eval,
- /*shouldCollectPreDeterminedSymbols=*/true,
- /*useDelayedPrivatization=*/true, symTable);
- simdItemDSP.processStep1(&simdClauseOps, simdItem->id);
-
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
// is placed by construct decomposition.
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -3343,8 +3338,9 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
wsloopOp.setComposite(/*val=*/true);
EntryBlockArgs simdArgs;
- simdArgs.priv.syms = simdItemDSP.getDelayedPrivSymbols();
- simdArgs.priv.vars = simdClauseOps.privateVars;
+ // For composite 'do simd', privatization is handled by the wsloop.
+ // The simd does not create separate private storage for variables already
+ // privatized by the worksharing construct.
simdArgs.reduction.syms = simdReductionSyms;
simdArgs.reduction.vars = simdClauseOps.reductionVars;
auto simdOp =
@@ -3354,7 +3350,7 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, simdItem,
loopNestClauseOps, iv,
{{wsloopOp, wsloopArgs}, {simdOp, simdArgs}},
- llvm::omp::Directive::OMPD_do_simd, simdItemDSP);
+ llvm::omp::Directive::OMPD_do_simd, wsloopItemDSP);
return wsloopOp;
}
diff --git a/flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90 b/flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90
new file mode 100644
index 0000000000000..4fef69188e0ee
--- /dev/null
+++ b/flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90
@@ -0,0 +1,48 @@
+! Test runtime behavior of DO SIMD with firstprivate and lastprivate on same variable
+! This is the reproducer from issue #168306
+
+! REQUIRES: openmp-runtime
+
+! RUN: %flang_fc1 -fopenmp -emit-llvm %s -o - | FileCheck %s --check-prefix=LLVM
+! RUN: %flang -fopenmp %s -o %t && %t | FileCheck %s
+
+! LLVM-LABEL: define {{.*}} @_QQmain
+program main
+ integer :: a
+ integer :: i
+
+ a = 10
+ !$omp do simd lastprivate(a) firstprivate(a)
+ do i = 1, 1
+ ! Inside loop: a should be 10 (from firstprivate initialization)
+ ! CHECK: main1 : a = 10
+ print *, "main1 : a = ", a
+ a = 20
+ end do
+ !$omp end do simd
+ ! After loop: a should be 20 (from lastprivate copy-out)
+ ! CHECK: main2 : a = 20
+ print *, "main2 : a = ", a
+
+ call sub
+ ! CHECK: pass
+ print *, 'pass'
+end program main
+
+subroutine sub
+ integer :: a
+ integer :: i
+
+ a = 10
+ !$omp do simd lastprivate(a) firstprivate(a)
+ do i = 1, 1
+ ! Inside loop: a should be 10 (from firstprivate initialization)
+ ! CHECK: sub1 : a = 10
+ print *, "sub1 : a = ", a
+ a = 20
+ end do
+ !$omp end do simd
+ ! After loop: a should be 20 (from lastprivate copy-out)
+ ! CHECK: sub2 : a = 20
+ print *, "sub2 : a = ", a
+end subroutine sub
diff --git a/flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90 b/flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90
new file mode 100644
index 0000000000000..429409926d47b
--- /dev/null
+++ b/flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90
@@ -0,0 +1,89 @@
+! Test for DO SIMD with the same variable in both firstprivate and lastprivate clauses
+! This tests the fix for issue #168306
+
+! RUN: %flang_fc1 -fopenmp -mmlir --enable-delayed-privatization-staging=true -emit-hlfir %s -o - | FileCheck %s
+
+! Test case 1: Basic test with firstprivate + lastprivate on same variable
+! CHECK-LABEL: func.func @_QPdo_simd_first_last_same_var
+subroutine do_simd_first_last_same_var()
+ integer :: a
+ integer :: i
+ a = 10
+
+ ! CHECK: omp.wsloop
+ ! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %[[FIRSTPRIV_A:.*]], @{{.*}}private{{.*}} %{{.*}} -> %[[PRIV_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+ ! CHECK-NEXT: omp.simd
+ ! CHECK-NOT: private
+ ! CHECK-NEXT: omp.loop_nest (%[[IV:.*]]) : i32
+ !$omp do simd firstprivate(a) lastprivate(a)
+ do i = 1, 1
+ ! CHECK: %[[FIRSTPRIV_A_DECL:.*]]:2 = hlfir.declare %[[FIRSTPRIV_A]]
+ ! CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[PRIV_I]]
+ ! The private copy should be initialized from firstprivate (value 10)
+ ! and then modified to 20
+ a = 20
+ end do
+ !$omp end do simd
+ ! After the loop, 'a' should be 20 due to lastprivate
+end subroutine do_simd_first_last_same_var
+
+! Test case 2: Test with lastprivate and firstprivate in reverse order
+! CHECK-LABEL: func.func @_QPdo_simd_last_first_reverse
+subroutine do_simd_last_first_reverse()
+ integer :: a
+ integer :: i
+ a = 10
+
+ ! CHECK: omp.wsloop
+ ! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %[[FIRSTPRIV_A:.*]], @{{.*}}private{{.*}} %{{.*}} -> %[[PRIV_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+ ! CHECK-NEXT: omp.simd
+ ! CHECK-NOT: private
+ !$omp do simd lastprivate(a) firstprivate(a)
+ do i = 1, 1
+ a = 20
+ end do
+ !$omp end do simd
+end subroutine do_simd_last_first_reverse
+
+! Test case 3: Multiple variables with mixed privatization
+! CHECK-LABEL: func.func @_QPdo_simd_multiple_vars
+subroutine do_simd_multiple_vars()
+ integer :: a, b, c
+ integer :: i
+ a = 10
+ b = 20
+ c = 30
+
+ ! CHECK: omp.wsloop
+ ! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %{{.*}}, @{{.*}}firstprivate{{.*}} %{{.*}} -> %{{.*}}, @{{.*}}private{{.*}} %{{.*}} -> %{{.*}} : !fir.ref<i32>, !fir.ref<i32>, !fir.ref<i32>)
+ ! CHECK-NEXT: omp.simd
+ ! CHECK-NOT: private
+ !$omp do simd firstprivate(a, b) lastprivate(a) private(c)
+ do i = 1, 5
+ a = a + 1
+ b = b + 1
+ c = i
+ end do
+ !$omp end do simd
+end subroutine do_simd_multiple_vars
+
+! Test case 4: Reproducer from issue #168306
+! CHECK-LABEL: func.func @_QPissue_168306_reproducer
+subroutine issue_168306_reproducer()
+ integer :: a
+ integer :: i
+ a = 10
+
+ ! CHECK: omp.wsloop
+ ! CHECK-SAME: private(@{{.*}}firstprivate{{.*}} %{{.*}} -> %[[FIRSTPRIV_A:.*]], @{{.*}}private{{.*}} %{{.*}} -> %[[PRIV_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+ ! CHECK-NEXT: omp.simd
+ ! CHECK-NOT: private
+ !$omp do simd lastprivate(a) firstprivate(a)
+ do i = 1, 1
+ ! Inside the loop, 'a' should start at 10 (from firstprivate)
+ ! This is the key behavior that was broken
+ a = 20
+ end do
+ !$omp end do simd
+ ! After the loop, 'a' should be 20 (from lastprivate)
+end subroutine issue_168306_reproducer
diff --git a/flang/test/Lower/OpenMP/order-clause.f90 b/flang/test/Lower/OpenMP/order-clause.f90
index d5799079b3759..9da7d905ceeed 100644
--- a/flang/test/Lower/OpenMP/order-clause.f90
+++ b/flang/test/Lower/OpenMP/order-clause.f90
@@ -36,15 +36,15 @@ end subroutine do_order
!CHECK-LABEL: func.func @_QPdo_simd_order() {
subroutine do_simd_order
- !CHECK: omp.wsloop order(reproducible:concurrent) {
+ !CHECK: omp.wsloop order(reproducible:concurrent)
!$omp do simd order(concurrent)
do i = 1, 10
end do
- !CHECK: omp.wsloop order(reproducible:concurrent) {
+ !CHECK: omp.wsloop order(reproducible:concurrent)
!$omp do simd order(reproducible:concurrent)
do i = 1, 10
end do
- !CHECK: omp.wsloop order(unconstrained:concurrent) {
+ !CHECK: omp.wsloop order(unconstrained:concurrent)
!$omp do simd order(unconstrained:concurrent)
do i = 1, 10
end do
@@ -53,7 +53,7 @@ end subroutine do_simd_order
!CHECK-LABEL: func.func @_QPdo_simd_order_parallel() {
subroutine do_simd_order_parallel
!CHECK: omp.parallel {
- !CHECK: omp.wsloop order(reproducible:concurrent) {
+ !CHECK: omp.wsloop order(reproducible:concurrent)
!$omp parallel do simd order(reproducible:concurrent)
do i = 1, 10
end do
diff --git a/flang/test/Lower/OpenMP/wsloop-simd.f90 b/flang/test/Lower/OpenMP/wsloop-simd.f90
index 03e35de04cace..b18bc29efb230 100644
--- a/flang/test/Lower/OpenMP/wsloop-simd.f90
+++ b/flang/test/Lower/OpenMP/wsloop-simd.f90
@@ -71,16 +71,13 @@ end subroutine do_simd_reduction
subroutine do_simd_private()
integer, allocatable :: tmp
! CHECK: omp.wsloop
+ ! CHECK-SAME: private(@[[PRIV_IVAR_SYM:.*]] %{{.*}} -> %[[PRIV_IVAR:.*]] : !fir.ref<i32>)
! CHECK-NEXT: omp.simd
- ! CHECK-SAME: private(@[[PRIV_BOX_SYM:.*]] %{{.*}} -> %[[PRIV_BOX:.*]], @[[PRIV_IVAR_SYM:.*]] %{{.*}} -> %[[PRIV_IVAR:.*]] : !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<i32>)
! CHECK-NEXT: omp.loop_nest (%[[IVAR:.*]]) : i32
!$omp do simd private(tmp)
do i=1, 10
- ! CHECK: %[[PRIV_BOX_DECL:.*]]:2 = hlfir.declare %[[PRIV_BOX]]
! CHECK: %[[PRIV_IVAR_DECL:.*]]:2 = hlfir.declare %[[PRIV_IVAR]]
! CHECK: hlfir.assign %[[IVAR]] to %[[PRIV_IVAR_DECL]]#0
- ! CHECK: %[[PRIV_BOX_LOAD:.*]] = fir.load %[[PRIV_BOX_DECL]]
- ! CHECK: hlfir.assign %{{.*}} to %[[PRIV_BOX_DECL]]#0
! CHECK: omp.yield
tmp = tmp + 1
end do
@@ -90,13 +87,11 @@ end subroutine do_simd_private
subroutine do_simd_lastprivate_firstprivate()
integer :: a
! CHECK: omp.wsloop
- ! CHECK-SAME: private(@[[FIRSTPRIVATE_A_SYM:.*]] %{{.*}} -> %[[FIRSTPRIVATE_A:.*]] : !fir.ref<i32>)
+ ! CHECK-SAME: private(@[[FIRSTPRIVATE_A_SYM:.*]] %{{.*}} -> %[[FIRSTPRIVATE_A:.*]], @[[PRIVATE_I_SYM:.*]] %{{.*}} -> %[[PRIVATE_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
! CHECK-NEXT: omp.simd
- ! CHECK-SAME: private(@[[PRIVATE_A_SYM:.*]] %{{.*}} -> %[[PRIVATE_A:.*]], @[[PRIVATE_I_SYM:.*]] %{{.*}} -> %[[PRIVATE_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
!$omp do simd lastprivate(a) firstprivate(a)
do i = 1, 10
! CHECK: %[[FIRSTPRIVATE_A_DECL:.*]]:2 = hlfir.declare %[[FIRSTPRIVATE_A]]
- ! CHECK: %[[PRIVATE_A_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_A]]
! CHECK: %[[PRIVATE_I_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_I]]
a = a + 1
end do
More information about the flang-commits
mailing list