[flang-commits] [flang] [flang][OpenMP] Fix firstprivate not working with lastprivate in DO SIMD (PR #170163)
Krish Gupta via flang-commits
flang-commits at lists.llvm.org
Wed Dec 3 05:47:04 PST 2025
https://github.com/KrxGu updated https://github.com/llvm/llvm-project/pull/170163
>From 6fe8471674c050fc61762b30bf50a12ffb2066cf Mon Sep 17 00:00:00 2001
From: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Date: Mon, 1 Dec 2025 21:36:25 +0530
Subject: [PATCH] [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
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 20 ++---
...-simd-firstprivate-lastprivate-runtime.f90 | 48 +++++++++++
.../do-simd-firstprivate-lastprivate.f90 | 85 +++++++++++++++++++
3 files changed, 141 insertions(+), 12 deletions(-)
create mode 100644 flang/test/Integration/OpenMP/do-simd-firstprivate-lastprivate-runtime.f90
create mode 100644 flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 6ca8636bb6459..046fdb0a9f03d 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3284,17 +3284,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;
@@ -3313,8 +3308,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 =
@@ -3324,7 +3320,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..4f59d4c21fafb
--- /dev/null
+++ b/flang/test/Lower/OpenMP/do-simd-firstprivate-lastprivate.f90
@@ -0,0 +1,85 @@
+! 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-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
+ !$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
+ !$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
+ !$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
More information about the flang-commits
mailing list