[flang][OpenMP] Explicitly set Shared DSA in symbols #142154
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-fir-hlfir Author: Leandro Lupori (luporl) ChangesBefore this change, OmpShared was not always set in shared symbols. Because of the host association symbols with no flags mentioned Besides that, some semantic checks need to know if a DSA clause With the changes above, AddToContextObjectWithDSA() and the symbol Some debug messages were also added, with the "omp" DEBUG_TYPE, to Fixes #130533 Patch is 55.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142154.diff 26 Files Affected:
diff --git a/flang/include/flang/Semantics/openmp-dsa.h b/flang/include/flang/Semantics/openmp-dsa.h
new file mode 100644
index 0000000000000..4b94a679f29ef
--- /dev/null
+++ b/flang/include/flang/Semantics/openmp-dsa.h
@@ -0,0 +1,20 @@
+//===-- include/flang/Semantics/openmp-dsa.h --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_SEMANTICS_OPENMP_DSA_H_
+#define FORTRAN_SEMANTICS_OPENMP_DSA_H_
+
+#include "flang/Semantics/symbol.h"
+
+namespace Fortran::semantics {
+
+Symbol::Flags GetSymbolDSA(const Symbol &symbol);
+
+} // namespace Fortran::semantics
+
+#endif // FORTRAN_SEMANTICS_OPENMP_DSA_H_
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index 4cded64d170cd..59920e08cc926 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -785,8 +785,8 @@ class Symbol {
OmpAllocate, OmpDeclarativeAllocateDirective,
OmpExecutableAllocateDirective, OmpDeclareSimd, OmpDeclareTarget,
OmpThreadprivate, OmpDeclareReduction, OmpFlushed, OmpCriticalLock,
- OmpIfSpecified, OmpNone, OmpPreDetermined, OmpImplicit, OmpDependObject,
- OmpInclusiveScan, OmpExclusiveScan, OmpInScanReduction);
+ OmpIfSpecified, OmpNone, OmpPreDetermined, OmpExplicit, OmpImplicit,
+ OmpDependObject, OmpInclusiveScan, OmpExclusiveScan, OmpInScanReduction);
using Flags = common::EnumSet<Flag, Flag_enumSize>;
const Scope &owner() const { return *owner_; }
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index c9e91cf3e8042..86d5e0d37bc38 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -58,6 +58,7 @@
#include "flang/Optimizer/Transforms/Passes.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Runtime/iostat-consts.h"
+#include "flang/Semantics/openmp-dsa.h"
#include "flang/Semantics/runtime-type-info.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
@@ -1385,7 +1386,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
if (isUnordered || sym.has<Fortran::semantics::HostAssocDetails>() ||
sym.has<Fortran::semantics::UseDetails>()) {
if (!shallowLookupSymbol(sym) &&
- !sym.test(Fortran::semantics::Symbol::Flag::OmpShared)) {
+ !GetSymbolDSA(sym).test(
+ Fortran::semantics::Symbol::Flag::OmpShared)) {
// Do concurrent loop variables are not mapped yet since they are local
// to the Do concurrent scope (same for OpenMP loops).
mlir::OpBuilder::InsertPoint insPt = builder->saveInsertionPoint();
diff --git a/flang/lib/Semantics/CMakeLists.txt b/flang/lib/Semantics/CMakeLists.txt
index bd8cc47365f06..18c89587843a9 100644
--- a/flang/lib/Semantics/CMakeLists.txt
+++ b/flang/lib/Semantics/CMakeLists.txt
@@ -32,6 +32,7 @@ add_flang_library(FortranSemantics
dump-expr.cpp
expression.cpp
mod-file.cpp
+ openmp-dsa.cpp
openmp-modifiers.cpp
pointer-assignment.cpp
program-tree.cpp
diff --git a/flang/lib/Semantics/openmp-dsa.cpp b/flang/lib/Semantics/openmp-dsa.cpp
new file mode 100644
index 0000000000000..48aa36febe5c5
--- /dev/null
+++ b/flang/lib/Semantics/openmp-dsa.cpp
@@ -0,0 +1,29 @@
+//===-- flang/lib/Semantics/openmp-dsa.cpp ----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "flang/Semantics/openmp-dsa.h"
+
+namespace Fortran::semantics {
+
+Symbol::Flags GetSymbolDSA(const Symbol &symbol) {
+ Symbol::Flags dsaFlags{Symbol::Flag::OmpPrivate,
+ Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
+ Symbol::Flag::OmpShared, Symbol::Flag::OmpLinear,
+ Symbol::Flag::OmpReduction};
+ Symbol::Flags dsa{symbol.flags() & dsaFlags};
+ if (dsa.any()) {
+ return dsa;
+ }
+ // If no DSA are set use those from the host associated symbol, if any.
+ if (const auto *details{symbol.detailsIf<HostAssocDetails>()}) {
+ return GetSymbolDSA(details->symbol());
+ }
+ return {};
+}
+
+} // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 9fa7bc8964854..e604bca4213c8 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -19,9 +19,11 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Parser/tools.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-dsa.h"
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "llvm/Support/Debug.h"
#include <list>
#include <map>
#include <sstream>
@@ -111,10 +113,9 @@ template <typename T> class DirectiveAttributeVisitor {
const parser::Name *GetLoopIndex(const parser::DoConstruct &);
const parser::DoConstruct *GetDoConstructIf(
const parser::ExecutionPartConstruct &);
- Symbol *DeclareNewPrivateAccessEntity(const Symbol &, Symbol::Flag, Scope &);
- Symbol *DeclarePrivateAccessEntity(
- const parser::Name &, Symbol::Flag, Scope &);
- Symbol *DeclarePrivateAccessEntity(Symbol &, Symbol::Flag, Scope &);
+ Symbol *DeclareNewAccessEntity(const Symbol &, Symbol::Flag, Scope &);
+ Symbol *DeclareAccessEntity(const parser::Name &, Symbol::Flag, Scope &);
+ Symbol *DeclareAccessEntity(Symbol &, Symbol::Flag, Scope &);
Symbol *DeclareOrMarkOtherAccessEntity(const parser::Name &, Symbol::Flag);
UnorderedSymbolSet dataSharingAttributeObjects_; // on one directive
@@ -749,10 +750,11 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
Symbol::Flags ompFlagsRequireNewSymbol{Symbol::Flag::OmpPrivate,
Symbol::Flag::OmpLinear, Symbol::Flag::OmpFirstPrivate,
- Symbol::Flag::OmpLastPrivate, Symbol::Flag::OmpReduction,
- Symbol::Flag::OmpCriticalLock, Symbol::Flag::OmpCopyIn,
- Symbol::Flag::OmpUseDevicePtr, Symbol::Flag::OmpUseDeviceAddr,
- Symbol::Flag::OmpIsDevicePtr, Symbol::Flag::OmpHasDeviceAddr};
+ Symbol::Flag::OmpLastPrivate, Symbol::Flag::OmpShared,
+ Symbol::Flag::OmpReduction, Symbol::Flag::OmpCriticalLock,
+ Symbol::Flag::OmpCopyIn, Symbol::Flag::OmpUseDevicePtr,
+ Symbol::Flag::OmpUseDeviceAddr, Symbol::Flag::OmpIsDevicePtr,
+ Symbol::Flag::OmpHasDeviceAddr};
Symbol::Flags ompFlagsRequireMark{Symbol::Flag::OmpThreadprivate,
Symbol::Flag::OmpDeclareTarget, Symbol::Flag::OmpExclusiveScan,
@@ -829,8 +831,24 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void IssueNonConformanceWarning(
llvm::omp::Directive D, parser::CharBlock source);
- void CreateImplicitSymbols(
- const Symbol *symbol, std::optional<Symbol::Flag> setFlag = std::nullopt);
+ void CreateImplicitSymbols(const Symbol *symbol);
+
+ void AddToContextObjectWithExplicitDSA(Symbol &symbol, Symbol::Flag flag) {
+ AddToContextObjectWithDSA(symbol, flag);
+ if (dataSharingAttributeFlags.test(flag)) {
+ symbol.set(Symbol::Flag::OmpExplicit);
+ }
+ }
+
+ // Clear any previous data-sharing attribute flags and set the new ones.
+ // Needed when setting PreDetermined DSAs, that take precedence over
+ // Implicit ones.
+ void SetSymbolDSA(Symbol &symbol, Symbol::Flags flags) {
+ symbol.flags() &= ~(dataSharingAttributeFlags |
+ Symbol::Flags{Symbol::Flag::OmpExplicit, Symbol::Flag::OmpImplicit,
+ Symbol::Flag::OmpPreDetermined});
+ symbol.flags() |= flags;
+ }
};
template <typename T>
@@ -867,7 +885,7 @@ const parser::DoConstruct *DirectiveAttributeVisitor<T>::GetDoConstructIf(
}
template <typename T>
-Symbol *DirectiveAttributeVisitor<T>::DeclareNewPrivateAccessEntity(
+Symbol *DirectiveAttributeVisitor<T>::DeclareNewAccessEntity(
const Symbol &object, Symbol::Flag flag, Scope &scope) {
assert(object.owner() != currScope());
auto &symbol{MakeAssocSymbol(object.name(), object, scope)};
@@ -880,20 +898,20 @@ Symbol *DirectiveAttributeVisitor<T>::DeclareNewPrivateAccessEntity(
}
template <typename T>
-Symbol *DirectiveAttributeVisitor<T>::DeclarePrivateAccessEntity(
+Symbol *DirectiveAttributeVisitor<T>::DeclareAccessEntity(
const parser::Name &name, Symbol::Flag flag, Scope &scope) {
if (!name.symbol) {
return nullptr; // not resolved by Name Resolution step, do nothing
}
- name.symbol = DeclarePrivateAccessEntity(*name.symbol, flag, scope);
+ name.symbol = DeclareAccessEntity(*name.symbol, flag, scope);
return name.symbol;
}
template <typename T>
-Symbol *DirectiveAttributeVisitor<T>::DeclarePrivateAccessEntity(
+Symbol *DirectiveAttributeVisitor<T>::DeclareAccessEntity(
Symbol &object, Symbol::Flag flag, Scope &scope) {
if (object.owner() != currScope()) {
- return DeclareNewPrivateAccessEntity(object, flag, scope);
+ return DeclareNewAccessEntity(object, flag, scope);
} else {
object.set(flag);
return &object;
@@ -1600,6 +1618,20 @@ void AccAttributeVisitor::CheckMultipleAppearances(
}
}
+#ifndef NDEBUG
+
+#define DEBUG_TYPE "omp"
+
+static llvm::raw_ostream &operator<<(
+ llvm::raw_ostream &os, const Symbol::Flags &flags);
+
+namespace dbg {
+static void DumpAssocSymbols(llvm::raw_ostream &os, const Symbol &sym);
+static std::string ScopeSourcePos(const Fortran::semantics::Scope &scope);
+} // namespace dbg
+
+#endif
+
bool OmpAttributeVisitor::Pre(const parser::OpenMPBlockConstruct &x) {
const auto &beginBlockDir{std::get<parser::OmpBeginBlockDirective>(x.t)};
const auto &beginDir{std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
@@ -1792,12 +1824,12 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
}
}
}
- // If this symbol is already Private or Firstprivate in the enclosing
- // OpenMP parallel or task then there is nothing to do here.
+ // If this symbol already has an explicit data-sharing attribute in the
+ // enclosing OpenMP parallel or task then there is nothing to do here.
if (auto *symbol{targetIt->scope.FindSymbol(iv.source)}) {
if (symbol->owner() == targetIt->scope) {
- if (symbol->test(Symbol::Flag::OmpPrivate) ||
- symbol->test(Symbol::Flag::OmpFirstPrivate)) {
+ if (symbol->test(Symbol::Flag::OmpExplicit) &&
+ (symbol->flags() & dataSharingAttributeFlags).any()) {
return;
}
}
@@ -1806,7 +1838,8 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
// parallel or task
if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
targetIt++;
- symbol->set(Symbol::Flag::OmpPreDetermined);
+ SetSymbolDSA(
+ *symbol, {Symbol::Flag::OmpPreDetermined, Symbol::Flag::OmpPrivate});
iv.symbol = symbol; // adjust the symbol within region
for (auto it{dirContext_.rbegin()}; it != targetIt; ++it) {
AddToContextObjectWithDSA(*symbol, Symbol::Flag::OmpPrivate, *it);
@@ -1918,7 +1951,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
const parser::Name *iv{GetLoopIndex(*loop)};
if (iv) {
if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) {
- symbol->set(Symbol::Flag::OmpPreDetermined);
+ SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA});
iv->symbol = symbol; // adjust the symbol within region
AddToContextObjectWithDSA(*symbol, ivDSA);
}
@@ -2178,42 +2211,48 @@ static bool IsPrivatizable(const Symbol *sym) {
misc->kind() != MiscDetails::Kind::ConstructName));
}
-void OmpAttributeVisitor::CreateImplicitSymbols(
- const Symbol *symbol, std::optional<Symbol::Flag> setFlag) {
+void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
if (!IsPrivatizable(symbol)) {
return;
}
+ LLVM_DEBUG(llvm::dbgs() << "CreateImplicitSymbols: " << *symbol << '\n');
+
// Implicitly determined DSAs
// OMP 5.2 5.1.1 - Variables Referenced in a Construct
Symbol *lastDeclSymbol = nullptr;
- std::optional<Symbol::Flag> prevDSA;
+ Symbol::Flags prevDSA;
for (int dirDepth{0}; dirDepth < (int)dirContext_.size(); ++dirDepth) {
DirContext &dirContext = dirContext_[dirDepth];
- std::optional<Symbol::Flag> dsa;
+ Symbol::Flags dsa;
- for (auto symMap : dirContext.objectWithDSA) {
- // if the `symbol` already has a data-sharing attribute
- if (symMap.first->name() == symbol->name()) {
- dsa = symMap.second;
- break;
+ Scope &scope{context_.FindScope(dirContext.directiveSource)};
+ auto it{scope.find(symbol->name())};
+ if (it != scope.end()) {
+ // There is already a symbol in the current scope, use its DSA.
+ dsa = GetSymbolDSA(*it->second);
+ } else {
+ for (auto symMap : dirContext.objectWithDSA) {
+ if (symMap.first->name() == symbol->name()) {
+ // `symbol` already has a data-sharing attribute in the current
+ // context, use it.
+ dsa.set(symMap.second);
+ break;
+ }
}
}
// When handling each implicit rule for a given symbol, one of the
- // following 3 actions may be taken:
- // 1. Declare a new private symbol.
- // 2. Create a new association symbol with no flags, that will represent
- // a shared symbol in the current scope. Note that symbols without
- // any private flags are considered as shared.
- // 3. Use the last declared private symbol, by inserting a new symbol
- // in the scope being processed, associated with it.
- // If no private symbol was declared previously, then no association
- // is needed and the symbol from the enclosing scope will be
- // inherited by the current one.
+ // following actions may be taken:
+ // 1. Declare a new private or shared symbol.
+ // 2. Use the last declared symbol, by inserting a new symbol in the
+ // scope being processed, associated with it.
+ // If no symbol was declared previously, then no association is needed
+ // and the symbol from the enclosing scope will be inherited by the
+ // current one.
//
// Because of how symbols are collected in lowering, not inserting a new
- // symbol in the last case could lead to the conclusion that a symbol
+ // symbol in the second case could lead to the conclusion that a symbol
// from an enclosing construct was declared in the current construct,
// which would result in wrong privatization code being generated.
// Consider the following example:
@@ -2231,46 +2270,71 @@ void OmpAttributeVisitor::CreateImplicitSymbols(
// it would have the private flag set.
// This would make x appear to be defined in p2, causing it to be
// privatized in p2 and its privatization in p1 to be skipped.
- auto makePrivateSymbol = [&](Symbol::Flag flag) {
+ auto makeSymbol = [&](Symbol::Flags flags) {
const Symbol *hostSymbol =
lastDeclSymbol ? lastDeclSymbol : &symbol->GetUltimate();
- lastDeclSymbol = DeclareNewPrivateAccessEntity(
+ assert(flags.LeastElement());
+ Symbol::Flag flag = *flags.LeastElement();
+ lastDeclSymbol = DeclareNewAccessEntity(
*hostSymbol, flag, context_.FindScope(dirContext.directiveSource));
- if (setFlag) {
- lastDeclSymbol->set(*setFlag);
- }
+ lastDeclSymbol->flags() |= flags;
return lastDeclSymbol;
};
- auto makeSharedSymbol = [&](std::optional<Symbol::Flag> flag = {}) {
- const Symbol *hostSymbol =
- lastDeclSymbol ? lastDeclSymbol : &symbol->GetUltimate();
- Symbol &assocSymbol = MakeAssocSymbol(symbol->name(), *hostSymbol,
- context_.FindScope(dirContext.directiveSource));
- if (flag) {
- assocSymbol.set(*flag);
- }
- };
auto useLastDeclSymbol = [&]() {
if (lastDeclSymbol) {
- makeSharedSymbol();
+ const Symbol *hostSymbol =
+ lastDeclSymbol ? lastDeclSymbol : &symbol->GetUltimate();
+ MakeAssocSymbol(symbol->name(), *hostSymbol,
+ context_.FindScope(dirContext.directiveSource));
}
};
+#ifndef NDEBUG
+ auto printImplicitRule = [&](const char *id) {
+ LLVM_DEBUG(llvm::dbgs() << "\t" << id << ": dsa: " << dsa << '\n');
+ LLVM_DEBUG(
+ llvm::dbgs() << "\t\tScope: " << dbg::ScopeSourcePos(scope) << '\n');
+ };
+#define PRINT_IMPLICIT_RULE(id) printImplicitRule(id)
+#else
+#define PRINT_IMPLICIT_RULE(id)
+#endif
+
bool taskGenDir = llvm::omp::taskGeneratingSet.test(dirContext.directive);
bool targetDir = llvm::omp::allTargetSet.test(dirContext.directive);
bool parallelDir = llvm::omp::allParallelSet.test(dirContext.directive);
bool teamsDir = llvm::omp::allTeamsSet.test(dirContext.directive);
- if (dsa.has_value()) {
- if (dsa.value() == Symbol::Flag::OmpShared &&
- (parallelDir || taskGenDir || teamsDir)) {
- makeSharedSymbol(Symbol::Flag::OmpShared);
+ if (dsa.any()) {
+ if (parallelDir || taskGenDir || teamsDir) {
+ Symbol *prevDeclSymbol{lastDeclSymbol};
+ // NOTE As `dsa` will match that of the symbol in the current scope
+ // (if any), we won't override the DSA of any existing symbol.
+ if ((dsa & dataSharingAttributeFlags).any()) {
+ makeSymbol(dsa);
+ }
+ // Fix host association of explicit symbols, as they can be created
+ // before implicit ones in enclosing scope.
+ if (prevDeclSymbol && prevDeclSymbol != lastDeclSymbol &&
+ lastDeclSymbol->test(Symbol::Flag::OmpExplicit)) {
+ const auto *hostAssoc{lastDeclSymbol->detailsIf<HostAssocDetails>()};
+ if (hostAssoc && hostAssoc->symbol() != *prevDeclSymbol) {
+ lastDeclSymbol->set_details(HostAssocDetails{*prevDeclSymbol});
+ }
+ }
}
- // Private symbols will have been declared already.
prevDSA = dsa;
+ PRINT_IMPLICIT_RULE("0) already has DSA");
continue;
}
+ // NOTE Because of how lowering uses OmpImplicit flag, we can only set it
+ // for symbols with private DSA.
+ // Also, as the default clause is handled separately in lowering,
+ // don't mark its symbols with OmpImplicit either.
+ // Ideally, lowering should be changed and all implicit symbols
+ // should be marked with OmpImplicit.
+
if (dirContext.defaultDSA == Symbol::Flag::OmpPrivate ||
dirContext.defaultDSA == Symbol::Flag::OmpFirstPrivate ||
dirContext.defaultDSA == Symbol::Flag::OmpShared) {
@@ -2279,33 +2343,34 @@ void OmpAttributeVisitor::CreateImplicitSymbols(
if (!parallelDir && !taskGenDir && !teamsDir) {
return;
}
- if (dirContext.defaultDSA != Symbol::Flag::OmpShared) {
- makePrivateSymbol(dirContext.defaultDSA);
- } else {
- makeSharedSymbol();
- }
- dsa = dirContext.defaultDSA;
+ dsa = {dirContext.defaultDSA};
+ makeSymbol(dsa);
+ PRINT_IMPLICIT_RULE("1) default");
} else if (parallelDir) {
// 2) parallel -> shared
- makeSharedSymbol();
- dsa = Symbol::Flag::OmpShared;
+ dsa = {Symbol::Flag::OmpShared};
+ makeSymbol(dsa);
+ PRINT_IMPLICIT_RULE("2) parallel");
} else if (!taskGenDir && !targetDir) {
// 3) enclosing context
- useLastDeclSymbol();
dsa = prevDSA;
+ useLastDeclSymbol();
+ PRINT_IMPLICIT_RULE("3) enclosing context");
} else if (targetDir) {
// TODO 4) not mapped target variable -> firstprivate
dsa = prevDSA;
} else if (taskGenDir) {
// TODO 5) dummy arg in orphaned taskgen construct -> firstprivate
- if (prevDSA == Symbol::Flag::OmpShared) {
+ if (prevDSA.test(Symbol::Flag::OmpShared)) {
// 6) shared in enclosing context -> shared
- makeSharedSymbol()...
[truncated]
|
luporl
requested review from
Leporacanthicus,
NimishMishra,
tblah,
kiranchandramohan and
raghavendhra
tblah
approved these changes
May 30, 2025
Yes, but it may take a while. |
Before this change, OmpShared was not always set in shared symbols. Instead, absence of private flags was interpreted as shared DSA. The problem was that symbols with no flags, with only a host association, could also mean "has same DSA as in the enclosing context". Now shared symbols behave the same as private and can be treated the same way. Because of the host association symbols with no flags mentioned above, it was also incorrect to simply test the flags of a given symbol to find out if it was private or shared. The function GetSymbolDSA() was added to fix this. It would be better to avoid the need of these special symbols, but this would require changes to how symbols are collected in lowering. Besides that, some semantic checks need to know if a DSA clause was used or not. To avoid confusing implicit symbols with DSA clauses a new flag was added: OmpExplicit. It is now set for all symbols with explicitly determined data-sharing attributes. With the changes above, AddToContextObjectWithDSA() and the symbol to DSA map could probably be removed and the DSA could be obtained directly from the symbol, but this was not attempted. Some debug messages were also added, with the "omp" DEBUG_TYPE, to make it easier to debug the creation of implicit symbols and to visualize all associations of a given symbol. Fixes llvm#130533 Fixes llvm#140882
luporl
force-pushed
the
luporl-omp-symbols
branch
from
6c89ac2 to
cf64aac
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this change, OmpShared was not always set in shared symbols.
Instead, absence of private flags was interpreted as shared DSA.
The problem was that symbols with no flags, with only a host
association, could also mean "has same DSA as in the enclosing
context". Now shared symbols behave the same as private and can be
treated the same way.
Because of the host association symbols with no flags mentioned
above, it was also incorrect to simply test the flags of a given
symbol to find out if it was private or shared. The function
GetSymbolDSA() was added to fix this. It would be better to avoid
the need of these special symbols, but this would require changes
to how symbols are collected in lowering.
Besides that, some semantic checks need to know if a DSA clause
was used or not. To avoid confusing implicit symbols with DSA
clauses a new flag was added: OmpExplicit. It is now set for all
symbols with explicitly determined data-sharing attributes.
With the changes above, AddToContextObjectWithDSA() and the symbol
to DSA map could probably be removed and the DSA could be obtained
directly from the symbol, but this was not attempted.
Some debug messages were also added, with the "omp" DEBUG_TYPE, to
make it easier to debug the creation of implicit symbols and to
visualize all associations of a given symbol.
Fixes #130533
Fixes #140882