[flang][semantics][OpenMP] no privatisation of stmt functions (#106550)

tblah · praveen-g-ctt · web-flow · commit c734d77b9913 · 2024-10-04T10:46:31.000+01:00
OpenMP prohibits privatisation of variables that appear in expressions for statement functions. This is a re-working of an old patch https://reviews.llvm.org/D93213 by @praveen-g-ctt. The old patch couldn't be landed because of ordering concerns. Statement functions are rewritten during parse tree rewriting, but this was done after resolve-directives and so some array expressions were incorrectly identified as statement functions. For this reason **I have opted to re-order the semantics driver so that resolve-directives is run after parse tree rewriting**. Closes #54677 --------- Co-authored-by: Praveen <praveen@compilertree.com>
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
@@ -755,7 +755,7 @@ class Symbol {
       OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective,
       OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction,
       OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined,
-      OmpImplicit, OmpFromStmtFunction);
+      OmpImplicit);
   using Flags = common::EnumSet<Flag, Flag_enumSize>;
 
   const Scope &owner() const { return *owner_; }
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -352,15 +352,6 @@ void DataSharingProcessor::collectSymbols(
                              /*collectSymbols=*/true,
                              /*collectHostAssociatedSymbols=*/true);
 
-  // Add implicitly referenced symbols from statement functions.
-  if (curScope) {
-    for (const auto &sym : curScope->GetSymbols()) {
-      if (sym->test(semantics::Symbol::Flag::OmpFromStmtFunction) &&
-          sym->test(flag))
-        allSymbols.insert(&*sym);
-    }
-  }
-
   llvm::SetVector<const semantics::Symbol *> symbolsInNestedRegions;
   collectSymbolsInNestedRegions(eval, flag, symbolsInNestedRegions);
 
@@ -376,7 +367,8 @@ void DataSharingProcessor::collectSymbols(
     return !semantics::IsProcedure(sym) &&
            !sym.GetUltimate().has<semantics::DerivedTypeDetails>() &&
            !sym.GetUltimate().has<semantics::NamelistDetails>() &&
-           !semantics::IsImpliedDoIndex(sym.GetUltimate());
+           !semantics::IsImpliedDoIndex(sym.GetUltimate()) &&
+           !semantics::IsStmtFunction(sym);
   };
 
   auto shouldCollectSymbol = [&](const semantics::Symbol *sym) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
@@ -719,7 +719,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
   void CheckDataCopyingClause(
       const parser::Name &, const Symbol &, Symbol::Flag);
   void CheckAssocLoopLevel(std::int64_t level, const parser::OmpClause *clause);
-  void CheckObjectInNamelistOrAssociate(
+  void CheckObjectIsPrivatizable(
       const parser::Name &, const Symbol &, Symbol::Flag);
   void CheckSourceLabel(const parser::Label &);
   void CheckLabelContext(const parser::CharBlock, const parser::CharBlock,
@@ -2226,20 +2226,7 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
         return;
     }
 
-    if (auto *stmtFunction{symbol->detailsIf<semantics::SubprogramDetails>()};
-        stmtFunction && stmtFunction->stmtFunction()) {
-      // Each non-dummy argument from a statement function must be handled too,
-      // as if it was explicitly referenced.
-      semantics::UnorderedSymbolSet symbols{
-          CollectSymbols(stmtFunction->stmtFunction().value())};
-      for (const auto &sym : symbols) {
-        if (!IsStmtFunctionDummy(sym) && !IsObjectWithDSA(*sym)) {
-          CreateImplicitSymbols(&*sym, Symbol::Flag::OmpFromStmtFunction);
-        }
-      }
-    } else {
-      CreateImplicitSymbols(symbol);
-    }
+    CreateImplicitSymbols(symbol);
   } // within OpenMP construct
 }
 
@@ -2358,7 +2345,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
                     CheckMultipleAppearances(*name, *symbol, ompFlag);
                   }
                   if (privateDataSharingAttributeFlags.test(ompFlag)) {
-                    CheckObjectInNamelistOrAssociate(*name, *symbol, ompFlag);
+                    CheckObjectIsPrivatizable(*name, *symbol, ompFlag);
                   }
 
                   if (ompFlag == Symbol::Flag::OmpAllocate) {
@@ -2730,7 +2717,7 @@ void OmpAttributeVisitor::CheckDataCopyingClause(
   }
 }
 
-void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
+void OmpAttributeVisitor::CheckObjectIsPrivatizable(
     const parser::Name &name, const Symbol &symbol, Symbol::Flag ompFlag) {
   const auto &ultimateSymbol{symbol.GetUltimate()};
   llvm::StringRef clauseName{"PRIVATE"};
@@ -2751,6 +2738,14 @@ void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
         "Variable '%s' in ASSOCIATE cannot be in a %s clause"_err_en_US,
         name.ToString(), clauseName.str());
   }
