diff options
| author | Akash Banerjee <akash.banerjee@amd.com> | 2025-11-13 15:07:46 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-11-13 15:07:46 +0000 |
| commit | bb5f3a08b6ee7baeab6cc4635a9240a8b9dbeb9e (patch) | |
| tree | 948de2b77509eefb5446b36a7866c7058417313d /flang/lib | |
| parent | 55aff64d2c6ef50d2ed725d7dd1fb34080486237 (diff) | |
[Flang][OpenMP] Update declare mapper lookup via use-module (#163860)
- Implemented semantic TODO to catch undeclared mappers.
- Fix mapper lookup to include modules imported through USE.
- Update and add tests.
Fixes #163385.
Diffstat (limited to 'flang/lib')
| -rw-r--r-- | flang/lib/Lower/Bridge.cpp | 7 | ||||
| -rw-r--r-- | flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 12 | ||||
| -rw-r--r-- | flang/lib/Lower/OpenMP/OpenMP.cpp | 59 | ||||
| -rw-r--r-- | flang/lib/Semantics/mod-file.cpp | 12 | ||||
| -rw-r--r-- | flang/lib/Semantics/resolve-names.cpp | 47 | ||||
| -rw-r--r-- | flang/lib/Semantics/symbol.cpp | 6 |
6 files changed, 120 insertions, 23 deletions
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index 20e85a940b18..5bfcff310c23 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -448,6 +448,13 @@ public: } }); + // Ensure imported OpenMP declare mappers are materialized at module + // scope before lowering any constructs that may reference them. + createBuilderOutsideOfFuncOpAndDo([&]() { + Fortran::lower::materializeOpenMPDeclareMappers( + *this, bridge.getSemanticsContext()); + }); + // Create definitions of intrinsic module constants. createBuilderOutsideOfFuncOpAndDo( [&]() { createIntrinsicModuleDefinitions(pft); }); diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 872f31fe45cc..2dd89168ca09 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1397,10 +1397,14 @@ bool ClauseProcessor::processMap( } if (mappers) { assert(mappers->size() == 1 && "more than one mapper"); - mapperIdName = mappers->front().v.id().symbol->name().ToString(); - if (mapperIdName != "default") - mapperIdName = converter.mangleName( - mapperIdName, mappers->front().v.id().symbol->owner()); + const semantics::Symbol *mapperSym = mappers->front().v.id().symbol; + mapperIdName = mapperSym->name().ToString(); + if (mapperIdName != "default") { + // Mangle with the ultimate owner so that use-associated mapper + // identifiers resolve to the same symbol as their defining scope. + const semantics::Symbol &ultimate = mapperSym->GetUltimate(); + mapperIdName = converter.mangleName(mapperIdName, ultimate.owner()); + } } processMapObjects(stmtCtx, clauseLocation, diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 4048aeea37b9..fe80c46c23d0 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -3553,10 +3553,10 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct"); } -static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, - lower::pft::Evaluation &eval, - const parser::OpenMPDeclareMapperConstruct &construct) { +static void genOpenMPDeclareMapperImpl( + lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, + const parser::OpenMPDeclareMapperConstruct &construct, + const semantics::Symbol *mapperSymOpt = nullptr) { mlir::Location loc = converter.genLocation(construct.source); fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); const parser::OmpArgumentList &args = construct.v.Arguments(); @@ -3572,8 +3572,17 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, "Expected derived type"); std::string mapperNameStr = mapperName; - if (auto *sym = converter.getCurrentScope().FindSymbol(mapperNameStr)) + if (mapperSymOpt && mapperNameStr != "default") { + mapperNameStr = converter.mangleName(mapperNameStr, mapperSymOpt->owner()); + } else if (auto *sym = + converter.getCurrentScope().FindSymbol(mapperNameStr)) { mapperNameStr = converter.mangleName(mapperNameStr, sym->owner()); + } + + // If the mapper op already exists (e.g., created by regular lowering or by + // materialization of imported mappers), do not recreate it. + if (converter.getModuleOp().lookupSymbol(mapperNameStr)) + return; // Save current insertion point before moving to the module scope to create // the DeclareMapperOp @@ -3596,6 +3605,13 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, mlir::omp::DeclareMapperInfoOp::create(firOpBuilder, loc, clauseOps.mapVars); } +static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, + semantics::SemanticsContext &semaCtx, + lower::pft::Evaluation &eval, + const parser::OpenMPDeclareMapperConstruct &construct) { + genOpenMPDeclareMapperImpl(converter, semaCtx, construct); +} + static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, @@ -4231,3 +4247,36 @@ void Fortran::lower::genOpenMPRequires(mlir::Operation *mod, offloadMod.setRequires(mlirFlags); } } + +// Walk scopes and materialize omp.declare_mapper ops for mapper declarations +// found in imported modules. If \p scope is null, start from the global scope. +void Fortran::lower::materializeOpenMPDeclareMappers( + Fortran::lower::AbstractConverter &converter, + semantics::SemanticsContext &semaCtx, const semantics::Scope *scope) { + const semantics::Scope &root = scope ? *scope : semaCtx.globalScope(); + + // Recurse into child scopes first (modules, submodules, etc.). + for (const semantics::Scope &child : root.children()) + materializeOpenMPDeclareMappers(converter, semaCtx, &child); + + // Only consider module scopes to avoid duplicating local constructs. + if (!root.IsModule()) + return; + + // Only materialize for modules coming from mod files to avoid duplicates. + if (!root.symbol() || !root.symbol()->test(semantics::Symbol::Flag::ModFile)) + return; + + // Scan symbols in this module scope for MapperDetails. + for (auto &it : root) { + const semantics::Symbol &sym = *it.second; + if (auto *md = sym.detailsIf<semantics::MapperDetails>()) { + for (const auto *decl : md->GetDeclList()) { + if (const auto *mapperDecl = + std::get_if<parser::OpenMPDeclareMapperConstruct>(&decl->u)) { + genOpenMPDeclareMapperImpl(converter, semaCtx, *mapperDecl, &sym); + } + } + } + } +} diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp index b419864f73b8..840b98dd4213 100644 --- a/flang/lib/Semantics/mod-file.cpp +++ b/flang/lib/Semantics/mod-file.cpp @@ -59,6 +59,7 @@ static void PutBound(llvm::raw_ostream &, const Bound &); static void PutShapeSpec(llvm::raw_ostream &, const ShapeSpec &); static void PutShape( llvm::raw_ostream &, const ArraySpec &, char open, char close); +static void PutMapper(llvm::raw_ostream &, const Symbol &, SemanticsContext &); static llvm::raw_ostream &PutAttr(llvm::raw_ostream &, Attr); static llvm::raw_ostream &PutType(llvm::raw_ostream &, const DeclTypeSpec &); @@ -938,6 +939,7 @@ void ModFileWriter::PutEntity(llvm::raw_ostream &os, const Symbol &symbol) { [&](const ProcEntityDetails &) { PutProcEntity(os, symbol); }, [&](const TypeParamDetails &) { PutTypeParam(os, symbol); }, [&](const UserReductionDetails &) { PutUserReduction(os, symbol); }, + [&](const MapperDetails &) { PutMapper(decls_, symbol, context_); }, [&](const auto &) { common::die("PutEntity: unexpected details: %s", DetailsToString(symbol.details()).c_str()); @@ -1101,6 +1103,16 @@ void ModFileWriter::PutUserReduction( } } +static void PutMapper( + llvm::raw_ostream &os, const Symbol &symbol, SemanticsContext &context) { + const auto &details{symbol.get<MapperDetails>()}; + // Emit each saved DECLARE MAPPER construct as-is, so that consumers of the + // module can reparse it and recreate the mapper symbol and semantics state. + for (const auto *decl : details.GetDeclList()) { + Unparse(os, *decl, context.langOptions()); + } +} + void PutInit(llvm::raw_ostream &os, const Symbol &symbol, const MaybeExpr &init, const parser::Expr *unanalyzed, SemanticsContext &context) { if (IsNamedConstant(symbol) || symbol.owner().IsDerivedType()) { diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 09ec951a422c..ea0d38c573af 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1852,21 +1852,25 @@ bool OmpVisitor::Pre(const parser::OmpMapClause &x) { // TODO: Do we need a specific flag or type here, to distinghuish against // other ConstructName things? Leaving this for the full implementation // of mapper lowering. - auto *misc{symbol->detailsIf<MiscDetails>()}; - if (!misc || misc->kind() != MiscDetails::Kind::ConstructName) + auto &ultimate{symbol->GetUltimate()}; + auto *misc{ultimate.detailsIf<MiscDetails>()}; + auto *md{ultimate.detailsIf<MapperDetails>()}; + if (!md && (!misc || misc->kind() != MiscDetails::Kind::ConstructName)) context().Say(mapper->v.source, "Name '%s' should be a mapper name"_err_en_US, mapper->v.source); else mapper->v.symbol = symbol; } else { - mapper->v.symbol = - &MakeSymbol(mapper->v, MiscDetails{MiscDetails::Kind::ConstructName}); - // TODO: When completing the implementation, we probably want to error if - // the symbol is not declared, but right now, testing that the TODO for - // OmpMapClause happens is obscured by the TODO for declare mapper, so - // leaving this out. Remove the above line once the declare mapper is - // implemented. context().Say(mapper->v.source, "'%s' not - // declared"_err_en_US, mapper->v.source); + // Allow the special 'default' mapper identifier without prior + // declaration so lowering can recognize and handle it. Emit an + // error for any other missing mapper identifier. + if (mapper->v.source.ToString() == "default") { + mapper->v.symbol = &MakeSymbol( + mapper->v, MiscDetails{MiscDetails::Kind::ConstructName}); + } else { + context().Say( + mapper->v.source, "'%s' not declared"_err_en_US, mapper->v.source); + } } } return true; @@ -1880,8 +1884,15 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec, // the type has been fully processed. BeginDeclTypeSpec(); auto &mapperName{std::get<std::string>(spec.t)}; - MakeSymbol(parser::CharBlock(mapperName), Attrs{}, - MiscDetails{MiscDetails::Kind::ConstructName}); + // Create or update the mapper symbol with MapperDetails and + // keep track of the declarative construct for module emission. + Symbol &mapperSym{MakeSymbol(parser::CharBlock(mapperName), Attrs{})}; + if (auto *md{mapperSym.detailsIf<MapperDetails>()}) { + md->AddDecl(declaratives_.back()); + } else if (mapperSym.has<UnknownDetails>() || mapperSym.has<MiscDetails>()) { + mapperSym.set_details(MapperDetails{}); + mapperSym.get<MapperDetails>().AddDecl(declaratives_.back()); + } PushScope(Scope::Kind::OtherConstruct, nullptr); Walk(std::get<parser::TypeSpec>(spec.t)); auto &varName{std::get<parser::Name>(spec.t)}; @@ -3611,10 +3622,20 @@ void ModuleVisitor::Post(const parser::UseStmt &x) { rename.u); } for (const auto &[name, symbol] : *useModuleScope_) { + // Default USE imports public names, excluding intrinsic-only and most + // miscellaneous details. Allow OpenMP mapper identifiers represented + // as MapperDetails, and also legacy MiscDetails::ConstructName. + bool isMapper{symbol->has<MapperDetails>()}; + if (!isMapper) { + if (const auto *misc{symbol->detailsIf<MiscDetails>()}) { + isMapper = misc->kind() == MiscDetails::Kind::ConstructName; + } + } if (symbol->attrs().test(Attr::PUBLIC) && !IsUseRenamed(symbol->name()) && (!symbol->implicitAttrs().test(Attr::INTRINSIC) || symbol->has<UseDetails>()) && - !symbol->has<MiscDetails>() && useNames.count(name) == 0) { + (!symbol->has<MiscDetails>() || isMapper) && + useNames.count(name) == 0) { SourceName location{x.moduleName.source}; if (auto *localSymbol{FindInScope(name)}) { DoAddUse(location, localSymbol->name(), *localSymbol, *symbol); diff --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp index 0ec44b7c4049..ed0715a422e7 100644 --- a/flang/lib/Semantics/symbol.cpp +++ b/flang/lib/Semantics/symbol.cpp @@ -338,7 +338,8 @@ std::string DetailsToString(const Details &details) { [](const TypeParamDetails &) { return "TypeParam"; }, [](const MiscDetails &) { return "Misc"; }, [](const AssocEntityDetails &) { return "AssocEntity"; }, - [](const UserReductionDetails &) { return "UserReductionDetails"; }}, + [](const UserReductionDetails &) { return "UserReductionDetails"; }, + [](const MapperDetails &) { return "MapperDetails"; }}, details); } @@ -379,6 +380,7 @@ bool Symbol::CanReplaceDetails(const Details &details) const { [&](const UserReductionDetails &) { return has<UserReductionDetails>(); }, + [&](const MapperDetails &) { return has<MapperDetails>(); }, [](const auto &) { return false; }, }, details); @@ -685,6 +687,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Details &details) { DumpType(os, type); } }, + // Avoid recursive streaming for MapperDetails; nothing more to dump + [&](const MapperDetails &) {}, [&](const auto &x) { os << x; }, }, details); |
