[flang-commits] [flang] [llvm] [Flang][OpenMP] Prevent unrequired implicit maps for private variables on composite directives (PR #217121)
via flang-commits
flang-commits at lists.llvm.org
Fri Oct 23 16:08:50 PDT 2026
================
@@ -4053,6 +4080,254 @@ static void collectSymbolsWithDynamicSubstring(
symbolsWithDynamicSubstring = visitor.symbolsWithDynamicSubstring;
}
+// Reverse search a composite/combined directive and return the innermost leaf
+// carrying a private clause. The OpenMP specification dictates that the first
+// leaf that can legally have a private clause becomes the owner, except when an
+// OpenMP 6.0+ directive-name-modifier explicitly attaches clauses to another
+// leaf or construct in the compound directive.
+//
+// TODO: Extend handling to support directive-name-modifier usage placing
+// private clauses on multiple leaf nodes or constituent constructs.
+static ConstructQueue::const_iterator
+findInnermostPrivateLeaf(ConstructQueue::const_iterator begin,
+ const ConstructQueue &queue) {
+ auto hasPrivate = [](ConstructQueue::const_iterator it) {
+ return llvm::any_of(it->clauses, [](const Clause &clause) {
+ return std::holds_alternative<clause::Private>(clause.u);
+ });
+ };
+ for (auto it = queue.end(); it != begin;) {
+ --it;
+ if (hasPrivate(it))
+ return it;
+ }
+ return queue.end();
+}
+
+static List<Clause>
+getOpenMPConstructClauses(const parser::OpenMPConstruct &ompEval,
+ semantics::SemanticsContext &semaCtx) {
+ List<Clause> clauses;
+ const parser::OmpClauseList *beginClauseList{nullptr};
+ const parser::OmpClauseList *endClauseList{nullptr};
+
+ common::visit(
+ [&](const auto &construct) {
+ using Type = llvm::remove_cvref_t<decltype(construct)>;
+ if constexpr (std::is_same_v<Type, parser::OmpBlockConstruct> ||
+ std::is_same_v<Type, parser::OpenMPLoopConstruct>) {
+ beginClauseList = &construct.BeginDir().Clauses();
+ if (auto &endSpec{construct.EndDir()})
+ endClauseList = &endSpec->Clauses();
+ }
+ },
+ ompEval.u);
+
+ if (!beginClauseList)
+ return clauses;
+
+ clauses.append(makeClauses(*beginClauseList, semaCtx));
+
+ if (endClauseList)
+ clauses.append(makeClauses(*endClauseList, semaCtx));
+
+ return clauses;
+}
+
+static bool isSymbolUsedInClauseRange(const semantics::Symbol &target,
+ ConstructQueue::const_iterator begin,
+ ConstructQueue::const_iterator end,
+ bool ignorePrivateAndMap) {
+ const semantics::Symbol &ultimateTarget = target.GetUltimate();
+
+ for (ConstructQueue::const_iterator it = begin; it != end; ++it) {
+ for (const Clause &clause : it->clauses) {
+ // Do not consider private itself. Also do not consider map clauses here:
+ // explicit target-level maps are already accounted for via mapObjects,
+ // and construct decomposition may synthesize map-like clauses for nested
+ // private list items.
+ if (ignorePrivateAndMap &&
+ (clause.id == llvm::omp::Clause::OMPC_private ||
+ clause.id == llvm::omp::Clause::OMPC_map))
+ continue;
+
+ if (visitSymbolsInClause(clause, [&](const semantics::Symbol &sym) {
+ return sym.GetUltimate() == ultimateTarget;
+ }))
+ return true;
+ }
+ }
+ return false;
+}
+
+// Verify if it is safe to drop the map and materialize a target-local clone.
+// Certain cases still require a map to be OpenMP specification compliant:
+//
+// Allocatable -
+// 1) If the status is unallocated, the private allocatable is unallocated
+// 2) If the allocation status is allocated, the private allocatable is
+// allocated
+// 3) If it is an array, the allocatable's shape and bounds are preserved.
+// Pointer - The initial status of a private pointer is undefined.
+// Other - If the type of the list item has default initialization, the new
+// list item has default initialization. Otherwise, it is undefined.
+//
+// From the above list, it is currently fine to drop maps and materialize clones
+// for scalars, pointers, and constant arrays. Their creation does not require
+// dynamic extent/shape/allocation-status values defined outside the target
+// region. Types that still need a map just to populate the initial state are
+// allocatables, derived types with allocatable or pointer ultimate components,
+// assumed-shape arrays, automatic arrays, and common block members. Currently
+// we leave these as they were before, but in the future we can minimize the
+// mapping by sending across exactly what is required to be specification
+// compliant.
+static bool isTargetLocalCloneable(const semantics::Symbol &sym,
+ semantics::SemanticsContext &semaCtx) {
+ const semantics::Symbol &ult = sym.GetUltimate();
+
+ if (Fortran::semantics::FindCommonBlockContaining(ult))
+ return false;
+
+ if (semantics::IsAllocatable(ult))
+ return false;
+
+ if (const semantics::DeclTypeSpec *declType = ult.GetType())
+ if (const semantics::DerivedTypeSpec *derived = declType->AsDerived())
+ if (semantics::FindAllocatableUltimateComponent(*derived) ||
+ semantics::FindPointerUltimateComponent(*derived))
+ return false;
+
+ // A private pointer's initial status is undefined, so it can be materialized
+ // with the correct type. Other non-scalar private list items must have
+ // constant extents and shape so their storage can be created target-locally.
+ if (ult.Rank() > 0 && !semantics::IsPointer(ult)) {
+ evaluate::FoldingContext &foldingContext = semaCtx.foldingContext();
+ std::optional<evaluate::Shape> shape =
+ evaluate::GetShape(foldingContext, ult);
+ std::optional<evaluate::ConstantSubscripts> extents =
+ evaluate::AsConstantExtents(foldingContext, shape);
+ if (!extents || evaluate::HasNegativeExtent(*extents))
+ return false;
+ }
+ return true;
+}
+
+static bool isExplicitlyMapped(const semantics::Symbol &sym,
+ llvm::ArrayRef<Object> mapObjects) {
+ const semantics::Symbol &ult = sym.GetUltimate();
+ return llvm::any_of(mapObjects, [&](const Object &object) {
+ return object.sym() && object.sym()->GetUltimate() == ult;
+ });
+}
+
+static bool isAlreadyPrivatizedOnTarget(const semantics::Symbol &sym,
+ const DataSharingProcessor &dsp) {
+ const semantics::Symbol &ult = sym.GetUltimate();
+ return llvm::any_of(dsp.getAllSymbolsToPrivatize(),
+ [&](const semantics::Symbol *priv) {
+ return priv->GetUltimate() == ult;
+ });
+}
+
+static void collectTargetNestedPrivateSymsFromLeaf(
+ ConstructQueue::const_iterator leaf, ConstructQueue::const_iterator begin,
+ ConstructQueue::const_iterator end, llvm::ArrayRef<Object> mapObjects,
+ const DataSharingProcessor &dsp, bool excludePrivatizedSyms,
+ semantics::SemanticsContext &semaCtx, TargetNestedPrivateInfo &result) {
+ result.innermostLeafIsTeams = llvm::omp::topTeamsSet.test(leaf->id);
+
+ for (const Clause &clause : leaf->clauses) {
+ if (const auto *priv = std::get_if<clause::Private>(&clause.u)) {
+ for (const Object &object : priv->v) {
+ const semantics::Symbol *sym = object.sym();
+ if (!sym)
+ continue;
+ if (isExplicitlyMapped(*sym, mapObjects))
+ continue;
+ if (excludePrivatizedSyms && isAlreadyPrivatizedOnTarget(*sym, dsp))
+ continue;
+ if (isSymbolUsedInClauseRange(*sym, begin, end,
+ /*ignorePrivateAndMap=*/true))
+ continue;
+ if (!isTargetLocalCloneable(*sym, semaCtx))
+ continue;
+ result.syms.push_back(sym);
+ }
+ }
+ }
+}
+
+static bool collectTargetNestedPrivateSymsFromQueue(
+ ConstructQueue::const_iterator begin, const ConstructQueue &queue,
+ llvm::ArrayRef<Object> mapObjects, const DataSharingProcessor &dsp,
+ bool excludePrivatizedSyms, semantics::SemanticsContext &semaCtx,
+ TargetNestedPrivateInfo &result) {
+ ConstructQueue::const_iterator leaf = findInnermostPrivateLeaf(begin, queue);
+ if (leaf == queue.end())
+ return false;
+
+ collectTargetNestedPrivateSymsFromLeaf(leaf, begin, queue.end(), mapObjects,
+ dsp, excludePrivatizedSyms, semaCtx,
+ result);
+ return true;
+}
+
+// Discover `private` list items on the inner leaf construct of a
+// combined/composite target construct, e.g. "s" in
+// `target teams distribute private(s)`. These symbols are not privatized on the
+// target itself and, without intervention, would usually be implicitly mapped.
+// Collect them so implicit-map generation can skip unneeded maps and target
+// body generation can materialize region-local replacements when needed.
+static TargetNestedPrivateInfo collectTargetNestedPrivateSyms(
+ mlir::ModuleOp modOp, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, ConstructQueue::const_iterator item,
+ const ConstructQueue &queue, llvm::ArrayRef<Object> mapObjects,
+ const DataSharingProcessor &dsp) {
+ TargetNestedPrivateInfo result;
+
+ ConstructQueue::const_iterator next = std::next(item);
+ if (next != queue.end() &&
+ collectTargetNestedPrivateSymsFromQueue(next, queue, mapObjects, dsp,
+ /*excludePrivatizedSyms=*/true,
+ semaCtx, result))
+ return result;
+
+ lower::pft::Evaluation *nestedEval = extractOnlyOmpNestedEval(eval);
----------------
agozillon wrote:
Nice catch thank you! And it appears that it self-handles the case where we have intervening loop code (not just for the above case, but if we moved the loop to rest in the target teams segment instead as well). Added test cases for both.
https://github.com/llvm/llvm-project/pull/217121
More information about the flang-commits
mailing list