diff --git a/flang/include/flang/Lower/OpenMP.h b/flang/include/flang/Lower/OpenMP.h index 962abd8952073..df01a7b82c66c 100644 --- a/flang/include/flang/Lower/OpenMP.h +++ b/flang/include/flang/Lower/OpenMP.h @@ -97,13 +97,6 @@ bool markOpenMPDeferredDeclareTargetFunctions( AbstractConverter &); void genOpenMPRequires(mlir::Operation *, const Fortran::semantics::Symbol *); -// Materialize omp.declare_mapper ops for mapper declarations found in -// imported modules. If \p scope is null, materialize for the whole -// semantics global scope; otherwise, operate recursively starting at \p scope. -void materializeOpenMPDeclareMappers( - Fortran::lower::AbstractConverter &, Fortran::semantics::SemanticsContext &, - const Fortran::semantics::Scope *scope = nullptr); - } // namespace lower } // namespace Fortran diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h index 95efe1ae2bd5e..cb27d544ed9f5 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -777,24 +777,6 @@ class UserReductionDetails { DeclVector declList_; }; -// Used for OpenMP DECLARE MAPPER, it holds the declaration constructs -// so they can be serialized into module files and later re-parsed when -// USE-associated. -class MapperDetails { -public: - using DeclVector = std::vector; - - MapperDetails() = default; - - void AddDecl(const parser::OpenMPDeclarativeConstruct *decl) { - declList_.emplace_back(decl); - } - const DeclVector &GetDeclList() const { return declList_; } - -private: - DeclVector declList_; -}; - class UnknownDetails {}; using Details = std::variant; + TypeParamDetails, MiscDetails, UserReductionDetails>; llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Details &); std::string DetailsToString(const Details &); diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index 5bfcff310c232..20e85a940b182 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -448,13 +448,6 @@ class FirConverter : public Fortran::lower::AbstractConverter { } }); - // 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 2dd89168ca098..872f31fe45cca 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1397,14 +1397,10 @@ bool ClauseProcessor::processMap( } if (mappers) { assert(mappers->size() == 1 && "more than one mapper"); - 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()); - } + mapperIdName = mappers->front().v.id().symbol->name().ToString(); + if (mapperIdName != "default") + mapperIdName = converter.mangleName( + mapperIdName, mappers->front().v.id().symbol->owner()); } processMapObjects(stmtCtx, clauseLocation, diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index fe80c46c23d06..4048aeea37b92 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 genOpenMPDeclareMapperImpl( - lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, - const parser::OpenMPDeclareMapperConstruct &construct, - const semantics::Symbol *mapperSymOpt = nullptr) { +static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, + semantics::SemanticsContext &semaCtx, + lower::pft::Evaluation &eval, + const parser::OpenMPDeclareMapperConstruct &construct) { mlir::Location loc = converter.genLocation(construct.source); fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); const parser::OmpArgumentList &args = construct.v.Arguments(); @@ -3572,17 +3572,8 @@ static void genOpenMPDeclareMapperImpl( "Expected derived type"); std::string mapperNameStr = mapperName; - if (mapperSymOpt && mapperNameStr != "default") { - mapperNameStr = converter.mangleName(mapperNameStr, mapperSymOpt->owner()); - } else if (auto *sym = - converter.getCurrentScope().FindSymbol(mapperNameStr)) { + 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 @@ -3605,13 +3596,6 @@ static void genOpenMPDeclareMapperImpl( 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, @@ -4247,36 +4231,3 @@ 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()) { - for (const auto *decl : md->GetDeclList()) { - if (const auto *mapperDecl = - std::get_if(&decl->u)) { - genOpenMPDeclareMapperImpl(converter, semaCtx, *mapperDecl, &sym); - } - } - } - } -} diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp index 840b98dd42139..b419864f73b8e 100644 --- a/flang/lib/Semantics/mod-file.cpp +++ b/flang/lib/Semantics/mod-file.cpp @@ -59,7 +59,6 @@ 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 &); @@ -939,7 +938,6 @@ 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()); @@ -1103,16 +1101,6 @@ void ModFileWriter::PutUserReduction( } } -static void PutMapper( - llvm::raw_ostream &os, const Symbol &symbol, SemanticsContext &context) { - const auto &details{symbol.get()}; - // 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 ea0d38c573af9..09ec951a422ca 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1852,25 +1852,21 @@ 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 &ultimate{symbol->GetUltimate()}; - auto *misc{ultimate.detailsIf()}; - auto *md{ultimate.detailsIf()}; - if (!md && (!misc || misc->kind() != MiscDetails::Kind::ConstructName)) + auto *misc{symbol->detailsIf()}; + if (!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 { - // 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); - } + 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); } } return true; @@ -1884,15 +1880,8 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec, // the type has been fully processed. BeginDeclTypeSpec(); auto &mapperName{std::get(spec.t)}; - // 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()}) { - md->AddDecl(declaratives_.back()); - } else if (mapperSym.has() || mapperSym.has()) { - mapperSym.set_details(MapperDetails{}); - mapperSym.get().AddDecl(declaratives_.back()); - } + MakeSymbol(parser::CharBlock(mapperName), Attrs{}, + MiscDetails{MiscDetails::Kind::ConstructName}); PushScope(Scope::Kind::OtherConstruct, nullptr); Walk(std::get(spec.t)); auto &varName{std::get(spec.t)}; @@ -3622,20 +3611,10 @@ 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()}; - if (!isMapper) { - if (const auto *misc{symbol->detailsIf()}) { - isMapper = misc->kind() == MiscDetails::Kind::ConstructName; - } - } if (symbol->attrs().test(Attr::PUBLIC) && !IsUseRenamed(symbol->name()) && (!symbol->implicitAttrs().test(Attr::INTRINSIC) || symbol->has()) && - (!symbol->has() || isMapper) && - useNames.count(name) == 0) { + !symbol->has() && 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 ed0715a422e78..0ec44b7c40491 100644 --- a/flang/lib/Semantics/symbol.cpp +++ b/flang/lib/Semantics/symbol.cpp @@ -338,8 +338,7 @@ std::string DetailsToString(const Details &details) { [](const TypeParamDetails &) { return "TypeParam"; }, [](const MiscDetails &) { return "Misc"; }, [](const AssocEntityDetails &) { return "AssocEntity"; }, - [](const UserReductionDetails &) { return "UserReductionDetails"; }, - [](const MapperDetails &) { return "MapperDetails"; }}, + [](const UserReductionDetails &) { return "UserReductionDetails"; }}, details); } @@ -380,7 +379,6 @@ bool Symbol::CanReplaceDetails(const Details &details) const { [&](const UserReductionDetails &) { return has(); }, - [&](const MapperDetails &) { return has(); }, [](const auto &) { return false; }, }, details); @@ -687,8 +685,6 @@ 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); diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90 index e4c010156ee39..c389d0ff4bd15 100644 --- a/flang/test/Lower/OpenMP/declare-mapper.f90 +++ b/flang/test/Lower/OpenMP/declare-mapper.f90 @@ -6,9 +6,7 @@ ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-3.f90 -o - | FileCheck %t/omp-declare-mapper-3.f90 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-4.f90 -o - | FileCheck %t/omp-declare-mapper-4.f90 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-5.f90 -o - | FileCheck %t/omp-declare-mapper-5.f90 -! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90 -! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 -module-dir %t %t/omp-declare-mapper-7.mod.f90 -o - >/dev/null -! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 -J %t %t/omp-declare-mapper-7.use.f90 -o - | FileCheck %t/omp-declare-mapper-7.use.f90 +! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=51 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90 !--- omp-declare-mapper-1.f90 subroutine declare_mapper_1 @@ -303,25 +301,3 @@ subroutine declare_mapper_nested_parent r%real_arr = r%base_arr(1) + r%inner%deep_arr(1) !$omp end target end subroutine declare_mapper_nested_parent - -!--- omp-declare-mapper-7.mod.f90 -! Module with DECLARE MAPPER to be compiled separately -module m_mod - implicit none - type :: mty - integer :: x - end type mty - !$omp declare mapper(mymap : mty :: v) map(tofrom: v%x) -end module m_mod - -!--- omp-declare-mapper-7.use.f90 -! Consumer program that USEs the module and applies the mapper by name. -! CHECK: %{{.*}} = omp.map.info {{.*}} mapper(@{{.*mymap}}) {{.*}} {name = "a"} -program use_module_mapper - use m_mod - implicit none - type(mty) :: a - !$omp target map(mapper(mymap) : a) - a%x = 42 - !$omp end target -end program use_module_mapper diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90 index 7d9b8856ac833..83662b70f08f5 100644 --- a/flang/test/Parser/OpenMP/map-modifiers.f90 +++ b/flang/test/Parser/OpenMP/map-modifiers.f90 @@ -320,7 +320,7 @@ subroutine f21(x, y) integer :: x(10) integer :: y integer, parameter :: p = 23 - !$omp target map(mapper(default), from: x) + !$omp target map(mapper(xx), from: x) x = x + 1 !$omp end target end @@ -329,7 +329,7 @@ subroutine f21(x, y) !UNPARSE: INTEGER x(10_4) !UNPARSE: INTEGER y !UNPARSE: INTEGER, PARAMETER :: p = 23_4 -!UNPARSE: !$OMP TARGET MAP(MAPPER(DEFAULT), FROM: X) +!UNPARSE: !$OMP TARGET MAP(MAPPER(XX), FROM: X) !UNPARSE: x=x+1_4 !UNPARSE: !$OMP END TARGET !UNPARSE: END SUBROUTINE @@ -337,7 +337,7 @@ subroutine f21(x, y) !PARSE-TREE: OmpBeginDirective !PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause -!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'default' +!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'xx' !PARSE-TREE: | | Modifier -> OmpMapType -> Value = From !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' @@ -375,3 +375,4 @@ subroutine f22(x) !PARSE-TREE: | | SectionSubscript -> Integer -> Expr = 'i' !PARSE-TREE: | | | Designator -> DataRef -> Name = 'i' !PARSE-TREE: | bool = 'true' + diff --git a/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 b/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 deleted file mode 100644 index 480f87bc0f8e9..0000000000000 --- a/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 +++ /dev/null @@ -1,14 +0,0 @@ -! RUN: split-file %s %t -! RUN: %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=50 -module-dir %t %t/m.f90 -! RUN: cat %t/m.mod | FileCheck --ignore-case %s - -!--- m.f90 -module m - implicit none - type :: t - integer :: x - end type t - !$omp declare mapper(mymap : t :: v) map(v%x) -end module m - -!CHECK: !$OMP DECLARE MAPPER(mymap:t::v) MAP(v%x) diff --git a/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 b/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 index 5d77540aa6453..e57a5c0c1cea6 100644 --- a/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 +++ b/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 @@ -11,9 +11,9 @@ program main !$omp declare mapper(ty :: maptwo) map(maptwo, maptwo%x) !! Note, symbols come out in their respective scope, but not in declaration order. -!CHECK: mymapper: MapperDetails +!CHECK: mymapper: Misc ConstructName !CHECK: ty: DerivedType components: x -!CHECK: ty.omp.default.mapper: MapperDetails +!CHECK: ty.omp.default.mapper: Misc ConstructName !CHECK: DerivedType scope: ty !CHECK: OtherConstruct scope: !CHECK: mapped (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty) @@ -21,3 +21,4 @@ program main !CHECK: maptwo (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty) end program main + diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90 index 3b723e817ce87..1d6315b4a2312 100644 --- a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 +++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90 @@ -1,16 +1,14 @@ ! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s program main !CHECK-LABEL: MainProgram scope: MAIN - type ty - real(4) :: x - end type ty - !$omp declare mapper(xx : ty :: v) map(v) integer, parameter :: n = 256 - type(ty) :: a(256) + real(8) :: a(256) !$omp target map(mapper(xx), from:a) do i=1,n - a(i)%x = 4.2 + a(i) = 4.2 end do !$omp end target -!CHECK: xx: MapperDetails +!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=74 bytes +!CHECK: OtherClause scope: size=0 alignment=1 sourceRange=0 bytes +!CHECK: xx: Misc ConstructName end program main