[flang][openmp] Don't mark loop variables with explicit DSA as predetermined

[DavidTruby]

This patch fixes a bug where loop variables are always marked as predetermined
even when they have an explicit data sharing attribute specified.

[llvmbot]

@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-fir-hlfir

Author: David Truby (DavidTruby)

Changes

This patch fixes a bug where loop variables are always marked as predetermined
even when they have an explicit data sharing attribute specified.

---

Full diff: https://github.com/llvm/llvm-project/pull/68723.diff

4 Files Affected:

• (modified) flang/lib/Semantics/resolve-directives.cpp (+4-1)
• (modified) flang/test/Lower/OpenMP/FIR/parallel-private-clause-fixes.f90 (+3-3)
• (modified) flang/test/Semantics/OpenMP/do05-positivecase.f90 (+9)
• (modified) flang/test/Semantics/OpenMP/symbol08.f90 (+2-2)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 794a78408863cb3..7d7f1ee2d24593a 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1542,7 +1542,10 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
   }
   if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
     targetIt++;
-    symbol->set(Symbol::Flag::OmpPreDetermined);
+    // If this object already had a DSA then it is not predetermined
+    if (!IsObjectWithDSA(*symbol)) {
+      symbol->set(Symbol::Flag::OmpPreDetermined);
+    }
     iv.symbol = symbol; // adjust the symbol within region
     for (auto it{dirContext_.rbegin()}; it != targetIt; ++it) {
       AddToContextObjectWithDSA(*symbol, Symbol::Flag::OmpPrivate, *it);
diff --git a/flang/test/Lower/OpenMP/FIR/parallel-private-clause-fixes.f90 b/flang/test/Lower/OpenMP/FIR/parallel-private-clause-fixes.f90
index 3152f9c44d0c64a..beb94bfc91a151a 100644
--- a/flang/test/Lower/OpenMP/FIR/parallel-private-clause-fixes.f90
+++ b/flang/test/Lower/OpenMP/FIR/parallel-private-clause-fixes.f90
@@ -7,9 +7,9 @@
 ! CHECK:         %[[VAL_1:.*]] = fir.alloca i32 {bindc_name = "j", uniq_name = "_QFmultiple_private_fixEj"}
 ! CHECK:         %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFmultiple_private_fixEx"}
 ! CHECK:         omp.parallel {
-! CHECK:           %[[PRIV_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned
-! CHECK:           %[[PRIV_I:.*]] = fir.alloca i32 {adapt.valuebyref, pinned
-! CHECK:           %[[PRIV_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned
+! CHECK-DAG:           %[[PRIV_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned
+! CHECK-DAG:           %[[PRIV_I:.*]] = fir.alloca i32 {adapt.valuebyref, pinned
+! CHECK-DAG:           %[[PRIV_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned
 ! CHECK:           %[[ONE:.*]] = arith.constant 1 : i32
 ! CHECK:           %[[VAL_3:.*]] = fir.load %[[VAL_4:.*]] : !fir.ref<i32>
 ! CHECK:           %[[VAL_5:.*]] = arith.constant 1 : i32
diff --git a/flang/test/Semantics/OpenMP/do05-positivecase.f90 b/flang/test/Semantics/OpenMP/do05-positivecase.f90
index b1d97b44567eb5a..4e02235f58a1a48 100644
--- a/flang/test/Semantics/OpenMP/do05-positivecase.f90
+++ b/flang/test/Semantics/OpenMP/do05-positivecase.f90
@@ -33,4 +33,13 @@ program omp_do
   !$omp end do
   !$omp end parallel
 
+  !$omp parallel private(i)
+  !DEF: /omp_do/OtherConstruct3/i (OmpPrivate) HostAssoc INTEGER(4)
+  do i=1,10
+     !$omp single
+     print *, "hello"
+     !$omp end single
+  end do
+  !$omp end parallel
+
 end program omp_do
diff --git a/flang/test/Semantics/OpenMP/symbol08.f90 b/flang/test/Semantics/OpenMP/symbol08.f90
index b80c94a9f189ae7..50f34b736cdb775 100644
--- a/flang/test/Semantics/OpenMP/symbol08.f90
+++ b/flang/test/Semantics/OpenMP/symbol08.f90
@@ -67,7 +67,7 @@ subroutine test_pardo
     do j=6,10
    !REF: /test_pardo/a
    a(1,1,1) = 0.
-   !DEF: /test_pardo/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+   !DEF: /test_pardo/OtherConstruct1/k (OmpPrivate) HostAssoc INTEGER(4)
    do k=11,15
     !REF: /test_pardo/a
     !REF: /test_pardo/OtherConstruct1/k
@@ -91,7 +91,7 @@ subroutine test_taskloop
 !$omp taskloop  private(j)
  !DEF: /test_taskloop/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
  do i=1,5
-  !DEF: /test_taskloop/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  !DEF: /test_taskloop/OtherConstruct1/j (OmpPrivate) HostAssoc INTEGER(4)
   !REF: /test_taskloop/OtherConstruct1/i
   do j=1,i
    !REF: /test_taskloop/a

[kiranchandramohan]

Thanks for the fix. LGTM.

[antmox]

Hello,
It looks like this commit broke several flang bots:
https://lab.llvm.org/buildbot/#/builders/268/builds/142
https://lab.llvm.org/buildbot/#/builders/160/builds/25655
https://lab.llvm.org/buildbot/#/builders/191/builds/24568
https://lab.llvm.org/buildbot/#/builders/266/builds/1609
https://lab.llvm.org/buildbot/#/builders/173/builds/24307
https://lab.llvm.org/buildbot/#/builders/175/builds/37511
https://lab.llvm.org/buildbot/#/builders/181/builds/24239
https://lab.llvm.org/buildbot/#/builders/177/builds/24336
Could you please look at this ?

[kiranchandramohan]

Sure, will have a look.

[kiranchandramohan]

Should be fixed by the commit mentioned above.