[flang][OpenACC] Relax COMMON block usage restriction in OpenACC directives #162659

eugeneepshteyn · 2025-10-09T13:57:08Z

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

for ACC directives.

llvmbot · 2025-10-09T21:03:35Z

@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

klausler · 2025-10-09T21:23:13Z

flang/lib/Semantics/resolve-directives.cpp

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


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.

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/)?

Yes, I think so.

klausler · 2025-10-09T21:24:00Z

flang/lib/Semantics/resolve-directives.cpp

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


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

…. (Currently the code doesn't fix ACC COMMON problem.)

…This resolves OpenMP regressions with COMMON. Still working on USE-association for COMMON blocks

flang/include/flang/Semantics/scope.h

klausler · 2025-10-13T21:45:06Z

flang/include/flang/Semantics/scope.h


+ /// Find COMMON block in current and surrounding scopes, follow USE
+ /// associations
+ Symbol *FindCommonBlockInScopes(const SourceName &) const;


The name could be more precise so that it's immediately understandable at call sites.

flang/lib/Semantics/symbol.cpp

flang/lib/Semantics/scope.cpp

…ent scope and COMMON with USE details

flang/include/flang/Semantics/scope.h

klausler · 2025-10-16T20:00:17Z

flang/include/flang/Semantics/scope.h

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


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

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

Maybe FindCommonBlockInLexicalScopes or VisibleScopes.

I went with FindCommonBlockInVisibleScopes()

klausler · 2025-10-16T20:00:55Z

flang/include/flang/Semantics/scope.h

 std::list<Scope> children_;
 mapType symbols_;
 mapType commonBlocks_;
+ mapType commonBlockUses_; // USE-assocated COMMON blocks


Is this data member necessary for correctness?

Having the data member makes it more convenient and avoid disturbing the code that assumes that commonBlocks_ only contain symbols with CommonBlockDetails. The previous iteration of this PR had guards around various places to ignore UseDetails and that seemed inelegant.

commonBlocks_ has to exist because their names are in a different namespace ("class" in standard-speak) than normal names. commonBlockUses_ symbols are already in the normal symbol map, so this doesn't seem like necessary new state.

The uses of COMMON block variables are in the normal symbol map, e.g. for this code

module mod_a integer :: a common /mod_a_common/ a end module mod_a module mod_b use mod_a integer :: b common /mod_b_common/ b end module mod_b use mod_b a = 1 b = 2 end

... the symbol table looks like

Module scope: mod_a size=0 alignment=1 sourceRange=66 bytes a, PUBLIC (InCommonBlock) size=4 offset=0: ObjectEntity type: INTEGER(4) mod_a_common size=4 offset=0: CommonBlockDetails alignment=4: a Module scope: mod_b size=0 alignment=1 sourceRange=76 bytes a, PUBLIC (InCommonBlock): Use from a in mod_a b, PUBLIC (InCommonBlock) size=4 offset=0: ObjectEntity type: INTEGER(4) mod_b_common size=4 offset=0: CommonBlockDetails alignment=4: b MainProgram scope: size=0 alignment=1 sourceRange=25 bytes a (InCommonBlock): Use from a in mod_b b (InCommonBlock): Use from b in mod_b

Are you suggesting that mod_a_common use should go into the normal symbol map, together with it's common variable a use?

I think I had a bad assumption there about the common block USE associations being in the normal symbol map. No, they aren't, and they can't be. So the new data member seems necessary.

Well, "necessary" is probably too strong; you could rediscover those common blocks with USE-associated members pretty easily, but doing it just once (at the right time) and saving the results is probably best.

flang/lib/Semantics/resolve-names.cpp

github-actions · 2025-10-17T02:08:31Z

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

klausler · 2025-10-17T19:37:04Z

flang/include/flang/Semantics/scope.h

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


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

klausler · 2025-10-17T19:39:15Z

flang/lib/Semantics/resolve-directives.cpp

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


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

klausler · 2025-10-17T19:40:06Z

flang/lib/Semantics/resolve-directives.cpp

+ if (!name) {
+ return nullptr;
+ }
+ if (auto *cb{


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

klausler · 2025-10-17T19:42:19Z

flang/lib/Semantics/resolve-names.cpp

+ 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(


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.

klausler · 2025-10-17T19:43:30Z

flang/lib/Semantics/resolve-names.cpp

 }
 }
 }
+ // Go through the list of COMMON block symbols in the module scope and add


What happens when the same COMMON block name arrives from multiple modules?

eugeneepshteyn added 8 commits October 6, 2025 05:51

[flang] Draft of the work to look up COMMON declarations in the modules

042d2f6

for ACC directives.

Merge branch 'main' into 2-acc-common

f31c976

Try to handle use association, but didn't work, because no symbol

931b300

Merge branch 'main' into 2-acc-common

0bfb06c

Searching for COMMON definition in top level modules

86cb8b6

Guard against null cb

f462bf5

Unit test

14ce6b6

Merge branch 'llvm:main' into 2-acc-common

8c344c3

eugeneepshteyn marked this pull request as ready for review October 9, 2025 21:02

llvmbot added flang Flang issues not falling into any other category flang:openmp openacc flang:semantics labels Oct 9, 2025

eugeneepshteyn requested review from akuhlens, clementval and klausler October 9, 2025 21:05

klausler reviewed Oct 9, 2025

View reviewed changes

eugeneepshteyn added 12 commits October 10, 2025 13:22

Merge branch '2-acc-common' into 3-acc-common

e00d8f5

Merge branch 'llvm:main' into 3-acc-common

827f8f0

Merge branch 'llvm:main' into 3-acc-common

430660f

Updated Scope::FindCommonBlock() to be similar to Scope::FindSymbol()…

ff637dd

…. (Currently the code doesn't fix ACC COMMON problem.)

clang-format

9e19979

Split FindCommonBlock() into FindCB() and FindCommonBlockInScopes(). …

66028ef

…This resolves OpenMP regressions with COMMON. Still working on USE-association for COMMON blocks

Add USE association details to COMMON blocks

41336f1

Handle the case of redeclared COMMON block

eac75b7

clang-format

553a3c3

Merge branch 'llvm:main' into 3-acc-common

63662d8

Merge branch 'llvm:main' into 2-acc-common

adcd69b

Merge branch '3-acc-common' into 2-acc-common

74de636

eugeneepshteyn requested a review from klausler October 13, 2025 21:40

klausler reviewed Oct 13, 2025

View reviewed changes

eugeneepshteyn added 9 commits October 16, 2025 08:05

Merge branch 'llvm:main' into 3-acc-common

6527159

Code review feedback + separated COMMON lists into COMMON in the curr…

c042379

…ent scope and COMMON with USE details

COMMON block uses renames. Extended test to using module chain

740894f

clang-format

ca82173

Enabled creating of USE symbols from USE symbols of COMMON blocks

2474da0

Merge branch 'llvm:main' into 3-acc-common

68363d2

clang-format

5c84d48

Merge branch 'llvm:main' into 2-acc-common

cc4145c

Merge branch '3-acc-common' into 2-acc-common

06b2a01

eugeneepshteyn requested a review from klausler October 16, 2025 19:56

klausler reviewed Oct 16, 2025

View reviewed changes

eugeneepshteyn added 2 commits October 16, 2025 16:43

Code review feedback

8f802f7

Renamed to FindCommonBlockInVisibleScope()

b711009

eugeneepshteyn added 2 commits October 16, 2025 22:09

clang-format

ee0cdde

Merge branch 'llvm:main' into 2-acc-common

5349a65

klausler reviewed Oct 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][OpenACC] Relax COMMON block usage restriction in OpenACC directives #162659

[flang][OpenACC] Relax COMMON block usage restriction in OpenACC directives #162659

eugeneepshteyn commented Oct 9, 2025

llvmbot commented Oct 9, 2025 •

edited

Loading

klausler Oct 9, 2025

eugeneepshteyn Oct 9, 2025

klausler Oct 9, 2025

klausler Oct 9, 2025

Uh oh!

Uh oh!

klausler Oct 13, 2025

Uh oh!

Uh oh!

Uh oh!

klausler Oct 16, 2025

eugeneepshteyn Oct 16, 2025

akuhlens Oct 16, 2025

eugeneepshteyn Oct 17, 2025

klausler Oct 16, 2025

eugeneepshteyn Oct 16, 2025

klausler Oct 16, 2025

eugeneepshteyn Oct 16, 2025

klausler Oct 16, 2025

klausler Oct 16, 2025

Uh oh!

Uh oh!

github-actions bot commented Oct 17, 2025 •

edited

Loading

klausler Oct 17, 2025

klausler Oct 17, 2025

klausler Oct 17, 2025

klausler Oct 17, 2025

klausler Oct 17, 2025

Labels

4 participants

[flang][OpenACC] Relax COMMON block usage restriction in OpenACC directives #162659

Are you sure you want to change the base?

[flang][OpenACC] Relax COMMON block usage restriction in OpenACC directives #162659

Conversation

eugeneepshteyn commented Oct 9, 2025

llvmbot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels

4 participants

llvmbot commented Oct 9, 2025 •

edited

Loading

github-actions bot commented Oct 17, 2025 •

edited

Loading