[llvm-branch-commits] [flang] release/22.x: [flang][OpenMP] Leave local automatic variables alone (#178739) (PR #180203)
Cullen Rhodes via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Feb 11 01:58:23 PST 2026
https://github.com/c-rhodes updated https://github.com/llvm/llvm-project/pull/180203
>From 909e38e659e9973e9e969069cc077faeb2ff06ab Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 4 Feb 2026 07:20:20 -0600
Subject: [PATCH 1/2] [flang][OpenMP] Leave local automatic variables alone
(#178739)
There is code in resolve-directives.cpp that tries to apply DSA flags to
symbols encountered inside constructs. This code was written with the
assumption that all such symbols will be declared outside of the
construct.
When a symbol declared in a BLOCK construct nested in a construct was
found, the code would attempt to either privatize or share it in the
enclosing construct (where the symbol didn't exist) leading to trouble.
BLOCK constructs (and thus the possibility of having local variables)
was introduced in F2008.
The first OpenMP spec that considered F2008 was 5.0, where the behavior
of the BLOCK construct was explicitly left unspecified. From OpenMP 5.1
onwards, all local non-static variables are private in the construct
enclosing the declaration. This PR extends this behavior retroactively
to all prior OpenMP versions.
Fixes https://github.com/llvm/llvm-project/issues/178613
(cherry picked from commit 7ccdc06780b05bd8f31c20a9734fca2fbf275d7f)
---
flang/lib/Semantics/resolve-directives.cpp | 73 +++++++++-----
.../test/Semantics/OpenMP/local-variables.f90 | 95 +++++++++++++++++++
2 files changed, 144 insertions(+), 24 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/local-variables.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 6467abf872c16..9f2512713015c 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -388,6 +388,29 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
explicit OmpAttributeVisitor(SemanticsContext &context)
: DirectiveAttributeVisitor(context) {}
+ static bool HasStaticStorageDuration(const Symbol &symbol) {
+ auto &ultSym = symbol.GetUltimate();
+ // Module-scope variable
+ return ultSym.owner().kind() == Scope::Kind::Module ||
+ // Data statement variable
+ ultSym.flags().test(Symbol::Flag::InDataStmt) ||
+ // Save attribute variable
+ ultSym.attrs().test(Attr::SAVE) ||
+ // Referenced in a common block
+ ultSym.flags().test(Symbol::Flag::InCommonBlock);
+ }
+
+ // Recognize symbols that are not created as a part of the OpenMP data-
+ // sharing processing, and that are declared inside of the construct.
+ // These symbols are predetermined private, but they shouldn't be marked
+ // in any special way, because there is nothing to be done for them.
+ // They are not symbols for which private copies need to be created,
+ // they are already themselves private.
+ static bool IsLocalInsideScope(const Symbol &symbol, const Scope &scope) {
+ return symbol.owner() != scope && scope.Contains(symbol.owner()) &&
+ !HasStaticStorageDuration(symbol);
+ }
+
template <typename A> void Walk(const A &x) { parser::Walk(x, *this); }
template <typename A> bool Pre(const A &) { return true; }
template <typename A> void Post(const A &) {}
@@ -2076,6 +2099,9 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
break;
}
}
+ if (IsLocalInsideScope(*iv.symbol, targetIt->scope)) {
+ return;
+ }
// If this symbol already has a data-sharing attribute then there is nothing
// to do here.
if (const Symbol * symbol{iv.symbol}) {
@@ -2400,14 +2426,14 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
}
}
// go through all the nested do-loops and resolve index variables
- const parser::Name *iv{GetLoopIndex(*loop)};
- if (iv) {
- if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) {
- SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA});
- iv->symbol = symbol; // adjust the symbol within region
- AddToContextObjectWithDSA(*symbol, ivDSA);
+ if (const parser::Name *iv{GetLoopIndex(*loop)}) {
+ if (!iv->symbol || !IsLocalInsideScope(*iv->symbol, currScope())) {
+ if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) {
+ SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA});
+ iv->symbol = symbol; // adjust the symbol within region
+ AddToContextObjectWithDSA(*symbol, ivDSA);
+ }
}
-
const auto &block{std::get<parser::Block>(loop->t)};
const auto it{block.begin()};
loop = it != block.end() ? GetDoConstructIf(*it) : nullptr;
@@ -2651,20 +2677,6 @@ static bool IsPrivatizable(const Symbol *sym) {
misc->kind() != MiscDetails::Kind::ConstructName));
}
-static bool IsSymbolStaticStorageDuration(const Symbol &symbol) {
- LLVM_DEBUG(llvm::dbgs() << "IsSymbolStaticStorageDuration(" << symbol.name()
- << "):\n");
- auto ultSym = symbol.GetUltimate();
- // Module-scope variable
- return (ultSym.owner().kind() == Scope::Kind::Module) ||
- // Data statement variable
- (ultSym.flags().test(Symbol::Flag::InDataStmt)) ||
- // Save attribute variable
- (ultSym.attrs().test(Attr::SAVE)) ||
- // Referenced in a common block
- (ultSym.flags().test(Symbol::Flag::InCommonBlock));
-}
-
static bool IsTargetCaptureImplicitlyFirstprivatizeable(const Symbol &symbol,
const Symbol::Flags &dsa, const Symbol::Flags &dataSharingAttributeFlags,
const Symbol::Flags &dataMappingAttributeFlags,
@@ -2823,7 +2835,9 @@ void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
bool targetDir = llvm::omp::allTargetSet.test(dirContext.directive);
bool parallelDir = llvm::omp::topParallelSet.test(dirContext.directive);
bool teamsDir = llvm::omp::allTeamsSet.test(dirContext.directive);
- bool isStaticStorageDuration = IsSymbolStaticStorageDuration(*symbol);
+ bool isStaticStorageDuration = HasStaticStorageDuration(*symbol);
+ LLVM_DEBUG(llvm::dbgs()
+ << "HasStaticStorageDuration(" << symbol->name() << "):\n");
if (dsa.any()) {
if (parallelDir || taskGenDir || teamsDir) {
@@ -2977,7 +2991,8 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
auto *symbol{name.symbol};
if (symbol && WithinConstruct()) {
- if (IsPrivatizable(symbol) && !IsObjectWithDSA(*symbol)) {
+ if (IsPrivatizable(symbol) && !IsObjectWithDSA(*symbol) &&
+ !IsLocalInsideScope(*symbol, currScope())) {
// TODO: create a separate function to go through the rules for
// predetermined, explicitly determined, and implicitly
// determined data-sharing attributes (2.15.1.1).
@@ -3046,7 +3061,17 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
return;
}
- CreateImplicitSymbols(symbol);
+ // We should only create any additional symbols, if the one mentioned
+ // in the source code was declared outside of the construct. This was
+ // always the case before Fortran 2008. F2008 introduced the BLOCK
+ // construct, and allowed local variable declarations.
+ // In OpenMP local (non-static) variables are always private in a given
+ // construct, if they are declared inside the construct. In those cases
+ // we don't need to do anything here (i.e. no flags are needed or
+ // anything else).
+ if (!IsLocalInsideScope(*symbol, currScope())) {
+ CreateImplicitSymbols(symbol);
+ }
} // within OpenMP construct
}
diff --git a/flang/test/Semantics/OpenMP/local-variables.f90 b/flang/test/Semantics/OpenMP/local-variables.f90
new file mode 100644
index 0000000000000..8e7a220319605
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/local-variables.f90
@@ -0,0 +1,95 @@
+!RUN: %flang_fc1 -fdebug-unparse-with-symbols -fopenmp %s | FileCheck %s
+
+!Make sure that the local `bbb`s are their own entities.
+
+!CHECK-LABEL: !DEF: /f00 (Subroutine) Subprogram
+!CHECK-NEXT: subroutine f00
+!CHECK-NEXT: !DEF: /f00/i ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer i
+!CHECK-NEXT: !$omp parallel
+!CHECK-NEXT: block
+!CHECK-NEXT: block
+!CHECK-NEXT: !DEF: /f00/OtherConstruct1/BlockConstruct1/BlockConstruct1/bbb ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer bbb
+!CHECK-NEXT: !REF: /f00/OtherConstruct1/BlockConstruct1/BlockConstruct1/bbb
+!CHECK-NEXT: bbb = 1
+!CHECK-NEXT: end block
+!CHECK-NEXT: block
+!CHECK-NEXT: !DEF: /f00/OtherConstruct1/BlockConstruct1/BlockConstruct2/bbb ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer bbb
+!CHECK-NEXT: !REF: /f00/OtherConstruct1/BlockConstruct1/BlockConstruct2/bbb
+!CHECK-NEXT: bbb = 2
+!CHECK-NEXT: end block
+!CHECK-NEXT: end block
+!CHECK-NEXT: !$omp end parallel
+!CHECK-NEXT: end subroutine
+
+subroutine f00()
+ integer :: i
+ !$omp parallel
+ block
+ block
+ integer :: bbb
+ bbb = 1
+ end block
+ block
+ integer :: bbb
+ bbb = 2
+ end block
+ end block
+ !$omp end parallel
+end subroutine
+
+
+!CHECK-LABEL: !DEF: /f01 (Subroutine) Subprogram
+!CHECK-NEXT: subroutine f01
+!CHECK-NEXT: !DEF: /f01/i ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer i
+!CHECK-NEXT: !DEF: /f01/bbb ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer bbb
+!CHECK-NEXT: !REF: /f01/bbb
+!CHECK-NEXT: bbb = 0
+!CHECK-NEXT: !$omp parallel
+!CHECK-NEXT: block
+!CHECK-NEXT: !DEF: /f01/OtherConstruct1/bbb (OmpShared) HostAssoc INTEGER(4)
+!CHECK-NEXT: bbb = 1234
+!CHECK-NEXT: block
+!CHECK-NEXT: !DEF: /f01/OtherConstruct1/BlockConstruct1/BlockConstruct1/bbb ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer bbb
+!CHECK-NEXT: !REF: /f01/OtherConstruct1/BlockConstruct1/BlockConstruct1/bbb
+!CHECK-NEXT: bbb = 1
+!CHECK-NEXT: end block
+!CHECK-NEXT: block
+!CHECK-NEXT: !DEF: /f01/OtherConstruct1/BlockConstruct1/BlockConstruct2/bbb ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer bbb
+!CHECK-NEXT: !REF: /f01/OtherConstruct1/BlockConstruct1/BlockConstruct2/bbb
+!CHECK-NEXT: bbb = 2
+!CHECK-NEXT: end block
+!CHECK-NEXT: end block
+!CHECK-NEXT: !$omp end parallel
+!CHECK-NEXT: !REF: /f01/bbb
+!CHECK-NEXT: print *, bbb
+!CHECK-NEXT: end subroutine
+
+subroutine f01()
+ integer :: i
+ integer :: bbb
+
+ bbb = 0
+
+ !$omp parallel
+ block
+ bbb = 1234
+ block
+ integer :: bbb
+ bbb = 1
+ end block
+ block
+ integer :: bbb
+ bbb = 2
+ end block
+ end block
+ !$omp end parallel
+
+ print *, bbb
+end subroutine
>From ece1c94778eeb06af23dfc0a3afebb4a1039ad47 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 9 Feb 2026 15:54:01 -0600
Subject: [PATCH 2/2] [flang][OpenMP] Improve locality check when determining
DSA (#180583)
Follow-up to https://github.com/llvm/llvm-project/pull/178739.
The locality check assumed that immediately after the initial symbol
resolution (i.e. prior to the OpenMP code in resolve-directives.cpp),
the scope that owns a given symbol is the scope which owns the symbol's
storage. Turns out that this isn't necessarily true as illustrated by
the included testcase, roughly something like:
```
program main
integer :: j ! host j (storage-owning)
contains
subroutine f
!$omp parallel ! scope that owns j, but j is host-associated
do j = ...
end do
!$omp end parallel
end
end program
```
In such cases, the locality should be checked for the symbol that owns
storage, i.e. a clone of the symbol that is has been privatized or a
symbol that is not host- or use-associated. This is similar to obtaning
the ultimate symbol (i.e. from the end of association chain), except the
chain traversal would stop at a privatized symbol, potentially before
reaching the end.
This fixes a few regressions in the Fujitsu test suite:
Fujitsu/Fortran/0160/Fujitsu-Fortran-0160_0000.test
Fujitsu/Fortran/0160/Fujitsu-Fortran-0160_0012.test
Fujitsu/Fortran/0160/Fujitsu-Fortran-0160_0013.test
Fujitsu/Fortran/0660/Fujitsu-Fortran-0660_0096.test
Fujitsu/Fortran/0660/Fujitsu-Fortran-0660_0097.test
Fujitsu/Fortran/1052/Fujitsu-Fortran-1052_0108.test
Fujitsu/Fortran/1052/Fujitsu-Fortran-1052_0112.test
(cherry picked from commit 0e7ddf395a12e0201ed0ca7131439c2fd355a72a)
---
flang/lib/Semantics/resolve-directives.cpp | 40 +++++++++++++-
...al-variables.f90 => local-variables-1.f90} | 0
.../Semantics/OpenMP/local-variables-2.f90 | 52 +++++++++++++++++++
3 files changed, 90 insertions(+), 2 deletions(-)
rename flang/test/Semantics/OpenMP/{local-variables.f90 => local-variables-1.f90} (100%)
create mode 100644 flang/test/Semantics/OpenMP/local-variables-2.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 9f2512713015c..45592b76dbfa6 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -400,6 +400,37 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
ultSym.flags().test(Symbol::Flag::InCommonBlock);
}
+ static const Symbol &GetStorageOwner(const Symbol &symbol) {
+ static auto getParent = [](const Symbol *s) -> const Symbol * {
+ if (auto *details{s->detailsIf<UseDetails>()}) {
+ return &details->symbol();
+ } else if (auto *details{s->detailsIf<HostAssocDetails>()}) {
+ return &details->symbol();
+ } else {
+ return nullptr;
+ }
+ };
+ static auto isPrivate = [](const Symbol &symbol) {
+ static const Symbol::Flags privatizing{Symbol::Flag::OmpPrivate,
+ Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
+ Symbol::Flag::OmpLinear};
+ return (symbol.flags() & privatizing).any();
+ };
+
+ const Symbol *sym = &symbol;
+ while (true) {
+ if (isPrivate(*sym)) {
+ return *sym;
+ }
+ if (const Symbol *parent{getParent(sym)}) {
+ sym = parent;
+ } else {
+ return *sym;
+ }
+ }
+ llvm_unreachable("Error while looking for storage owning symbol");
+ }
+
// Recognize symbols that are not created as a part of the OpenMP data-
// sharing processing, and that are declared inside of the construct.
// These symbols are predetermined private, but they shouldn't be marked
@@ -407,8 +438,13 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
// They are not symbols for which private copies need to be created,
// they are already themselves private.
static bool IsLocalInsideScope(const Symbol &symbol, const Scope &scope) {
- return symbol.owner() != scope && scope.Contains(symbol.owner()) &&
- !HasStaticStorageDuration(symbol);
+ // A symbol that is marked with a DSA will be cloned in the construct
+ // scope and marked as host-associated. This applies to privatized symbols
+ // as well even though they will have their own storage. They should be
+ // considered local regardless of the status of the original symbol.
+ const Symbol &actual{GetStorageOwner(symbol)};
+ return actual.owner() != scope && scope.Contains(actual.owner()) &&
+ !HasStaticStorageDuration(actual);
}
template <typename A> void Walk(const A &x) { parser::Walk(x, *this); }
diff --git a/flang/test/Semantics/OpenMP/local-variables.f90 b/flang/test/Semantics/OpenMP/local-variables-1.f90
similarity index 100%
rename from flang/test/Semantics/OpenMP/local-variables.f90
rename to flang/test/Semantics/OpenMP/local-variables-1.f90
diff --git a/flang/test/Semantics/OpenMP/local-variables-2.f90 b/flang/test/Semantics/OpenMP/local-variables-2.f90
new file mode 100644
index 0000000000000..1924b5782d16d
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/local-variables-2.f90
@@ -0,0 +1,52 @@
+!RUN: %flang_fc1 -fdebug-unparse-with-symbols -fopenmp -fopenmp-version=60 %s | FileCheck %s
+
+! Shortened version of Fujitsu/Fortran/0160/0160_0000.f90
+! Make sure that j is privatized.
+
+!CHECK-LABEL: !DEF: /MAIN MainProgram
+!CHECK-NEXT: program MAIN
+!CHECK-NEXT: implicit none
+!CHECK-NEXT: !DEF: /MAIN/j ObjectEntity INTEGER(4)
+!CHECK-NEXT: !DEF: /MAIN/k ObjectEntity INTEGER(4)
+!CHECK-NEXT: !DEF: /MAIN/ndim ObjectEntity INTEGER(4)
+!CHECK-NEXT: integer j, k, ndim
+!CHECK-NEXT: !DEF: /MAIN/flux (Subroutine) Subprogram
+!CHECK-NEXT: call flux
+!CHECK-NEXT: contains
+!CHECK-NEXT: !REF: /MAIN/flux
+!CHECK-NEXT: subroutine flux
+!CHECK-NEXT: !$omp parallel
+!CHECK-NEXT: !$omp do
+!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/ndim HostAssoc INTEGER(4)
+!CHECK-NEXT: do k=-1,ndim+1
+!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+!CHECK-NEXT: !REF: /MAIN/flux/OtherConstruct1/OtherConstruct1/ndim
+!CHECK-NEXT: do j=-1,ndim+1
+!CHECK-NEXT: end do
+!CHECK-NEXT: end do
+!CHECK-NEXT: !$omp end do
+!CHECK-NEXT: !$omp end parallel
+!CHECK-NEXT: end subroutine flux
+!CHECK-NEXT: end program MAIN
+
+program main
+ implicit none
+ integer :: j, k, ndim
+
+ call flux()
+
+ contains
+
+ subroutine flux
+ !$omp parallel
+ !$omp do
+ do k = -1, ndim + 1
+ do j = -1, ndim + 1
+ enddo
+ enddo
+ !$omp end do
+ !$omp end parallel
+ end subroutine flux
+
+end program main
More information about the llvm-branch-commits
mailing list