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..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, !fir.ref) + ! 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, !fir.ref) + ! 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, !fir.ref, !fir.ref) + ! 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, !fir.ref) + ! 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) ! CHECK-NEXT: omp.simd - ! CHECK-SAME: private(@[[PRIV_BOX_SYM:.*]] %{{.*}} -> %[[PRIV_BOX:.*]], @[[PRIV_IVAR_SYM:.*]] %{{.*}} -> %[[PRIV_IVAR:.*]] : !fir.ref>>, !fir.ref) ! 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) + ! CHECK-SAME: private(@[[FIRSTPRIVATE_A_SYM:.*]] %{{.*}} -> %[[FIRSTPRIVATE_A:.*]], @[[PRIVATE_I_SYM:.*]] %{{.*}} -> %[[PRIVATE_I:.*]] : !fir.ref, !fir.ref) ! CHECK-NEXT: omp.simd - ! CHECK-SAME: private(@[[PRIVATE_A_SYM:.*]] %{{.*}} -> %[[PRIVATE_A:.*]], @[[PRIVATE_I_SYM:.*]] %{{.*}} -> %[[PRIVATE_I:.*]] : !fir.ref, !fir.ref) !$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