[flang-commits] [flang] [Flang][OpenMP] Dont add PreDetermined Flag if symbol is privatized already (PR #70931)

[Kiran Chandramohan via flang-commits]

https://github.com/kiranchandramohan updated https://github.com/llvm/llvm-project/pull/70931

>From 66a6f515719aacb1cb6681ef6760e7e27bd1a83d Mon Sep 17 00:00:00 2001
From: Kiran Chandramohan <kiran.chandramohan at arm.com>
Date: Wed, 1 Nov 2023 12:44:41 +0000
Subject: [PATCH 1/2] [Flang][OpenMP] Dont add PreDetermined Flag if symbol is
 privatized already

If the symbol is already privatized due to a user specification then it
is not required to mark it as PreDetermined. This happens if there is
a sequential loop in a parallel region that has the private
specification for the index of the sequential loop.

Fixes #63143
---
 flang/lib/Semantics/resolve-directives.cpp | 14 ++++++++
 flang/test/Semantics/OpenMP/symbol09.f90   | 37 ++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 flang/test/Semantics/OpenMP/symbol09.f90

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2a6523f579f2773..610236914f48dfc 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1518,6 +1518,8 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
 
 void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
     const parser::Name &iv) {
+  // Find the parallel or task generating construct enclosing the
+  // sequential loop.
   auto targetIt{dirContext_.rbegin()};
   for (;; ++targetIt) {
     if (targetIt == dirContext_.rend()) {
@@ -1528,6 +1530,18 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
       break;
     }
   }
+  // If this symbol is already Private or Firstprivate in the enclosing
+  // OpenMP parallel or task then there is nothing to do here.
+  if (auto *symbol{targetIt->scope.FindSymbol(iv.source)}) {
+    if (symbol->owner() == targetIt->scope) {
+      if (symbol->test(Symbol::Flag::OmpPrivate) ||
+          symbol->test(Symbol::Flag::OmpFirstPrivate)) {
+        return;
+      }
+    }
+  }
+  // Otherwise find the symbol and make it Private for the entire enclosing
+  // parallel or task
   if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
     targetIt++;
     // If this object already had a DSA then it is not predetermined
diff --git a/flang/test/Semantics/OpenMP/symbol09.f90 b/flang/test/Semantics/OpenMP/symbol09.f90
new file mode 100644
index 000000000000000..e2250f5c7908aa5
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/symbol09.f90
@@ -0,0 +1,37 @@
+! RUN: %python %S/../test_symbols.py %s %flang_fc1 -fopenmp
+
+! Generic tests
+!   1. subroutine or function calls should not be fixed for DSA or DMA
+
+!DEF: /foo (Function) Subprogram REAL(4)
+!DEF: /foo/rnum ObjectEntity REAL(4)
+function foo(rnum)
+  !REF: /foo/rnum
+  real rnum
+  !REF: /foo/rnum
+  rnum = rnum+1.
+end function foo
+!DEF: /function_call_in_region EXTERNAL (Subroutine) Subprogram
+subroutine function_call_in_region
+  implicit none
+  !DEF: /function_call_in_region/foo (Function) ProcEntity REAL(4)
+  real foo
+  !DEF: /function_call_in_region/a ObjectEntity REAL(4)
+  real :: a = 0.
+  !DEF: /function_call_in_region/b ObjectEntity REAL(4)
+  real :: b = 5.
+  !$omp parallel  default(none) private(a) shared(b)
+  !DEF: /function_call_in_region/OtherConstruct1/a (OmpPrivate) HostAssoc REAL(4)
+  !REF: /function_call_in_region/foo
+  !REF: /function_call_in_region/b
+  a = foo(b)
+  !$omp end parallel
+  !REF: /function_call_in_region/a
+  !REF: /function_call_in_region/b
+  print *, a, b
+end subroutine function_call_in_region
+!DEF: /mm MainProgram
+program mm
+  !REF: /function_call_in_region
+  call function_call_in_region
+end program mm

>From 1316d88efef56f007b8ce8c20914a36edf74b680 Mon Sep 17 00:00:00 2001
From: Kiran Chandramohan <kiran.chandramohan at arm.com>
Date: Mon, 6 Nov 2023 14:30:20 +0000
Subject: [PATCH 2/2] Remove previous code that handled a subset of the cases

---
 flang/lib/Semantics/resolve-directives.cpp | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 610236914f48dfc..ea67eb747e1010e 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1544,10 +1544,7 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
   // parallel or task
   if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
     targetIt++;
-    // If this object already had a DSA then it is not predetermined
-    if (!IsObjectWithDSA(*symbol)) {
-      symbol->set(Symbol::Flag::OmpPreDetermined);
-    }
+    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);