llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-parser @llvm/pr-subscribers-flang-semantics Author: Kareem Ergawy (ergawy) <details> <summary>Changes</summary> An implicit attribute the compiler applied under `-gpu=mem:managed` is written into the module file the same way a user-written one is, so a reader cannot tell them apart. It then treats the attribute as a user requirement: allocating such a component in a DEVICE object is rejected, and the memory space the user did ask for on the object no longer wins. Spell the distinction in the module file as `MANAGED(IMPLICIT)`, modelled on `INTENT(IN)`: CUDA-data-attr gains an optional parenthesized qualifier, carried by a new CUDADataAttrSpec parse-tree node in AttrSpec and ComponentAttrSpec. ATTRIBUTES(...) keeps the bare attribute, so the qualifier cannot be written there. The attribute itself is still written out, so a component keeps the same memory space no matter which options a consumer is compiled with. Also stop an implicitly applied attribute from making a module a definer of CUDA symbols. Without this, adding `-gpu=mem:managed` to a module's build rejects its OpenACC-only consumers over an attribute the user never wrote. --- Full diff: https://github.com/llvm/llvm-project/pull/224601.diff 8 Files Affected: - (modified) flang/include/flang/Parser/dump-parse-tree.h (+2) - (modified) flang/include/flang/Parser/parse-tree.h (+13-2) - (modified) flang/lib/Parser/Fortran-parsers.cpp (+10-2) - (modified) flang/lib/Parser/unparse.cpp (+6) - (modified) flang/lib/Semantics/mod-file.cpp (+19-3) - (modified) flang/lib/Semantics/resolve-names.cpp (+29-8) - (modified) flang/test/Parser/cuf-sanity-tree.CUF (+4-2) - (added) flang/test/Semantics/CUDA/cuda-managed-implicit-modfile.cuf (+76) ``````````diff diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 3b4d467f14345..7ca404663b486 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -63,6 +63,8 @@ class ParseTreeDumper { NODE(std, int64_t) NODE(std, uint64_t) NODE_ENUM(common, CUDADataAttr) + NODE(parser, CUDADataAttrSpec) + NODE(CUDADataAttrSpec, Implicit) NODE_ENUM(common, CUDASubprogramAttrs) NODE_ENUM(common, ImportKind) NODE_ENUM(common, OmpDependenceKind) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 3e9a035d0bfee..207a5543ab775 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -999,10 +999,21 @@ struct ComponentArraySpec { EMPTY_CLASS(Allocatable); EMPTY_CLASS(Pointer); EMPTY_CLASS(Contiguous); +// CUDA-data-attr [( IMPLICIT )] +// The (IMPLICIT) qualifier marks an attribute that the compiler applied on the +// user's behalf (e.g. an unattributed ALLOCATABLE under -gpu=mem:managed) +// rather than one the user wrote. It exists so that module files can carry +// that distinction; user code is not expected to spell it. +struct CUDADataAttrSpec { + TUPLE_CLASS_BOILERPLATE(CUDADataAttrSpec); + EMPTY_CLASS(Implicit); + std::tuple<common::CUDADataAttr, std::optional<Implicit>> t; +}; + struct ComponentAttrSpec { UNION_CLASS_BOILERPLATE(ComponentAttrSpec); std::variant<AccessSpec, Allocatable, CoarraySpec, Contiguous, - ComponentArraySpec, Pointer, common::CUDADataAttr, ErrorRecovery> + ComponentArraySpec, Pointer, CUDADataAttrSpec, ErrorRecovery> u; }; @@ -1412,7 +1423,7 @@ struct AttrSpec { std::variant<AccessSpec, Allocatable, Asynchronous, CoarraySpec, Contiguous, ArraySpec, External, IntentSpec, Intrinsic, LanguageBindingSpec, Optional, Parameter, Pointer, Protected, RankClause, Save, Target, Value, Volatile, - common::CUDADataAttr> + CUDADataAttrSpec> u; }; diff --git a/flang/lib/Parser/Fortran-parsers.cpp b/flang/lib/Parser/Fortran-parsers.cpp index a20983e095d18..af62a15edd1f5 100644 --- a/flang/lib/Parser/Fortran-parsers.cpp +++ b/flang/lib/Parser/Fortran-parsers.cpp @@ -477,7 +477,7 @@ TYPE_PARSER(construct<ComponentAttrSpec>(accessSpec) || construct<ComponentAttrSpec>("DIMENSION" >> componentArraySpec) || construct<ComponentAttrSpec>(pointer) || extension<LanguageFeature::CUDA>( - construct<ComponentAttrSpec>(Parser<common::CUDADataAttr>{})) || + construct<ComponentAttrSpec>(Parser<CUDADataAttrSpec>{})) || construct<ComponentAttrSpec>(recovery( fail<ErrorRecovery>( "type parameter definitions must appear before component declarations"_err_en_US), @@ -764,7 +764,15 @@ TYPE_PARSER(construct<AttrSpec>(accessSpec) || construct<AttrSpec>(construct<Value>("VALUE"_tok)) || construct<AttrSpec>(construct<Volatile>("VOLATILE"_tok)) || extension<LanguageFeature::CUDA>( - construct<AttrSpec>(Parser<common::CUDADataAttr>{}))) + construct<AttrSpec>(Parser<CUDADataAttrSpec>{}))) + +// CUDA-data-attr-spec -> CUDA-data-attr [( IMPLICIT )] +// The parenthesized qualifier marks a compiler-applied attribute; it is +// emitted into module files so the distinction survives, and is not meant to +// be written in user code. +TYPE_PARSER(construct<CUDADataAttrSpec>(Parser<common::CUDADataAttr>{}, + maybe(parenthesized( + construct<CUDADataAttrSpec::Implicit>("IMPLICIT" >> ok))))) // CUDA-data-attr -> // CONSTANT | DEVICE | MANAGED | PINNED | SHARED | TEXTURE | UNIFIED diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index d075a77e17b43..a046c08e710c6 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2910,6 +2910,12 @@ class UnparseVisitor { WALK_NESTED_ENUM(AccDataModifier, Modifier) WALK_NESTED_ENUM(AccessSpec, Kind) // R807 WALK_NESTED_ENUM(common, TypeParamAttr) // R734 + void Unparse(const CUDADataAttrSpec &x) { // CUDA + Walk(std::get<common::CUDADataAttr>(x.t)); + if (std::get<std::optional<CUDADataAttrSpec::Implicit>>(x.t)) { + Word("(IMPLICIT)"); + } + } WALK_NESTED_ENUM(common, CUDADataAttr) // CUDA WALK_NESTED_ENUM(common, CUDASubprogramAttrs) // CUDA WALK_NESTED_ENUM(common, OmpDependenceKind) diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp index e17bfb2ee3f32..a30581b30b892 100644 --- a/flang/lib/Semantics/mod-file.cpp +++ b/flang/lib/Semantics/mod-file.cpp @@ -1418,6 +1418,12 @@ void ModFileWriter::PutEntity(llvm::raw_ostream &os, const Symbol &symbol, if (const auto *details{symbol.detailsIf<ObjectEntityDetails>()}) { if (auto attr{details->cudaDataAttr()}) { PutLower(os << ',', common::EnumToString(*attr)); + // Record that the compiler applied this attribute, so that a reader can + // tell it from one the user wrote and let an explicit memory space on an + // enclosing object take precedence over it. + if (details->cudaDataAttrIsImplicit()) { + os << "(implicit)"; + } } } if (symbol.owner().kind() == Scope::Kind::DerivedType && @@ -2010,18 +2016,28 @@ static std::optional<SourceName> GetSubmoduleParent( } } +// Does this symbol carry a CUDA data attribute the user actually wrote? An +// attribute the compiler applied on the user's behalf does not make the module +// a definer of CUDA symbols: the user wrote no CUDA Fortran, so a consumer +// without CUDA enabled has nothing to object to. +static bool HasExplicitCUDADataAttr(const Symbol &symbol) { + const auto *object{symbol.detailsIf<ObjectEntityDetails>()}; + return object && object->cudaDataAttr() && !object->cudaDataAttrIsImplicit(); +} + static bool ScopeHasCUDAModuleVariables(const Scope &scope) { for (const auto &[_, symbolRef] : scope) { const Symbol &symbol{*symbolRef}; if (const auto *object{symbol.detailsIf<ObjectEntityDetails>()}) { - if (object->cudaDataAttr()) { + if (HasExplicitCUDADataAttr(symbol)) { return true; } const DeclTypeSpec *type{object->type()}; const DerivedTypeSpec *derived{type ? type->AsDerived() : nullptr}; if (derived && - FindUltimateComponent(*derived, - [](const Symbol &component) { return HasCUDAAttr(component); })) { + FindUltimateComponent(*derived, [](const Symbol &component) { + return HasExplicitCUDADataAttr(component); + })) { return true; } } diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index c5f7fba49fffb..4bdeb52f1cad8 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -262,6 +262,7 @@ class AttrsVisitor : public virtual BaseVisitor { bool BeginAttrs(); // always returns true Attrs GetAttrs(); std::optional<common::CUDADataAttr> cudaDataAttr() { return cudaDataAttr_; } + bool cudaDataAttrIsImplicit() const { return cudaDataAttrIsImplicit_; } Attrs EndAttrs(); bool SetPassNameOn(Symbol &); void SetBindNameOn(Symbol &); @@ -304,10 +305,12 @@ class AttrsVisitor : public virtual BaseVisitor { HANDLE_ATTR_CLASS(Volatile, VOLATILE) #undef HANDLE_ATTR_CLASS bool Pre(const common::CUDADataAttr); + bool Pre(const parser::CUDADataAttrSpec::Implicit &); protected: std::optional<Attrs> attrs_; std::optional<common::CUDADataAttr> cudaDataAttr_; + bool cudaDataAttrIsImplicit_{false}; Attr AccessSpecToAttr(const parser::AccessSpec &x) { switch (x.v) { @@ -777,7 +780,8 @@ class ScopeHandler : public ImplicitRulesVisitor { symbol.implicitAttrs().set(attr); } void SetCUDADataAttr( - SourceName, Symbol &, std::optional<common::CUDADataAttr>); + SourceName, Symbol &, std::optional<common::CUDADataAttr>, + bool isImplicit = false); protected: FuncResultStack &funcResultStack() { return funcResultStack_; } @@ -2586,6 +2590,7 @@ Attrs AttrsVisitor::EndAttrs() { Attrs result{GetAttrs()}; attrs_.reset(); cudaDataAttr_.reset(); + cudaDataAttrIsImplicit_ = false; passName_ = std::nullopt; bindName_.reset(); isCDefined_ = false; @@ -2726,6 +2731,12 @@ bool AttrsVisitor::Pre(const common::CUDADataAttr x) { cudaDataAttr_ = x; return false; } +bool AttrsVisitor::Pre(const parser::CUDADataAttrSpec::Implicit &) { + // The (IMPLICIT) qualifier only appears in module files, marking an + // attribute this compiler applied rather than one the user wrote. + cudaDataAttrIsImplicit_ = true; + return false; +} // DeclTypeSpecVisitor implementation @@ -3836,7 +3847,7 @@ bool ScopeHandler::CheckDuplicatedAttrs( } void ScopeHandler::SetCUDADataAttr(SourceName source, Symbol &symbol, - std::optional<common::CUDADataAttr> attr) { + std::optional<common::CUDADataAttr> attr, bool isImplicit) { if (attr) { ConvertToObjectEntity(symbol); if (auto *object{symbol.detailsIf<ObjectEntityDetails>()}) { @@ -3847,6 +3858,7 @@ void ScopeHandler::SetCUDADataAttr(SourceName source, Symbol &symbol, std::string{common::EnumToString(*object->cudaDataAttr())}.c_str()); } else { object->set_cudaDataAttr(attr); + object->set_cudaDataAttrIsImplicit(isImplicit); } } else { Say(source, @@ -5772,7 +5784,8 @@ void SubprogramVisitor::PostEntryStmt(const parser::EntryStmt &stmt) { } SubprogramDetails &entryDetails{entrySymbol.get<SubprogramDetails>()}; CHECK(entryDetails.entryScope() == &inclusiveScope); - SetCUDADataAttr(name.source, entrySymbol, cudaDataAttr()); + SetCUDADataAttr( + name.source, entrySymbol, cudaDataAttr(), cudaDataAttrIsImplicit()); entrySymbol.attrs() |= GetAttrs(); SetBindNameOn(entrySymbol); for (const auto &dummyArg : std::get<std::list<parser::DummyArg>>(stmt.t)) { @@ -6243,7 +6256,8 @@ void DeclarationVisitor::Post(const parser::EntityDecl &x) { attrs.set(Attr::INTRINSIC, false); // dealt with in Pre(TypeDeclarationStmt) Symbol &symbol{DeclareUnknownEntity(name, attrs)}; symbol.ReplaceName(name.source); - SetCUDADataAttr(name.source, symbol, cudaDataAttr()); + SetCUDADataAttr( + name.source, symbol, cudaDataAttr(), cudaDataAttrIsImplicit()); if (const auto &init{std::get<std::optional<parser::Initialization>>(x.t)}) { ConvertToObjectEntity(symbol) || ConvertToProcEntity(symbol); symbol.set( @@ -6686,6 +6700,8 @@ bool DeclarationVisitor::Pre(const parser::CUDAAttributesStmt &x) { if (attr == common::CUDADataAttr::Value) { SetExplicitAttr(*symbol, Attr::VALUE); } else { + // ATTRIBUTES(...) carries a bare CUDA-data-attr, with no place for + // the (IMPLICIT) qualifier, so such an attribute is always the user's. SetCUDADataAttr(name.source, *symbol, attr); } } @@ -7600,7 +7616,8 @@ void DeclarationVisitor::Post(const parser::ComponentDecl &x) { } if (OkToAddComponent(name)) { auto &symbol{DeclareObjectEntity(name, attrs)}; - SetCUDADataAttr(name.source, symbol, cudaDataAttr()); + SetCUDADataAttr( + name.source, symbol, cudaDataAttr(), cudaDataAttrIsImplicit()); // Implicitely attribute allocatable/pointer components with `managed` // memory if CUDA and `-gpu=mem:managed` are enabled. @@ -7734,7 +7751,8 @@ void DeclarationVisitor::Post(const parser::ProcDecl &x) { attrs.set(Attr::PRIVATE); } Symbol &symbol{DeclareProcEntity(name, attrs, procInterface)}; - SetCUDADataAttr(name.source, symbol, cudaDataAttr()); // for error + SetCUDADataAttr( + name.source, symbol, cudaDataAttr(), cudaDataAttrIsImplicit()); // for error symbol.ReplaceName(name.source); if (dtDetails) { dtDetails->add_component(symbol); @@ -8717,7 +8735,8 @@ Symbol *DeclarationVisitor::MakeTypeSymbol( attrs.set(Attr::PRIVATE); } Symbol &result{MakeSymbol(name, attrs, std::move(details))}; - SetCUDADataAttr(name, result, cudaDataAttr()); + + SetCUDADataAttr(name, result, cudaDataAttr(), cudaDataAttrIsImplicit()); return &result; } } @@ -10992,8 +11011,10 @@ void ResolveNamesVisitor::FinishSpecificationPart( if (context().languageFeatures().IsEnabled( common::LanguageFeature::CUDA)) { if (context().languageFeatures().IsEnabled( - common::LanguageFeature::CudaManaged)) + common::LanguageFeature::CudaManaged)) { object->set_cudaDataAttr(common::CUDADataAttr::Managed); + object->set_cudaDataAttrIsImplicit(); + } // Implicitly treat allocatable arrays as pinned when feature is // enabled. else if (IsAllocatable(symbol) && diff --git a/flang/test/Parser/cuf-sanity-tree.CUF b/flang/test/Parser/cuf-sanity-tree.CUF index b4d53f27cf395..5acd8f2b0cc48 100644 --- a/flang/test/Parser/cuf-sanity-tree.CUF +++ b/flang/test/Parser/cuf-sanity-tree.CUF @@ -21,7 +21,8 @@ include "cuf-sanity-common" !CHECK: | | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt !CHECK: | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> Real !CHECK: | | | AttrSpec -> Allocatable -!CHECK: | | | AttrSpec -> CUDADataAttr = Pinned +!CHECK: | | | AttrSpec -> CUDADataAttrSpec +!CHECK: | | | | CUDADataAttr = Pinned !CHECK: | | | EntityDecl !CHECK: | | | | Name = 'pa' !CHECK: | | | | ArraySpec -> DeferredShapeSpecList -> int @@ -111,7 +112,8 @@ include "cuf-sanity-common" !CHECK: | | | | | Name = 'devx1' !CHECK: | | | | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt !CHECK: | | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> Real -!CHECK: | | | | | AttrSpec -> CUDADataAttr = Device +!CHECK: | | | | | AttrSpec -> CUDADataAttrSpec +!CHECK: | | | | | | CUDADataAttr = Device !CHECK: | | | | | EntityDecl !CHECK: | | | | | | Name = 'devx2' !CHECK: | | | ExecutionPart -> Block diff --git a/flang/test/Semantics/CUDA/cuda-managed-implicit-modfile.cuf b/flang/test/Semantics/CUDA/cuda-managed-implicit-modfile.cuf new file mode 100644 index 0000000000000..ca5e8037d5e75 --- /dev/null +++ b/flang/test/Semantics/CUDA/cuda-managed-implicit-modfile.cuf @@ -0,0 +1,76 @@ +! Under -gpu=managed the compiler attributes unattributed ALLOCATABLE and +! POINTER entities as managed on the user's behalf. A module file records that +! it did so, with the (IMPLICIT) qualifier, so that a reader can tell such an +! attribute from one the user wrote. + +! RUN: rm -rf %t && split-file %s %t +! RUN: cd %t && bbc -emit-hlfir -fcuda -gpu=managed def.cuf -o /dev/null +! RUN: cat %t/m.mod | FileCheck %s --check-prefix=MODFILE + +! The memory space the user asked for on an enclosing object wins over an +! implicitly attributed component, even across the module file. +! RUN: cd %t && bbc -emit-hlfir -fcuda -gpu=managed use_device.cuf -o - \ +! RUN: | FileCheck %s --check-prefix=DEVICE + +! With no enclosing object asking for a space, the implicit attribute applies, +! and it does so whether or not the consumer repeats -gpu=managed. +! RUN: cd %t && bbc -emit-hlfir -fcuda -gpu=managed use_host.cuf -o - \ +! RUN: | FileCheck %s --check-prefix=HOST +! RUN: cd %t && bbc -emit-hlfir -fcuda use_host.cuf -o - \ +! RUN: | FileCheck %s --check-prefix=HOST + +! An attribute the compiler applied does not make the module a definer of CUDA +! symbols, so a consumer without CUDA Fortran enabled is not rejected. +! RUN: cd %t && bbc -emit-hlfir -fopenacc use_acc.cuf -o /dev/null + +!--- def.cuf +module m + ! Every component here was attributed by the compiler, so an enclosing + ! object's own memory space takes precedence over them. + type :: t + real, allocatable :: implicit_comp(:) + end type + ! A component the user attributed, kept apart so that it does not affect + ! where an object of type t is placed. + type :: t_explicit + real, allocatable, managed :: explicit_comp(:) + end type + real, allocatable :: implicit_var(:) +end module + +! The compiler applied the attribute here, and the module file says so. +! MODFILE: real(4),allocatable,managed(implicit)::implicit_comp(:) +! The user wrote this one, so it is recorded without the qualifier. +! MODFILE: real(4),allocatable,managed::explicit_comp(:) +! The same distinction is kept for an entity in the module's own scope. +! MODFILE: real(4),allocatable,managed(implicit)::implicit_var(:) + +!--- use_device.cuf +subroutine device_object() + use m + type(t), device :: d + allocate(d%implicit_comp(10)) + deallocate(d%implicit_comp) +end subroutine + +! DEVICE-LABEL: func.func @_QPdevice_object() +! DEVICE: cuf.alloc {{.*}} {bindc_name = "d", data_attr = #cuf.cuda<device> +! DEVICE: fir.embox {{.*}} {allocator_idx = 2 : i32} +! DEVICE: cuf.allocate {{.*}} {data_attr = #cuf.cuda<device>} -> i32 +! DEVICE: cuf.deallocate {{.*}} {data_attr = #cuf.cuda<device>} -> i32 + +!--- use_host.cuf +subroutine host_object() + use m + type(t) :: h + allocate(h%implicit_comp(10)) +end subroutine + +! HOST-LABEL: func.func @_QPhost_object() +! HOST: fir.embox {{.*}} {allocator_idx = 3 : i32} +! HOST: cuf.allocate {{.*}} {data_attr = #cuf.cuda<managed>} -> i32 + +!--- use_acc.cuf +subroutine acc_only() + use m +end subroutine `````````` </details> https://github.com/llvm/llvm-project/pull/224601 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
