[flang][OpenMP] Fix firstprivate not working with lastprivate in DO SIMD

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;
 }
 

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

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

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

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