https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/180203
Backport 7ccdc06780b05bd8f31c20a9734fca2fbf275d7f Requested by: @kparzysz >From 5ede27625d6eac5656661f3f11eae570c692e260 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Wed, 4 Feb 2026 07:20:20 -0600 Subject: [PATCH] [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 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
