[flang-commits] [flang] Revert "[flang][OpenMP] Fix firstprivate not working with lastprivate in DO SIMD" (PR #171646)
via flang-commits
flang-commits at lists.llvm.org
Wed Dec 10 09:33:44 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Tom Eccles (tblah)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->170163
Regression in fujitsu test suite
---
Full diff: https://github.com/llvm/llvm-project/pull/171646.diff
5 Files Affected:
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+12-8)
- (removed) flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90 (-48)
- (removed) flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90 (-89)
- (modified) flang/test/Lower/OpenMP/order-clause.f90 (+4-4)
- (modified) flang/test/Lower/OpenMP/wsloop-simd.f90 (+7-2)
``````````diff
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 9c25c1955cb78..582e684442dfc 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3314,12 +3314,17 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps,
simdReductionSyms);
- DataSharingProcessor wsloopItemDSP(converter, semaCtx, doItem->clauses, eval,
- /*shouldCollectPreDeterminedSymbols=*/true,
- /*useDelayedPrivatization=*/true,
- symTable);
+ DataSharingProcessor wsloopItemDSP(
+ converter, semaCtx, doItem->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/false,
+ /*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;
@@ -3338,9 +3343,8 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
wsloopOp.setComposite(/*val=*/true);
EntryBlockArgs simdArgs;
- // 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.priv.syms = simdItemDSP.getDelayedPrivSymbols();
+ simdArgs.priv.vars = simdClauseOps.privateVars;
simdArgs.reduction.syms = simdReductionSyms;
simdArgs.reduction.vars = simdClauseOps.reductionVars;
auto simdOp =
@@ -3350,7 +3354,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, wsloopItemDSP);
+ llvm::omp::Directive::OMPD_do_simd, simdItemDSP);
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
deleted file mode 100644
index 4fef69188e0ee..0000000000000
--- a/flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90
+++ /dev/null
@@ -1,48 +0,0 @@
-! 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
deleted file mode 100644
index 429409926d47b..0000000000000
--- a/flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90
+++ /dev/null
@@ -1,89 +0,0 @@
-! 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 9da7d905ceeed..d5799079b3759 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 b18bc29efb230..03e35de04cace 100644
--- a/flang/test/Lower/OpenMP/wsloop-simd.f90
+++ b/flang/test/Lower/OpenMP/wsloop-simd.f90
@@ -71,13 +71,16 @@ 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
@@ -87,11 +90,13 @@ end subroutine do_simd_private
subroutine do_simd_lastprivate_firstprivate()
integer :: a
! CHECK: omp.wsloop
- ! CHECK-SAME: private(@[[FIRSTPRIVATE_A_SYM:.*]] %{{.*}} -> %[[FIRSTPRIVATE_A:.*]], @[[PRIVATE_I_SYM:.*]] %{{.*}} -> %[[PRIVATE_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+ ! CHECK-SAME: private(@[[FIRSTPRIVATE_A_SYM:.*]] %{{.*}} -> %[[FIRSTPRIVATE_A:.*]] : !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
``````````
</details>
https://github.com/llvm/llvm-project/pull/171646
More information about the flang-commits
mailing list