+
+  if (stmtFunctionExprSymbols_.find(ultimateSymbol) !=
+      stmtFunctionExprSymbols_.end()) {
+    context_.Say(name.source,
+        "Variable '%s' in statement function expression cannot be in a "
+        "%s clause"_err_en_US,
+        name.ToString(), clauseName.str());
+  }
 }
 
 void OmpAttributeVisitor::CheckSourceLabel(const parser::Label &label) {
diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
@@ -645,8 +645,8 @@ bool Semantics::Perform() {
       CanonicalizeAcc(context_.messages(), program_) &&
       CanonicalizeOmp(context_.messages(), program_) &&
       CanonicalizeCUDA(program_) &&
-      CanonicalizeDirectives(context_.messages(), program_) &&
       PerformStatementSemantics(context_, program_) &&
+      CanonicalizeDirectives(context_.messages(), program_) &&
       ModFileWriter{context_}
           .set_hermeticModuleFileOutput(hermeticModuleFileOutput_)
           .WriteAll();
diff --git a/flang/test/Lower/OpenMP/statement-function.f90 b/flang/test/Lower/OpenMP/statement-function.f90
@@ -1,13 +1,11 @@
-! Test privatization within OpenMP constructs containing statement functions.
+! Test statement functions are not privatised
 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
 
 !CHECK-LABEL: func @_QPtest_implicit_use
 !CHECK:         %[[IEXP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_useEiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 !CHECK:         %[[IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_useEiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK:         omp.parallel private({{.*firstprivate.*}} %[[IEXP]]#0 -> %[[PRIV_IEXP:[^,]+]],
-!CHECK-SAME:                         {{.*firstprivate.*}} %[[IIMP]]#0 -> %[[PRIV_IIMP:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+!CHECK:         omp.parallel private({{.*firstprivate.*}} %[[IEXP]]#0 -> %[[PRIV_IEXP:[^,]+]] : !fir.ref<i32>) {
 !CHECK:           %{{.*}}:2 = hlfir.declare %[[PRIV_IEXP]] {uniq_name = "_QFtest_implicit_useEiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK:           %{{.*}}:2 = hlfir.declare %[[PRIV_IIMP]] {uniq_name = "_QFtest_implicit_useEiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 subroutine test_implicit_use()
   implicit none
   integer :: iexp, iimp
@@ -27,9 +25,7 @@ subroutine test_implicit_use()
 !CHECK:           %[[PRIV_IEXP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 !CHECK:           %[[TEMP0:.*]] = fir.load %[[IEXP]]#0 : !fir.ref<i32>
 !CHECK:           hlfir.assign %[[TEMP0]] to %[[PRIV_IEXP]]#0 : i32, !fir.ref<i32>
-!CHECK:           %[[PRIV_IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK:           %[[TEMP1:.*]] = fir.load %[[IIMP]]#0 : !fir.ref<i32>
-!CHECK:           hlfir.assign %[[TEMP1]] to %[[PRIV_IIMP]]#0 : i32, !fir.ref<i32>
+!CHECK-NOT:       %[[PRIV_IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 subroutine test_implicit_use2()
   implicit none
   integer :: iexp, iimp
diff --git a/flang/test/Semantics/OpenMP/private03.f90 b/flang/test/Semantics/OpenMP/private03.f90
@@ -0,0 +1,39 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 4.5
+! Variables that appear in expressions for statement function definitions
+! may not appear in private, firstprivate or lastprivate clauses.
+
+subroutine stmt_function(temp)
+
+  integer :: i, p, q, r
+  real :: c, f, s, v, t(10)
+  real, intent(in) :: temp
+
+  c(temp) = p * (temp - q) / r
+  f(temp) = q + (temp * r/p)
+  v(temp) = c(temp) + f(temp)/2 - s
+
+  p = 5
+  q = 32
+  r = 9
+
+  !ERROR: Variable 'p' in statement function expression cannot be in a PRIVATE clause
+  !$omp parallel private(p)
+  s = c(temp)
+  !$omp end parallel
+
+  !ERROR: Variable 's' in statement function expression cannot be in a FIRSTPRIVATE clause
+  !$omp parallel firstprivate(s)
+  s = s + f(temp)
+  !$omp end parallel
+
+  !ERROR: Variable 's' in statement function expression cannot be in a LASTPRIVATE clause
+  !$omp parallel do lastprivate(s, t)
+  do i = 1, 10
+  t(i) = v(temp) + i - s
+  end do
+  !$omp end parallel do
+
+  print *, t
+
+end subroutine stmt_function
