[flang-commits] [flang] [Flang][OpenMP] DEFAULT(NONE) error checking on implicit references (PR #182214)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Phoebe Linck (phi-bee)

<details>
<summary>Changes</summary>

A variable with an unspecified data-sharing attribute under a DEFAULT(NONE) clause only emits an error if the variable is explicitly referenced in the body of the construct with DEFAULT(NONE).

Ex:

```
!$omp parallel default(none)
!$omp task
a = 1
!$omp end task
!$omp end parallel
end
```
gfortran will error with `‘a’ not specified in enclosing ‘parallel’` on the above. flang doesn't error.

Fix moves the error check to `CreateImplicitSymbols` and checks the variable for a violation in any of its enclosing contexts.

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


2 Files Affected:

- (modified) flang/lib/Semantics/resolve-directives.cpp (+23-21) 
- (modified) flang/test/Semantics/OpenMP/resolve05.f90 (+13) 


``````````diff
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 1fde0d62581e8..45aced325c8eb 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1142,7 +1142,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
   void IssueNonConformanceWarning(llvm::omp::Directive D,
       parser::CharBlock source, unsigned EmitFromVersion);
 
-  void CreateImplicitSymbols(const Symbol *symbol);
+  void CreateImplicitSymbols(const parser::Name &, const Symbol *symbol);
 
   void AddToContextObjectWithExplicitDSA(Symbol &symbol, Symbol::Flag flag) {
     AddToContextObjectWithDSA(symbol, flag);
@@ -2795,7 +2795,8 @@ static bool IsTargetCaptureImplicitlyFirstprivatizeable(const Symbol &symbol,
       symbol.details());
 }
 
-void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
+void OmpAttributeVisitor::CreateImplicitSymbols(
+    const parser::Name &name, const Symbol *symbol) {
   if (!IsPrivatizable(symbol)) {
     return;
   }
@@ -2922,6 +2923,25 @@ void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
     //      Ideally, lowering should be changed and all implicit symbols
     //      should be marked with OmpImplicit.
 
+    if (dirContext.defaultDSA == Symbol::Flag::OmpNone) {
+      if (!symbol->GetUltimate().test(Symbol::Flag::OmpThreadprivate) &&
+          !symbol->test(Symbol::Flag::OmpPrivate)) {
+        if (symbol->GetUltimate().test(Symbol::Flag::CrayPointee)) {
+          std::string crayPtrName{
+              semantics::GetCrayPointer(*symbol).name().ToString()};
+          if (!IsObjectWithDSA(*currScope().FindSymbol(crayPtrName))) {
+            context_.Say(name.source,
+                "The DEFAULT(NONE) clause requires that the Cray Pointer '%s' must be listed in a data-sharing attribute clause"_err_en_US,
+                crayPtrName);
+          }
+        } else {
+          context_.Say(name.source,
+              "The DEFAULT(NONE) clause requires that '%s' must be listed in a data-sharing attribute clause"_err_en_US,
+              symbol->name());
+        }
+      }
+    }
+
     if (dirContext.defaultDSA == Symbol::Flag::OmpPrivate ||
         dirContext.defaultDSA == Symbol::Flag::OmpFirstPrivate ||
         dirContext.defaultDSA == Symbol::Flag::OmpShared) {
@@ -3052,24 +3072,6 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
       if (Symbol * found{currScope().FindSymbol(name.source)}) {
         if (symbol != found) {
           name.symbol = found; // adjust the symbol within region
-        } else if (GetContext().defaultDSA == Symbol::Flag::OmpNone &&
-            !symbol->GetUltimate().test(Symbol::Flag::OmpThreadprivate) &&
-            // Exclude indices of sequential loops that are privatised in
-            // the scope of the parallel region, and not in this scope.
-            // TODO: check whether this should be caught in IsObjectWithDSA
-            !symbol->test(Symbol::Flag::OmpPrivate)) {
-          if (symbol->GetUltimate().test(Symbol::Flag::CrayPointee)) {
-            std::string crayPtrName{
-                semantics::GetCrayPointer(*symbol).name().ToString()};
-            if (!IsObjectWithDSA(*currScope().FindSymbol(crayPtrName)))
-              context_.Say(name.source,
-                  "The DEFAULT(NONE) clause requires that the Cray Pointer '%s' must be listed in a data-sharing attribute clause"_err_en_US,
-                  crayPtrName);
-          } else {
-            context_.Say(name.source,
-                "The DEFAULT(NONE) clause requires that '%s' must be listed in a data-sharing attribute clause"_err_en_US,
-                symbol->name());
-          }
         }
       }
     }
@@ -3123,7 +3125,7 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
     // we don't need to do anything here (i.e. no flags are needed or
     // anything else).
     if (!IsLocalInsideScope(*symbol, currScope())) {
-      CreateImplicitSymbols(symbol);
+      CreateImplicitSymbols(name, symbol);
     }
   } // within OpenMP construct
 }
diff --git a/flang/test/Semantics/OpenMP/resolve05.f90 b/flang/test/Semantics/OpenMP/resolve05.f90
index c4cebb48ac5c2..80e6ce964bba4 100644
--- a/flang/test/Semantics/OpenMP/resolve05.f90
+++ b/flang/test/Semantics/OpenMP/resolve05.f90
@@ -29,8 +29,21 @@ subroutine default_none_seq_loop
   enddo
 end subroutine
 
+! Test that DEFAULT(NONE) error check sees implicit references
+subroutine default_none_nested()
+  integer :: a
+
+  !$omp parallel default(none)
+  !$omp task
+  !ERROR: The DEFAULT(NONE) clause requires that 'a' must be listed in a data-sharing attribute clause
+  a = 1
+  !$omp end task
+  !$omp end parallel
+end subroutine default_none_nested
+
 program mm
   call default_none()
   call default_none_seq_loop()
+  call default_none_nested()
   !TODO: private, firstprivate, shared
 end

``````````

</details>


https://github.com/llvm/llvm-project/pull/182214