[flang][OpenMP] Only privaize pre-determined symbols when defined the evaluation.

[ergawy]

Fixes a regression uncovered by Fujitsu test 0686_0024.f90. In particular, verifies that a pre-determined symbol is only privatized by its defining evaluation (e.g. the loop for which the symbol was marked as pre-determined).

[llvmbot]

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

Author: Kareem Ergawy (ergawy)

Changes

Fixes a regression uncovered by Fujitsu test 0686_0024.f90. In particular, verifies that a pre-determined symbol is only privatized by its defining evaluation (e.g. the loop for which the symbol was marked as pre-determined).

---

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

2 Files Affected:

• (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+2-1)
• (added) flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90 (+35)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index e3f792ee296f7..9d1c730b38edd 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -571,7 +571,8 @@ void DataSharingProcessor::collectSymbols(
     if (collectPreDetermined) {
       // Similar to implicit symbols, collect pre-determined symbols only if
       // they are not defined by a nested `DeclarationConstruct`
-      return !visitor.isSymbolDefineByNestedDeclaration(sym) &&
+      return visitor.isSymbolDefineBy(sym, eval) &&
+             !visitor.isSymbolDefineByNestedDeclaration(sym) &&
              sym->test(semantics::Symbol::Flag::OmpPreDetermined);
     }
 
diff --git a/flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90 b/flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90
new file mode 100644
index 0000000000000..7671073c2598a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90
@@ -0,0 +1,35 @@
+! Fixes a regression uncovered by Fujitsu test 0686_0024.f90. In particular,
+! verifies that a pre-determined symbol is only privatized by its defining
+! evaluation (e.g. the loop for which the symbol was marked as pre-determined).
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+subroutine privatize_predetermined_when_defined_by_eval
+  integer::i,ii
+  integer::j
+
+  !$omp parallel
+    !$omp do lastprivate(ii)
+    do i=1,10
+      do ii=1,10
+      enddo
+    enddo
+
+    !$omp do 
+    do j=1,ii
+    enddo
+  !$omp end parallel
+end subroutine
+
+! Verify that nothing is privatized by the `omp.parallel` op.
+! CHECK: omp.parallel {
+
+! Verify that `i` and `ii` are privatized by the first loop.
+! CHECK:   omp.wsloop private(@{{.*}}ii_private_i32 %{{.*}}#0 -> %{{.*}}, @{{.*}}i_private_i32 %2#0 -> %{{.*}} : {{.*}}) {
+! CHECK:   }
+
+! Verify that `j` is privatized by the second loop.
+! CHECK:   omp.wsloop private(@{{.*}}j_private_i32 %{{.*}}#0 -> %{{.*}} : {{.*}}) {
+! CHECK:   }
+
+! CHECK: }

[llvmbot]

@llvm/pr-subscribers-flang-openmp

Author: Kareem Ergawy (ergawy)

Changes

Fixes a regression uncovered by Fujitsu test 0686_0024.f90. In particular, verifies that a pre-determined symbol is only privatized by its defining evaluation (e.g. the loop for which the symbol was marked as pre-determined).

---

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

2 Files Affected:

• (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+2-1)
• (added) flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90 (+35)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index e3f792ee296f7..9d1c730b38edd 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -571,7 +571,8 @@ void DataSharingProcessor::collectSymbols(
     if (collectPreDetermined) {
       // Similar to implicit symbols, collect pre-determined symbols only if
       // they are not defined by a nested `DeclarationConstruct`
-      return !visitor.isSymbolDefineByNestedDeclaration(sym) &&
+      return visitor.isSymbolDefineBy(sym, eval) &&
+             !visitor.isSymbolDefineByNestedDeclaration(sym) &&
              sym->test(semantics::Symbol::Flag::OmpPreDetermined);
     }
 
diff --git a/flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90 b/flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90
new file mode 100644
index 0000000000000..7671073c2598a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/privatize_predetermined_only_when_defined_by_eval.f90
@@ -0,0 +1,35 @@
+! Fixes a regression uncovered by Fujitsu test 0686_0024.f90. In particular,
+! verifies that a pre-determined symbol is only privatized by its defining
+! evaluation (e.g. the loop for which the symbol was marked as pre-determined).
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+subroutine privatize_predetermined_when_defined_by_eval
+  integer::i,ii
+  integer::j
+
+  !$omp parallel
+    !$omp do lastprivate(ii)
+    do i=1,10
+      do ii=1,10
+      enddo
+    enddo
+
+    !$omp do 
+    do j=1,ii
+    enddo
+  !$omp end parallel
+end subroutine
+
+! Verify that nothing is privatized by the `omp.parallel` op.
+! CHECK: omp.parallel {
+
+! Verify that `i` and `ii` are privatized by the first loop.
+! CHECK:   omp.wsloop private(@{{.*}}ii_private_i32 %{{.*}}#0 -> %{{.*}}, @{{.*}}i_private_i32 %2#0 -> %{{.*}} : {{.*}}) {
+! CHECK:   }
+
+! Verify that `j` is privatized by the second loop.
+! CHECK:   omp.wsloop private(@{{.*}}j_private_i32 %{{.*}}#0 -> %{{.*}} : {{.*}}) {
+! CHECK:   }
+
+! CHECK: }

[tblah]

Thanks for the fix