Skip to content

Conversation

eugeneepshteyn
Copy link
Contributor

Unlike OpenMP, OpenACC doesn't require that the COMMON block be defined in the same scope as the directive.

@eugeneepshteyn eugeneepshteyn marked this pull request as ready for review October 9, 2025 21:02
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp openacc flang:semantics labels Oct 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 9, 2025

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-openacc

Author: Eugene Epshteyn (eugeneepshteyn)

Changes

Unlike OpenMP, OpenACC doesn't require that the COMMON block be defined in the same scope as the directive.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/resolve-directives.cpp (+19-13)
  • (added) flang/test/Semantics/OpenACC/acc-common.f90 (+19)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 18fc63814d973..44cfb75e06164 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1695,17 +1695,23 @@ void AccAttributeVisitor::Post(const parser::Name &name) {
 
 Symbol *AccAttributeVisitor::ResolveAccCommonBlockName(
     const parser::Name *name) {
-  if (auto *prev{name
-              ? GetContext().scope.parent().FindCommonBlock(name->source)
-              : nullptr}) {
-    name->symbol = prev;
-    return prev;
-  }
-  // Check if the Common Block is declared in the current scope
-  if (auto *commonBlockSymbol{
-          name ? GetContext().scope.FindCommonBlock(name->source) : nullptr}) {
-    name->symbol = commonBlockSymbol;
-    return commonBlockSymbol;
+  if (!name) {
+    return nullptr;
+  }
+  // Check the local and surrounding scopes first
+  if (auto *cb{GetProgramUnitOrBlockConstructContaining(GetContext().scope)
+              .FindCommonBlock(name->source)}) {
+    name->symbol = cb;
+    return cb;
+  }
+  // Look for COMMON block in the modules
+  for (const Scope &childScope : context_.globalScope().children()) {
+    if (childScope.kind() == Scope::Kind::Module) {
+      if (auto *cb{childScope.FindCommonBlock(name->source)}) {
+        name->symbol = cb;
+        return cb;
+      }
+    }
   }
   return nullptr;
 }
@@ -1755,8 +1761,8 @@ void AccAttributeVisitor::ResolveAccObject(
               }
             } else {
               context_.Say(name.source,
-                  "COMMON block must be declared in the same scoping unit "
-                  "in which the OpenACC directive or clause appears"_err_en_US);
+                  "Could not find COMMON block '%s' used in OpenACC directive"_err_en_US,
+                  name.ToString());
             }
           },
       },
diff --git a/flang/test/Semantics/OpenACC/acc-common.f90 b/flang/test/Semantics/OpenACC/acc-common.f90
new file mode 100644
index 0000000000000..c6142bdb915f6
--- /dev/null
+++ b/flang/test/Semantics/OpenACC/acc-common.f90
@@ -0,0 +1,19 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenacc
+module acc_common_decl
+  implicit none
+  integer a
+  common /a_common/ a
+!$acc declare create (/a_common/)
+  data a/42/
+end module acc_common_decl
+
+program acc_decl_test
+  use acc_common_decl
+  implicit none
+
+  a = 1
+!$acc update device (/a_common/)
+  a = 2
+!ERROR: Could not find COMMON block 'a_common_bad' used in OpenACC directive
+!$acc update device (/a_common_bad/)
+end program

name->symbol = cb;
return cb;
}
// Look for COMMON block in the modules
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this part makes sense? Do you have an example where NVF looks at non-USE-associated modules? You probably just want to look for a USE-associated common block in the enclosing scopes instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would have been very convenient to have a USE-associated common block available, but I don't see one created. With this program

module acc_decl
  implicit none
  integer a
  common /a_common/ a
!$acc declare create (/a_common/)
  data a/42/
end module acc_decl

program acc_decl_test
  use acc_decl
  implicit none

  a = 1
!$acc update device (/a_common/)
  a = 2
end program

... and -fdebug-dump-symbols I only see

  MainProgram scope: ACC_DECL_TEST size=0 alignment=1 sourceRange=72 bytes
    a (InDataStmt, InCommonBlock): Use from a in acc_decl

Perhaps this is the problem? Perhaps we should be creating USE-associated symbol from its use in !$acc update device (/a_common/)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so.

if (!name) {
return nullptr;
}
// Check the local and surrounding scopes first
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only check the innermost BLOCK construct or subprogram, not any containing construct or host subprogram.


// Find COMMON block in current and surrounding scopes, follow USE
// associations
Symbol *FindCommonBlockInSurroundingScopes(const SourceName &) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name isn't consistent with the comment; will this member function detect a common block in this scope, or only in its parents?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate suggestion for a better name, otherwise it'll be FindCommonBlockInCurrentAndSurroundingScopes() :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe FindCommonBlockInLexicalScopes or VisibleScopes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with FindCommonBlockInVisibleScopes()

Copy link

github-actions bot commented Oct 17, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Symbol &MakeCommonBlock(SourceName, SourceName location);
Symbol *FindCommonBlock(const SourceName &) const;
bool AddCommonBlockUse(const SourceName &name, Symbol &cbSymbol);
mapType &commonBlockUses() { return commonBlockUses_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a const accessor as well, and moving this up to be with the other accessors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

name ? GetContext().scope.FindCommonBlock(name->source) : nullptr}) {
name->symbol = commonBlockSymbol;
return commonBlockSymbol;
if (!name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be more clear as if (name) { ..., I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if (!name) {
return nullptr;
}
if (auto *cb{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use const for pointers and references that are auto, and maybe const Symbol * would be even clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it Symbol *, but cannot make it const, because it's assigned to non-const below.

if (!currScope().FindCommonBlockInVisibleScopes(name)) {
// Make a symbol, but don't add it to the Scope, since it needs to
// be added to the USE-associated COMMON blocks
Symbol *localCB{&currScope().MakeSymbol(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable doesn't have to be a pointer. MakeSymbol returns a reference, and you can declare localCB to be a reference that you can then just pass directly to AddCommonBlockUse as such.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the update.

@eugeneepshteyn eugeneepshteyn merged commit 4cb91e7 into llvm:main Oct 21, 2025
10 checks passed
@eugeneepshteyn eugeneepshteyn deleted the 2-acc-common branch October 21, 2025 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:openmp flang:semantics flang Flang issues not falling into any other category openacc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants