From 80194f3be4fc2a0fe8e0752d022a020d7e5e1b72 Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Thu, 16 Oct 2025 21:36:07 +0100 Subject: [PATCH 1/5] [Flang][OpenMP] Update declare mapper lookup via use-module - Implemented semantic TODO to catch undeclared mappers. - Fix mapper lookup to include modules imported through USE. - Update and add tests. Fixes #163385. --- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 12 ++++--- flang/lib/Semantics/resolve-names.cpp | 31 +++++++++++++------ flang/test/Lower/OpenMP/declare-mapper.f90 | 25 ++++++++++++++- flang/test/Parser/OpenMP/map-modifiers.f90 | 7 ++--- .../Semantics/OpenMP/map-clause-symbols.f90 | 8 ++--- 5 files changed, 59 insertions(+), 24 deletions(-) diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 85398be778382..41820c74e3921 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1402,10 +1402,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/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index ae0ff9ca8068d..ef45c692acc8c 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1824,21 +1824,24 @@ 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()}; + const Symbol &ultimate{symbol->GetUltimate()}; + auto *misc{const_cast(ultimate).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 { - 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; @@ -3614,10 +3617,18 @@ 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. However, allow OpenMP mapper identifiers, + // which are currently represented with MiscDetails::ConstructName. + bool isMapper{false}; + 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() && useNames.count(name) == 0) { + (!symbol->has() || 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/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90 index 3d4d0da1e18a3..a5fb0710592ce 100644 --- a/flang/test/Lower/OpenMP/declare-mapper.f90 +++ b/flang/test/Lower/OpenMP/declare-mapper.f90 @@ -6,7 +6,8 @@ ! 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=51 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.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 %t/omp-declare-mapper-7.f90 -o - | FileCheck %t/omp-declare-mapper-7.f90 !--- omp-declare-mapper-1.f90 subroutine declare_mapper_1 @@ -301,3 +302,25 @@ 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.f90 +! Check mappers declared inside modules and used via USE association. +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 + +program use_module_mapper + use m_mod + implicit none + type(mty) :: a + + ! CHECK: omp.declare_mapper @[[MODMAP:[^ ]*mymap]] + ! CHECK: %{{.*}} = omp.map.info {{.*}} mapper(@[[MODMAP]]) {{.*}} {name = "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 83662b70f08f5..7d9b8856ac833 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(xx), from: x) + !$omp target map(mapper(default), 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(XX), FROM: X) +!UNPARSE: !$OMP TARGET MAP(MAPPER(DEFAULT), 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 = 'xx' +!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'default' !PARSE-TREE: | | Modifier -> OmpMapType -> Value = From !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' @@ -375,4 +375,3 @@ 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/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90 index 1d6315b4a2312..cdb65a71e8887 100644 --- a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 +++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90 @@ -1,6 +1,5 @@ -! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s +! RUN: not %flang_fc1 -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s program main -!CHECK-LABEL: MainProgram scope: MAIN integer, parameter :: n = 256 real(8) :: a(256) !$omp target map(mapper(xx), from:a) @@ -8,7 +7,6 @@ program main a(i) = 4.2 end do !$omp end target -!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 + +! CHECK: error: '{{.*}}' not declared From a8f161380f23b270435680599e466a9e560b531f Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Thu, 16 Oct 2025 21:51:22 +0100 Subject: [PATCH 2/5] code cleanup --- flang/lib/Semantics/resolve-names.cpp | 92 +++++++++++++-------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index ef45c692acc8c..aa7527723dfb6 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1824,8 +1824,7 @@ 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. - const Symbol &ultimate{symbol->GetUltimate()}; - auto *misc{const_cast(ultimate).detailsIf()}; + auto *misc{symbol->GetUltimate().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); @@ -2376,7 +2375,7 @@ void AttrsVisitor::SetBindNameOn(Symbol &symbol) { } symbol.SetBindName(std::move(*label)); if (!oldBindName.empty()) { - if (const std::string * newBindName{symbol.GetBindName()}) { + if (const std::string *newBindName{symbol.GetBindName()}) { if (oldBindName != *newBindName) { Say(symbol.name(), "The entity '%s' has multiple BIND names ('%s' and '%s')"_err_en_US, @@ -2502,7 +2501,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) { // expression semantics if the DeclTypeSpec is a valid TypeSpec. // The grammar ensures that it's an intrinsic or derived type spec, // not TYPE(*) or CLASS(*) or CLASS(T). - if (const DeclTypeSpec * spec{state_.declTypeSpec}) { + if (const DeclTypeSpec *spec{state_.declTypeSpec}) { switch (spec->category()) { case DeclTypeSpec::Numeric: case DeclTypeSpec::Logical: @@ -2510,7 +2509,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) { typeSpec.declTypeSpec = spec; break; case DeclTypeSpec::TypeDerived: - if (const DerivedTypeSpec * derived{spec->AsDerived()}) { + if (const DerivedTypeSpec *derived{spec->AsDerived()}) { CheckForAbstractType(derived->typeSymbol()); // C703 typeSpec.declTypeSpec = spec; } @@ -3101,8 +3100,8 @@ Symbol &ScopeHandler::MakeSymbol(const parser::Name &name, Attrs attrs) { Symbol &ScopeHandler::MakeHostAssocSymbol( const parser::Name &name, const Symbol &hostSymbol) { Symbol &symbol{*NonDerivedTypeScope() - .try_emplace(name.source, HostAssocDetails{hostSymbol}) - .first->second}; + .try_emplace(name.source, HostAssocDetails{hostSymbol}) + .first->second}; name.symbol = &symbol; symbol.attrs() = hostSymbol.attrs(); // TODO: except PRIVATE, PUBLIC? // These attributes can be redundantly reapplied without error @@ -3190,7 +3189,7 @@ void ScopeHandler::ApplyImplicitRules( if (context().HasError(symbol) || !NeedsType(symbol)) { return; } - if (const DeclTypeSpec * type{GetImplicitType(symbol)}) { + if (const DeclTypeSpec *type{GetImplicitType(symbol)}) { if (!skipImplicitTyping_) { symbol.set(Symbol::Flag::Implicit); symbol.SetType(*type); @@ -3290,7 +3289,7 @@ const DeclTypeSpec *ScopeHandler::GetImplicitType( const auto *type{implicitRulesMap_->at(scope).GetType( symbol.name(), respectImplicitNoneType)}; if (type) { - if (const DerivedTypeSpec * derived{type->AsDerived()}) { + if (const DerivedTypeSpec *derived{type->AsDerived()}) { // Resolve any forward-referenced derived type; a quick no-op else. auto &instantiatable{*const_cast(derived)}; instantiatable.Instantiate(currScope()); @@ -4299,7 +4298,7 @@ Scope *ModuleVisitor::FindModule(const parser::Name &name, if (scope) { if (DoesScopeContain(scope, currScope())) { // 14.2.2(1) std::optional submoduleName; - if (const Scope * container{FindModuleOrSubmoduleContaining(currScope())}; + if (const Scope *container{FindModuleOrSubmoduleContaining(currScope())}; container && container->IsSubmodule()) { submoduleName = container->GetName(); } @@ -4404,7 +4403,7 @@ bool InterfaceVisitor::isAbstract() const { void InterfaceVisitor::AddSpecificProcs( const std::list &names, ProcedureKind kind) { - if (Symbol * symbol{GetGenericInfo().symbol}; + if (Symbol *symbol{GetGenericInfo().symbol}; symbol && symbol->has()) { for (const auto &name : names) { specificsForGenericProcs_.emplace(symbol, std::make_pair(&name, kind)); @@ -4504,7 +4503,7 @@ void GenericHandler::DeclaredPossibleSpecificProc(Symbol &proc) { } void InterfaceVisitor::ResolveNewSpecifics() { - if (Symbol * generic{genericInfo_.top().symbol}; + if (Symbol *generic{genericInfo_.top().symbol}; generic && generic->has()) { ResolveSpecificsInGeneric(*generic, false); } @@ -4589,7 +4588,7 @@ bool SubprogramVisitor::HandleStmtFunction(const parser::StmtFunctionStmt &x) { name.source); MakeSymbol(name, Attrs{}, UnknownDetails{}); } else if (auto *entity{ultimate.detailsIf()}; - entity && !ultimate.has()) { + entity && !ultimate.has()) { resultType = entity->type(); ultimate.details() = UnknownDetails{}; // will be replaced below } else { @@ -4648,7 +4647,7 @@ bool SubprogramVisitor::Pre(const parser::Suffix &suffix) { } else { Message &msg{Say(*suffix.resultName, "RESULT(%s) may appear only in a function"_err_en_US)}; - if (const Symbol * subprogram{InclusiveScope().symbol()}) { + if (const Symbol *subprogram{InclusiveScope().symbol()}) { msg.Attach(subprogram->name(), "Containing subprogram"_en_US); } } @@ -5164,7 +5163,7 @@ Symbol *ScopeHandler::FindSeparateModuleProcedureInterface( symbol = generic->specific(); } } - if (const Symbol * defnIface{FindSeparateModuleSubprogramInterface(symbol)}) { + if (const Symbol *defnIface{FindSeparateModuleSubprogramInterface(symbol)}) { // Error recovery in case of multiple definitions symbol = const_cast(defnIface); } @@ -5303,8 +5302,8 @@ bool SubprogramVisitor::HandlePreviousCalls( return generic->specific() && HandlePreviousCalls(name, *generic->specific(), subpFlag); } else if (const auto *proc{symbol.detailsIf()}; proc && - !proc->isDummy() && - !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) { + !proc->isDummy() && + !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) { // There's a symbol created for previous calls to this subprogram or // ENTRY's name. We have to replace that symbol in situ to avoid the // obligation to rewrite symbol pointers in the parse tree. @@ -5346,7 +5345,7 @@ const Symbol *SubprogramVisitor::CheckExtantProc( if (prev) { if (IsDummy(*prev)) { } else if (auto *entity{prev->detailsIf()}; - IsPointer(*prev) && entity && !entity->type()) { + IsPointer(*prev) && entity && !entity->type()) { // POINTER attribute set before interface } else if (inInterfaceBlock() && currScope() != prev->owner()) { // Procedures in an INTERFACE block do not resolve to symbols @@ -5418,7 +5417,7 @@ Symbol *SubprogramVisitor::PushSubprogramScope(const parser::Name &name, } set_inheritFromParent(false); // interfaces don't inherit, even if MODULE } - if (Symbol * found{FindSymbol(name)}; + if (Symbol *found{FindSymbol(name)}; found && found->has()) { found->set(subpFlag); // PushScope() created symbol } @@ -6266,9 +6265,9 @@ void DeclarationVisitor::Post(const parser::VectorTypeSpec &x) { vectorDerivedType.CookParameters(GetFoldingContext()); } - if (const DeclTypeSpec * - extant{ppcBuiltinTypesScope->FindInstantiatedDerivedType( - vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) { + if (const DeclTypeSpec *extant{ + ppcBuiltinTypesScope->FindInstantiatedDerivedType( + vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) { // This derived type and parameter expressions (if any) are already present // in the __ppc_intrinsics scope. SetDeclTypeSpec(*extant); @@ -6290,7 +6289,7 @@ bool DeclarationVisitor::Pre(const parser::DeclarationTypeSpec::Type &) { void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Type &type) { const parser::Name &derivedName{std::get(type.derived.t)}; - if (const Symbol * derivedSymbol{derivedName.symbol}) { + if (const Symbol *derivedSymbol{derivedName.symbol}) { CheckForAbstractType(*derivedSymbol); // C706 } } @@ -6359,8 +6358,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) { if (!spec->MightBeParameterized()) { spec->EvaluateParameters(context()); } - if (const DeclTypeSpec * - extant{currScope().FindInstantiatedDerivedType(*spec, category)}) { + if (const DeclTypeSpec *extant{ + currScope().FindInstantiatedDerivedType(*spec, category)}) { // This derived type and parameter expressions (if any) are already present // in this scope. SetDeclTypeSpec(*extant); @@ -6391,8 +6390,7 @@ void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Record &rec) { if (auto spec{ResolveDerivedType(typeName)}) { spec->CookParameters(GetFoldingContext()); spec->EvaluateParameters(context()); - if (const DeclTypeSpec * - extant{currScope().FindInstantiatedDerivedType( + if (const DeclTypeSpec *extant{currScope().FindInstantiatedDerivedType( *spec, DeclTypeSpec::TypeDerived)}) { SetDeclTypeSpec(*extant); } else { @@ -7403,7 +7401,7 @@ bool DeclarationVisitor::PassesLocalityChecks( "Coarray '%s' not allowed in a %s locality-spec"_err_en_US, specName); return false; } - if (const DeclTypeSpec * type{symbol.GetType()}) { + if (const DeclTypeSpec *type{symbol.GetType()}) { if (type->IsPolymorphic() && IsDummy(symbol) && !IsPointer(symbol) && !isReduce) { // F'2023 C1130 SayWithDecl(name, symbol, @@ -7630,7 +7628,7 @@ Symbol *DeclarationVisitor::NoteInterfaceName(const parser::Name &name) { } void DeclarationVisitor::CheckExplicitInterface(const parser::Name &name) { - if (const Symbol * symbol{name.symbol}) { + if (const Symbol *symbol{name.symbol}) { const Symbol &ultimate{symbol->GetUltimate()}; if (!context().HasError(*symbol) && !context().HasError(ultimate) && !BypassGeneric(ultimate).HasExplicitInterface()) { @@ -7948,7 +7946,7 @@ bool ConstructVisitor::Pre(const parser::DataStmtValue &x) { auto &mutableData{const_cast(data)}; if (auto *elem{parser::Unwrap(mutableData)}) { if (const auto *name{std::get_if(&elem->base.u)}) { - if (const Symbol * symbol{FindSymbol(*name)}; + if (const Symbol *symbol{FindSymbol(*name)}; symbol && symbol->GetUltimate().has()) { mutableData.u = elem->ConvertToStructureConstructor( DerivedTypeSpec{name->source, *symbol}); @@ -8094,15 +8092,15 @@ void ConstructVisitor::Post(const parser::SelectTypeStmt &x) { } } } else { - if (const Symbol * - whole{UnwrapWholeSymbolDataRef(association.selector.expr)}) { + if (const Symbol *whole{ + UnwrapWholeSymbolDataRef(association.selector.expr)}) { ConvertToObjectEntity(const_cast(*whole)); if (!IsVariableName(*whole)) { Say(association.selector.source, // C901 "Selector is not a variable"_err_en_US); association = {}; } - if (const DeclTypeSpec * type{whole->GetType()}) { + if (const DeclTypeSpec *type{whole->GetType()}) { if (!type->IsPolymorphic()) { // C1159 Say(association.selector.source, "Selector '%s' in SELECT TYPE statement must be " @@ -8242,8 +8240,8 @@ Symbol *ConstructVisitor::MakeAssocEntity() { "The associate name '%s' is already used in this associate statement"_err_en_US); return nullptr; } - } else if (const Symbol * - whole{UnwrapWholeSymbolDataRef(association.selector.expr)}) { + } else if (const Symbol *whole{ + UnwrapWholeSymbolDataRef(association.selector.expr)}) { symbol = &MakeSymbol(whole->name()); } else { return nullptr; @@ -8864,7 +8862,7 @@ bool DeclarationVisitor::CheckForHostAssociatedImplicit( if (name.symbol) { ApplyImplicitRules(*name.symbol, true); } - if (Scope * host{GetHostProcedure()}; host && !isImplicitNoneType(*host)) { + if (Scope *host{GetHostProcedure()}; host && !isImplicitNoneType(*host)) { Symbol *hostSymbol{nullptr}; if (!name.symbol) { if (currScope().CanImport(name.source)) { @@ -8935,7 +8933,7 @@ const parser::Name *DeclarationVisitor::FindComponent( if (!type) { return nullptr; // should have already reported error } - if (const IntrinsicTypeSpec * intrinsic{type->AsIntrinsic()}) { + if (const IntrinsicTypeSpec *intrinsic{type->AsIntrinsic()}) { auto category{intrinsic->category()}; MiscDetails::Kind miscKind{MiscDetails::Kind::None}; if (component.source == "kind") { @@ -8957,7 +8955,7 @@ const parser::Name *DeclarationVisitor::FindComponent( } } else if (DerivedTypeSpec * derived{type->AsDerived()}) { derived->Instantiate(currScope()); // in case of forward referenced type - if (const Scope * scope{derived->scope()}) { + if (const Scope *scope{derived->scope()}) { if (Resolve(component, scope->FindComponent(component.source))) { if (auto msg{CheckAccessibleSymbol(currScope(), *component.symbol)}) { context().Say(component.source, *msg); @@ -9108,8 +9106,8 @@ void DeclarationVisitor::PointerInitialization( if (evaluate::IsNullProcedurePointer(&*expr)) { CHECK(!details->init()); details->set_init(nullptr); - } else if (const Symbol * - targetSymbol{evaluate::UnwrapWholeSymbolDataRef(*expr)}) { + } else if (const Symbol *targetSymbol{ + evaluate::UnwrapWholeSymbolDataRef(*expr)}) { CHECK(!details->init()); details->set_init(*targetSymbol); } else { @@ -9677,7 +9675,7 @@ void ResolveNamesVisitor::EarlyDummyTypeDeclaration( for (const auto &ent : entities) { const auto &objName{std::get(ent.t)}; Resolve(objName, FindInScope(currScope(), objName)); - if (Symbol * symbol{objName.symbol}; + if (Symbol *symbol{objName.symbol}; symbol && IsDummy(*symbol) && NeedsType(*symbol)) { if (!type) { type = ProcessTypeSpec(declTypeSpec); @@ -9816,7 +9814,7 @@ void ResolveNamesVisitor::FinishSpecificationPart( if (auto *proc{symbol.detailsIf()}; proc && !proc->isDummy() && !IsPointer(symbol) && !symbol.attrs().test(Attr::BIND_C)) { - if (const Symbol * iface{proc->procInterface()}; + if (const Symbol *iface{proc->procInterface()}; iface && IsBindCProcedure(*iface)) { SetImplicitAttr(symbol, Attr::BIND_C); SetBindNameOn(symbol); @@ -9949,7 +9947,7 @@ bool ResolveNamesVisitor::Pre(const parser::PointerAssignmentStmt &x) { Symbol *ptrSymbol{parser::GetLastName(dataRef).symbol}; Walk(bounds); // Resolve unrestricted specific intrinsic procedures as in "p => cos". - if (const parser::Name * name{parser::Unwrap(expr)}) { + if (const parser::Name *name{parser::Unwrap(expr)}) { if (NameIsKnownOrIntrinsic(*name)) { if (Symbol * symbol{name->symbol}) { if (IsProcedurePointer(ptrSymbol) && @@ -10390,8 +10388,8 @@ void ResolveNamesVisitor::ResolveSpecificationParts(ProgramTree &node) { // implied SAVE so that evaluate::IsSaved() will return true. if (node.scope()->kind() == Scope::Kind::MainProgram) { if (const auto *object{symbol.detailsIf()}) { - if (const DeclTypeSpec * type{object->type()}) { - if (const DerivedTypeSpec * derived{type->AsDerived()}) { + if (const DeclTypeSpec *type{object->type()}) { + if (const DerivedTypeSpec *derived{type->AsDerived()}) { if (!IsSaved(symbol) && FindCoarrayPotentialComponent(*derived)) { SetImplicitAttr(symbol, Attr::SAVE); } @@ -10648,7 +10646,7 @@ void ResolveNamesVisitor::FinishDerivedTypeInstantiation(Scope &scope) { if (DerivedTypeSpec * spec{scope.derivedTypeSpec()}) { spec->Instantiate(currScope()); const Symbol &origTypeSymbol{spec->typeSymbol()}; - if (const Scope * origTypeScope{origTypeSymbol.scope()}) { + if (const Scope *origTypeScope{origTypeSymbol.scope()}) { CHECK(origTypeScope->IsDerivedType() && origTypeScope->symbol() == &origTypeSymbol); auto &foldingContext{GetFoldingContext()}; @@ -10659,7 +10657,7 @@ void ResolveNamesVisitor::FinishDerivedTypeInstantiation(Scope &scope) { if (IsPointer(comp)) { if (auto *details{comp.detailsIf()}) { auto origDetails{origComp.get()}; - if (const MaybeExpr & init{origDetails.init()}) { + if (const MaybeExpr &init{origDetails.init()}) { SomeExpr newInit{*init}; MaybeExpr folded{FoldExpr(std::move(newInit))}; details->set_init(std::move(folded)); From 0298b8189f8f065bbf084bae83c30e3290d29352 Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Wed, 22 Oct 2025 22:36:10 +0100 Subject: [PATCH 3/5] Emit declare_mapper to .mod file. Update test to check separate module and program file compilation. --- flang/include/flang/Lower/OpenMP.h | 7 +++ flang/include/flang/Semantics/symbol.h | 20 ++++++- flang/lib/Lower/Bridge.cpp | 7 +++ flang/lib/Lower/OpenMP/OpenMP.cpp | 55 +++++++++++++++++-- flang/lib/Semantics/mod-file.cpp | 12 ++++ flang/lib/Semantics/resolve-names.cpp | 29 +++++++--- flang/lib/Semantics/symbol.cpp | 6 +- flang/test/Lower/OpenMP/declare-mapper.f90 | 13 +++-- .../OpenMP/declare-mapper-symbols.f90 | 5 +- 9 files changed, 129 insertions(+), 25 deletions(-) diff --git a/flang/include/flang/Lower/OpenMP.h b/flang/include/flang/Lower/OpenMP.h index df01a7b82c66c..962abd8952073 100644 --- a/flang/include/flang/Lower/OpenMP.h +++ b/flang/include/flang/Lower/OpenMP.h @@ -97,6 +97,13 @@ 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 04a063957082a..c8b0d74d86f86 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -777,6 +777,24 @@ 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, MapperDetails>; 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 68adf346fe8c0..29cd94ff284fd 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -447,6 +447,13 @@ 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/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index f86ee01355104..e75c48ab1c4f5 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), + // 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, @@ -4239,3 +4255,32 @@ 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; + + // 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 556259d1e5e63..dd2284ed9ffae 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()); @@ -1098,6 +1100,16 @@ 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 aa7527723dfb6..8a87add032fae 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1824,8 +1824,10 @@ 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->GetUltimate().detailsIf()}; - if (!misc || misc->kind() != MiscDetails::Kind::ConstructName) + auto &ultimate{symbol->GetUltimate()}; + auto *misc{ultimate.detailsIf()}; + auto *md{ultimate.detailsIf()}; + 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 @@ -1854,8 +1856,15 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec, // the type has been fully processed. BeginDeclTypeSpec(); auto &mapperName{std::get(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()}) { + md->AddDecl(declaratives_.back()); + } else if (mapperSym.has() || mapperSym.has()) { + mapperSym.set_details(MapperDetails{}); + mapperSym.get().AddDecl(declaratives_.back()); + } PushScope(Scope::Kind::OtherConstruct, nullptr); Walk(std::get(spec.t)); auto &varName{std::get(spec.t)}; @@ -3617,11 +3626,13 @@ void ModuleVisitor::Post(const parser::UseStmt &x) { } for (const auto &[name, symbol] : *useModuleScope_) { // Default USE imports public names, excluding intrinsic-only and most - // miscellaneous details. However, allow OpenMP mapper identifiers, - // which are currently represented with MiscDetails::ConstructName. - bool isMapper{false}; - if (const auto *misc{symbol->detailsIf()}) { - isMapper = misc->kind() == MiscDetails::Kind::ConstructName; + // 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) || diff --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp index 0ec44b7c40491..ed0715a422e78 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(); }, + [&](const MapperDetails &) { return has(); }, [](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); diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90 index a5fb0710592ce..860f0a3ccd613 100644 --- a/flang/test/Lower/OpenMP/declare-mapper.f90 +++ b/flang/test/Lower/OpenMP/declare-mapper.f90 @@ -7,7 +7,8 @@ ! 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 %t/omp-declare-mapper-7.f90 -o - | FileCheck %t/omp-declare-mapper-7.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 !--- omp-declare-mapper-1.f90 subroutine declare_mapper_1 @@ -303,8 +304,8 @@ subroutine declare_mapper_nested_parent !$omp end target end subroutine declare_mapper_nested_parent -!--- omp-declare-mapper-7.f90 -! Check mappers declared inside modules and used via USE association. +!--- omp-declare-mapper-7.mod.f90 +! Module with DECLARE MAPPER to be compiled separately module m_mod implicit none type :: mty @@ -313,13 +314,13 @@ module m_mod !$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 - - ! CHECK: omp.declare_mapper @[[MODMAP:[^ ]*mymap]] - ! CHECK: %{{.*}} = omp.map.info {{.*}} mapper(@[[MODMAP]]) {{.*}} {name = "a"} !$omp target map(mapper(mymap) : a) a%x = 42 !$omp end target diff --git a/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 b/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 index e57a5c0c1cea6..5d77540aa6453 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: Misc ConstructName +!CHECK: mymapper: MapperDetails !CHECK: ty: DerivedType components: x -!CHECK: ty.omp.default.mapper: Misc ConstructName +!CHECK: ty.omp.default.mapper: MapperDetails !CHECK: DerivedType scope: ty !CHECK: OtherConstruct scope: !CHECK: mapped (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty) @@ -21,4 +21,3 @@ program main !CHECK: maptwo (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty) end program main - From a94177716434a68e75d5c0807a55bdb57fa58c49 Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Fri, 24 Oct 2025 19:31:51 +0100 Subject: [PATCH 4/5] Address reviewer comments. Add test to check declare mapper export to .mod file. --- flang/lib/Lower/OpenMP/OpenMP.cpp | 8 ++ flang/lib/Semantics/resolve-names.cpp | 89 ++++++++++--------- .../OpenMP/declare-mapper-modfile.f90 | 14 +++ .../Semantics/OpenMP/map-clause-symbols.f90 | 14 +-- 4 files changed, 76 insertions(+), 49 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index e75c48ab1c4f5..8aa2fd12ab9d9 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -4271,6 +4271,14 @@ void Fortran::lower::materializeOpenMPDeclareMappers( if (!root.IsModule()) return; + // Only materialize for modules coming from mod files to avoid duplicates. + if (const semantics::Symbol *modSym = root.symbol()) { + if (!modSym->test(semantics::Symbol::Flag::ModFile)) + return; + } else { + return; + } + // Scan symbols in this module scope for MapperDetails. for (auto &it : root) { const semantics::Symbol &sym = *it.second; diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 8a87add032fae..e0f5c33b78b73 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -2384,7 +2384,7 @@ void AttrsVisitor::SetBindNameOn(Symbol &symbol) { } symbol.SetBindName(std::move(*label)); if (!oldBindName.empty()) { - if (const std::string *newBindName{symbol.GetBindName()}) { + if (const std::string * newBindName{symbol.GetBindName()}) { if (oldBindName != *newBindName) { Say(symbol.name(), "The entity '%s' has multiple BIND names ('%s' and '%s')"_err_en_US, @@ -2510,7 +2510,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) { // expression semantics if the DeclTypeSpec is a valid TypeSpec. // The grammar ensures that it's an intrinsic or derived type spec, // not TYPE(*) or CLASS(*) or CLASS(T). - if (const DeclTypeSpec *spec{state_.declTypeSpec}) { + if (const DeclTypeSpec * spec{state_.declTypeSpec}) { switch (spec->category()) { case DeclTypeSpec::Numeric: case DeclTypeSpec::Logical: @@ -2518,7 +2518,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) { typeSpec.declTypeSpec = spec; break; case DeclTypeSpec::TypeDerived: - if (const DerivedTypeSpec *derived{spec->AsDerived()}) { + if (const DerivedTypeSpec * derived{spec->AsDerived()}) { CheckForAbstractType(derived->typeSymbol()); // C703 typeSpec.declTypeSpec = spec; } @@ -3109,8 +3109,8 @@ Symbol &ScopeHandler::MakeSymbol(const parser::Name &name, Attrs attrs) { Symbol &ScopeHandler::MakeHostAssocSymbol( const parser::Name &name, const Symbol &hostSymbol) { Symbol &symbol{*NonDerivedTypeScope() - .try_emplace(name.source, HostAssocDetails{hostSymbol}) - .first->second}; + .try_emplace(name.source, HostAssocDetails{hostSymbol}) + .first->second}; name.symbol = &symbol; symbol.attrs() = hostSymbol.attrs(); // TODO: except PRIVATE, PUBLIC? // These attributes can be redundantly reapplied without error @@ -3198,7 +3198,7 @@ void ScopeHandler::ApplyImplicitRules( if (context().HasError(symbol) || !NeedsType(symbol)) { return; } - if (const DeclTypeSpec *type{GetImplicitType(symbol)}) { + if (const DeclTypeSpec * type{GetImplicitType(symbol)}) { if (!skipImplicitTyping_) { symbol.set(Symbol::Flag::Implicit); symbol.SetType(*type); @@ -3298,7 +3298,7 @@ const DeclTypeSpec *ScopeHandler::GetImplicitType( const auto *type{implicitRulesMap_->at(scope).GetType( symbol.name(), respectImplicitNoneType)}; if (type) { - if (const DerivedTypeSpec *derived{type->AsDerived()}) { + if (const DerivedTypeSpec * derived{type->AsDerived()}) { // Resolve any forward-referenced derived type; a quick no-op else. auto &instantiatable{*const_cast(derived)}; instantiatable.Instantiate(currScope()); @@ -4309,7 +4309,7 @@ Scope *ModuleVisitor::FindModule(const parser::Name &name, if (scope) { if (DoesScopeContain(scope, currScope())) { // 14.2.2(1) std::optional submoduleName; - if (const Scope *container{FindModuleOrSubmoduleContaining(currScope())}; + if (const Scope * container{FindModuleOrSubmoduleContaining(currScope())}; container && container->IsSubmodule()) { submoduleName = container->GetName(); } @@ -4414,7 +4414,7 @@ bool InterfaceVisitor::isAbstract() const { void InterfaceVisitor::AddSpecificProcs( const std::list &names, ProcedureKind kind) { - if (Symbol *symbol{GetGenericInfo().symbol}; + if (Symbol * symbol{GetGenericInfo().symbol}; symbol && symbol->has()) { for (const auto &name : names) { specificsForGenericProcs_.emplace(symbol, std::make_pair(&name, kind)); @@ -4514,7 +4514,7 @@ void GenericHandler::DeclaredPossibleSpecificProc(Symbol &proc) { } void InterfaceVisitor::ResolveNewSpecifics() { - if (Symbol *generic{genericInfo_.top().symbol}; + if (Symbol * generic{genericInfo_.top().symbol}; generic && generic->has()) { ResolveSpecificsInGeneric(*generic, false); } @@ -4599,7 +4599,7 @@ bool SubprogramVisitor::HandleStmtFunction(const parser::StmtFunctionStmt &x) { name.source); MakeSymbol(name, Attrs{}, UnknownDetails{}); } else if (auto *entity{ultimate.detailsIf()}; - entity && !ultimate.has()) { + entity && !ultimate.has()) { resultType = entity->type(); ultimate.details() = UnknownDetails{}; // will be replaced below } else { @@ -4658,7 +4658,7 @@ bool SubprogramVisitor::Pre(const parser::Suffix &suffix) { } else { Message &msg{Say(*suffix.resultName, "RESULT(%s) may appear only in a function"_err_en_US)}; - if (const Symbol *subprogram{InclusiveScope().symbol()}) { + if (const Symbol * subprogram{InclusiveScope().symbol()}) { msg.Attach(subprogram->name(), "Containing subprogram"_en_US); } } @@ -5174,7 +5174,7 @@ Symbol *ScopeHandler::FindSeparateModuleProcedureInterface( symbol = generic->specific(); } } - if (const Symbol *defnIface{FindSeparateModuleSubprogramInterface(symbol)}) { + if (const Symbol * defnIface{FindSeparateModuleSubprogramInterface(symbol)}) { // Error recovery in case of multiple definitions symbol = const_cast(defnIface); } @@ -5313,8 +5313,8 @@ bool SubprogramVisitor::HandlePreviousCalls( return generic->specific() && HandlePreviousCalls(name, *generic->specific(), subpFlag); } else if (const auto *proc{symbol.detailsIf()}; proc && - !proc->isDummy() && - !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) { + !proc->isDummy() && + !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) { // There's a symbol created for previous calls to this subprogram or // ENTRY's name. We have to replace that symbol in situ to avoid the // obligation to rewrite symbol pointers in the parse tree. @@ -5356,7 +5356,7 @@ const Symbol *SubprogramVisitor::CheckExtantProc( if (prev) { if (IsDummy(*prev)) { } else if (auto *entity{prev->detailsIf()}; - IsPointer(*prev) && entity && !entity->type()) { + IsPointer(*prev) && entity && !entity->type()) { // POINTER attribute set before interface } else if (inInterfaceBlock() && currScope() != prev->owner()) { // Procedures in an INTERFACE block do not resolve to symbols @@ -5428,7 +5428,7 @@ Symbol *SubprogramVisitor::PushSubprogramScope(const parser::Name &name, } set_inheritFromParent(false); // interfaces don't inherit, even if MODULE } - if (Symbol *found{FindSymbol(name)}; + if (Symbol * found{FindSymbol(name)}; found && found->has()) { found->set(subpFlag); // PushScope() created symbol } @@ -6276,9 +6276,9 @@ void DeclarationVisitor::Post(const parser::VectorTypeSpec &x) { vectorDerivedType.CookParameters(GetFoldingContext()); } - if (const DeclTypeSpec *extant{ - ppcBuiltinTypesScope->FindInstantiatedDerivedType( - vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) { + if (const DeclTypeSpec * + extant{ppcBuiltinTypesScope->FindInstantiatedDerivedType( + vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) { // This derived type and parameter expressions (if any) are already present // in the __ppc_intrinsics scope. SetDeclTypeSpec(*extant); @@ -6300,7 +6300,7 @@ bool DeclarationVisitor::Pre(const parser::DeclarationTypeSpec::Type &) { void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Type &type) { const parser::Name &derivedName{std::get(type.derived.t)}; - if (const Symbol *derivedSymbol{derivedName.symbol}) { + if (const Symbol * derivedSymbol{derivedName.symbol}) { CheckForAbstractType(*derivedSymbol); // C706 } } @@ -6369,8 +6369,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) { if (!spec->MightBeParameterized()) { spec->EvaluateParameters(context()); } - if (const DeclTypeSpec *extant{ - currScope().FindInstantiatedDerivedType(*spec, category)}) { + if (const DeclTypeSpec * + extant{currScope().FindInstantiatedDerivedType(*spec, category)}) { // This derived type and parameter expressions (if any) are already present // in this scope. SetDeclTypeSpec(*extant); @@ -6401,7 +6401,8 @@ void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Record &rec) { if (auto spec{ResolveDerivedType(typeName)}) { spec->CookParameters(GetFoldingContext()); spec->EvaluateParameters(context()); - if (const DeclTypeSpec *extant{currScope().FindInstantiatedDerivedType( + if (const DeclTypeSpec * + extant{currScope().FindInstantiatedDerivedType( *spec, DeclTypeSpec::TypeDerived)}) { SetDeclTypeSpec(*extant); } else { @@ -7412,7 +7413,7 @@ bool DeclarationVisitor::PassesLocalityChecks( "Coarray '%s' not allowed in a %s locality-spec"_err_en_US, specName); return false; } - if (const DeclTypeSpec *type{symbol.GetType()}) { + if (const DeclTypeSpec * type{symbol.GetType()}) { if (type->IsPolymorphic() && IsDummy(symbol) && !IsPointer(symbol) && !isReduce) { // F'2023 C1130 SayWithDecl(name, symbol, @@ -7639,7 +7640,7 @@ Symbol *DeclarationVisitor::NoteInterfaceName(const parser::Name &name) { } void DeclarationVisitor::CheckExplicitInterface(const parser::Name &name) { - if (const Symbol *symbol{name.symbol}) { + if (const Symbol * symbol{name.symbol}) { const Symbol &ultimate{symbol->GetUltimate()}; if (!context().HasError(*symbol) && !context().HasError(ultimate) && !BypassGeneric(ultimate).HasExplicitInterface()) { @@ -7957,7 +7958,7 @@ bool ConstructVisitor::Pre(const parser::DataStmtValue &x) { auto &mutableData{const_cast(data)}; if (auto *elem{parser::Unwrap(mutableData)}) { if (const auto *name{std::get_if(&elem->base.u)}) { - if (const Symbol *symbol{FindSymbol(*name)}; + if (const Symbol * symbol{FindSymbol(*name)}; symbol && symbol->GetUltimate().has()) { mutableData.u = elem->ConvertToStructureConstructor( DerivedTypeSpec{name->source, *symbol}); @@ -8103,15 +8104,15 @@ void ConstructVisitor::Post(const parser::SelectTypeStmt &x) { } } } else { - if (const Symbol *whole{ - UnwrapWholeSymbolDataRef(association.selector.expr)}) { + if (const Symbol * + whole{UnwrapWholeSymbolDataRef(association.selector.expr)}) { ConvertToObjectEntity(const_cast(*whole)); if (!IsVariableName(*whole)) { Say(association.selector.source, // C901 "Selector is not a variable"_err_en_US); association = {}; } - if (const DeclTypeSpec *type{whole->GetType()}) { + if (const DeclTypeSpec * type{whole->GetType()}) { if (!type->IsPolymorphic()) { // C1159 Say(association.selector.source, "Selector '%s' in SELECT TYPE statement must be " @@ -8251,8 +8252,8 @@ Symbol *ConstructVisitor::MakeAssocEntity() { "The associate name '%s' is already used in this associate statement"_err_en_US); return nullptr; } - } else if (const Symbol *whole{ - UnwrapWholeSymbolDataRef(association.selector.expr)}) { + } else if (const Symbol * + whole{UnwrapWholeSymbolDataRef(association.selector.expr)}) { symbol = &MakeSymbol(whole->name()); } else { return nullptr; @@ -8873,7 +8874,7 @@ bool DeclarationVisitor::CheckForHostAssociatedImplicit( if (name.symbol) { ApplyImplicitRules(*name.symbol, true); } - if (Scope *host{GetHostProcedure()}; host && !isImplicitNoneType(*host)) { + if (Scope * host{GetHostProcedure()}; host && !isImplicitNoneType(*host)) { Symbol *hostSymbol{nullptr}; if (!name.symbol) { if (currScope().CanImport(name.source)) { @@ -8944,7 +8945,7 @@ const parser::Name *DeclarationVisitor::FindComponent( if (!type) { return nullptr; // should have already reported error } - if (const IntrinsicTypeSpec *intrinsic{type->AsIntrinsic()}) { + if (const IntrinsicTypeSpec * intrinsic{type->AsIntrinsic()}) { auto category{intrinsic->category()}; MiscDetails::Kind miscKind{MiscDetails::Kind::None}; if (component.source == "kind") { @@ -8966,7 +8967,7 @@ const parser::Name *DeclarationVisitor::FindComponent( } } else if (DerivedTypeSpec * derived{type->AsDerived()}) { derived->Instantiate(currScope()); // in case of forward referenced type - if (const Scope *scope{derived->scope()}) { + if (const Scope * scope{derived->scope()}) { if (Resolve(component, scope->FindComponent(component.source))) { if (auto msg{CheckAccessibleSymbol(currScope(), *component.symbol)}) { context().Say(component.source, *msg); @@ -9117,8 +9118,8 @@ void DeclarationVisitor::PointerInitialization( if (evaluate::IsNullProcedurePointer(&*expr)) { CHECK(!details->init()); details->set_init(nullptr); - } else if (const Symbol *targetSymbol{ - evaluate::UnwrapWholeSymbolDataRef(*expr)}) { + } else if (const Symbol * + targetSymbol{evaluate::UnwrapWholeSymbolDataRef(*expr)}) { CHECK(!details->init()); details->set_init(*targetSymbol); } else { @@ -9686,7 +9687,7 @@ void ResolveNamesVisitor::EarlyDummyTypeDeclaration( for (const auto &ent : entities) { const auto &objName{std::get(ent.t)}; Resolve(objName, FindInScope(currScope(), objName)); - if (Symbol *symbol{objName.symbol}; + if (Symbol * symbol{objName.symbol}; symbol && IsDummy(*symbol) && NeedsType(*symbol)) { if (!type) { type = ProcessTypeSpec(declTypeSpec); @@ -9825,7 +9826,7 @@ void ResolveNamesVisitor::FinishSpecificationPart( if (auto *proc{symbol.detailsIf()}; proc && !proc->isDummy() && !IsPointer(symbol) && !symbol.attrs().test(Attr::BIND_C)) { - if (const Symbol *iface{proc->procInterface()}; + if (const Symbol * iface{proc->procInterface()}; iface && IsBindCProcedure(*iface)) { SetImplicitAttr(symbol, Attr::BIND_C); SetBindNameOn(symbol); @@ -9958,7 +9959,7 @@ bool ResolveNamesVisitor::Pre(const parser::PointerAssignmentStmt &x) { Symbol *ptrSymbol{parser::GetLastName(dataRef).symbol}; Walk(bounds); // Resolve unrestricted specific intrinsic procedures as in "p => cos". - if (const parser::Name *name{parser::Unwrap(expr)}) { + if (const parser::Name * name{parser::Unwrap(expr)}) { if (NameIsKnownOrIntrinsic(*name)) { if (Symbol * symbol{name->symbol}) { if (IsProcedurePointer(ptrSymbol) && @@ -10399,8 +10400,8 @@ void ResolveNamesVisitor::ResolveSpecificationParts(ProgramTree &node) { // implied SAVE so that evaluate::IsSaved() will return true. if (node.scope()->kind() == Scope::Kind::MainProgram) { if (const auto *object{symbol.detailsIf()}) { - if (const DeclTypeSpec *type{object->type()}) { - if (const DerivedTypeSpec *derived{type->AsDerived()}) { + if (const DeclTypeSpec * type{object->type()}) { + if (const DerivedTypeSpec * derived{type->AsDerived()}) { if (!IsSaved(symbol) && FindCoarrayPotentialComponent(*derived)) { SetImplicitAttr(symbol, Attr::SAVE); } @@ -10657,7 +10658,7 @@ void ResolveNamesVisitor::FinishDerivedTypeInstantiation(Scope &scope) { if (DerivedTypeSpec * spec{scope.derivedTypeSpec()}) { spec->Instantiate(currScope()); const Symbol &origTypeSymbol{spec->typeSymbol()}; - if (const Scope *origTypeScope{origTypeSymbol.scope()}) { + if (const Scope * origTypeScope{origTypeSymbol.scope()}) { CHECK(origTypeScope->IsDerivedType() && origTypeScope->symbol() == &origTypeSymbol); auto &foldingContext{GetFoldingContext()}; @@ -10668,7 +10669,7 @@ void ResolveNamesVisitor::FinishDerivedTypeInstantiation(Scope &scope) { if (IsPointer(comp)) { if (auto *details{comp.detailsIf()}) { auto origDetails{origComp.get()}; - if (const MaybeExpr &init{origDetails.init()}) { + if (const MaybeExpr & init{origDetails.init()}) { SomeExpr newInit{*init}; MaybeExpr folded{FoldExpr(std::move(newInit))}; details->set_init(std::move(folded)); diff --git a/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 b/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 new file mode 100644 index 0000000000000..480f87bc0f8e9 --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 @@ -0,0 +1,14 @@ +! 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/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90 index cdb65a71e8887..3b723e817ce87 100644 --- a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 +++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90 @@ -1,12 +1,16 @@ -! RUN: not %flang_fc1 -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s +! 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 - real(8) :: a(256) + type(ty) :: a(256) !$omp target map(mapper(xx), from:a) do i=1,n - a(i) = 4.2 + a(i)%x = 4.2 end do !$omp end target +!CHECK: xx: MapperDetails end program main - -! CHECK: error: '{{.*}}' not declared From cdf2bd8e06f9c51a8ece4eac5a1f8c2e0c5de1e8 Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Mon, 27 Oct 2025 14:01:52 +0000 Subject: [PATCH 5/5] Address copilot comments. --- flang/lib/Lower/OpenMP/OpenMP.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 8aa2fd12ab9d9..8322be64001ba 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -3579,8 +3579,8 @@ static void genOpenMPDeclareMapperImpl( mapperNameStr = converter.mangleName(mapperNameStr, sym->owner()); } - // If the mapper op already exists (e.g., created by regular lowering), - // do not recreate it. + // 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; @@ -4272,12 +4272,8 @@ void Fortran::lower::materializeOpenMPDeclareMappers( return; // Only materialize for modules coming from mod files to avoid duplicates. - if (const semantics::Symbol *modSym = root.symbol()) { - if (!modSym->test(semantics::Symbol::Flag::ModFile)) - return; - } else { + if (!root.symbol() || !root.symbol()->test(semantics::Symbol::Flag::ModFile)) return; - } // Scan symbols in this module scope for MapperDetails. for (auto &it : root) {