https://github.com/vhscampos updated https://github.com/llvm/llvm-project/pull/110657
>From 4c9756a323f67dbe5fbdb45cf2ddb5f97adad18d Mon Sep 17 00:00:00 2001 From: Victor Campos <victor.cam...@arm.com> Date: Thu, 26 Sep 2024 14:43:18 +0100 Subject: [PATCH 1/2] [Multilib] Custom flags YAML parsing This patch adds support for custom flags in the multilib YAML file. Details about this change can be found in: https://discourse.llvm.org/t/rfc-multilib-custom-flags/81058 CustomFlagDeclaration objects are instantiated using shared_ptr. This is motivated by the fact that each custom flag value, CustomFlagValueDetail, contains a back reference to their corresponding flag declaration. Since the CustomFlagDeclaration objects are transferred from the YAML parser to the MultilibSet instance after the parsing is finished, back references implemented as raw pointers would become dangling. This would need to be remediated in the copy/move constructors by updating the pointer. Therefore it's just simpler and less error-prone to have all references to CustomFlagDeclaration, including the back reference, as shared_ptr. This way dangling pointers are not a concern. --- clang/include/clang/Driver/Multilib.h | 28 +++- clang/lib/Driver/Multilib.cpp | 73 ++++++++-- ...remetal-multilib-custom-flags-parsing.yaml | 133 ++++++++++++++++++ 3 files changed, 223 insertions(+), 11 deletions(-) create mode 100644 clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h index dbed70f4f9008f..69d0b9b31714ff 100644 --- a/clang/include/clang/Driver/Multilib.h +++ b/clang/include/clang/Driver/Multilib.h @@ -101,6 +101,25 @@ class Multilib { raw_ostream &operator<<(raw_ostream &OS, const Multilib &M); +namespace custom_flag { +struct Declaration; +using DeclarationPtr = std::shared_ptr<Declaration>; + +struct ValueDetail { + std::string Name; + std::optional<SmallVector<std::string>> DriverArgs; + DeclarationPtr Decl; +}; + +struct Declaration { + std::string Name; + SmallVector<ValueDetail> ValueList; + std::optional<size_t> DefaultValueIdx; +}; + +static constexpr StringRef Prefix = "-fmultilib-flag="; +} // namespace custom_flag + /// See also MultilibSetBuilder for combining multilibs into a set. class MultilibSet { public: @@ -120,15 +139,18 @@ class MultilibSet { private: multilib_list Multilibs; - std::vector<FlagMatcher> FlagMatchers; + SmallVector<FlagMatcher> FlagMatchers; + SmallVector<custom_flag::DeclarationPtr> CustomFlagDecls; IncludeDirsFunc IncludeCallback; IncludeDirsFunc FilePathsCallback; public: MultilibSet() = default; MultilibSet(multilib_list &&Multilibs, - std::vector<FlagMatcher> &&FlagMatchers = {}) - : Multilibs(Multilibs), FlagMatchers(FlagMatchers) {} + SmallVector<FlagMatcher> &&FlagMatchers = {}, + SmallVector<custom_flag::DeclarationPtr> &&CustomFlagDecls = {}) + : Multilibs(std::move(Multilibs)), FlagMatchers(std::move(FlagMatchers)), + CustomFlagDecls(std::move(CustomFlagDecls)) {} const multilib_list &getMultilibs() { return Multilibs; } diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp index 0207e0f2eb2ded..0d7d590a0eed8b 100644 --- a/clang/lib/Driver/Multilib.cpp +++ b/clang/lib/Driver/Multilib.cpp @@ -10,6 +10,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Driver/Driver.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" @@ -201,13 +202,20 @@ struct MultilibGroupSerialization { struct MultilibSetSerialization { llvm::VersionTuple MultilibVersion; - std::vector<MultilibGroupSerialization> Groups; - std::vector<MultilibSerialization> Multilibs; - std::vector<MultilibSet::FlagMatcher> FlagMatchers; + SmallVector<MultilibGroupSerialization> Groups; + SmallVector<MultilibSerialization> Multilibs; + SmallVector<MultilibSet::FlagMatcher> FlagMatchers; + SmallVector<custom_flag::DeclarationPtr> CustomFlagDeclarations; }; } // end anonymous namespace +LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization) +LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization) +LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher) +LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::ValueDetail) +LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::DeclarationPtr) + template <> struct llvm::yaml::MappingTraits<MultilibSerialization> { static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) { io.mapOptional("Dir", V.Dir); @@ -255,11 +263,63 @@ template <> struct llvm::yaml::MappingTraits<MultilibSet::FlagMatcher> { } }; +template <> +struct llvm::yaml::MappingContextTraits<custom_flag::ValueDetail, + llvm::SmallSet<std::string, 32>> { + static void mapping(llvm::yaml::IO &io, custom_flag::ValueDetail &V, + llvm::SmallSet<std::string, 32> &) { + io.mapRequired("Name", V.Name); + io.mapOptional("DriverArgs", V.DriverArgs); + } + static std::string validate(IO &io, custom_flag::ValueDetail &V, + llvm::SmallSet<std::string, 32> &NameSet) { + if (V.Name.empty()) + return "custom flag value requires a name"; + if (!NameSet.insert(V.Name).second) + return "duplicate custom flag value name: \"" + V.Name + "\""; + return {}; + } +}; + +template <> +struct llvm::yaml::MappingContextTraits<custom_flag::DeclarationPtr, + llvm::SmallSet<std::string, 32>> { + static void mapping(llvm::yaml::IO &io, custom_flag::DeclarationPtr &V, + llvm::SmallSet<std::string, 32> &NameSet) { + assert(!V); + V = std::make_shared<custom_flag::Declaration>(); + io.mapRequired("Name", V->Name); + io.mapRequired("Values", V->ValueList, NameSet); + std::string DefaultValueName; + io.mapRequired("Default", DefaultValueName); + + for (auto [Idx, Value] : llvm::enumerate(V->ValueList)) { + Value.Decl = V; + if (Value.Name == DefaultValueName) { + assert(!V->DefaultValueIdx); + V->DefaultValueIdx = Idx; + } + } + } + static std::string validate(IO &io, custom_flag::DeclarationPtr &V, + llvm::SmallSet<std::string, 32> &) { + if (V->Name.empty()) + return "custom flag requires a name"; + if (V->ValueList.empty()) + return "custom flag must have at least one value"; + if (!V->DefaultValueIdx) + return "custom flag must have a default value"; + return {}; + } +}; + template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> { static void mapping(llvm::yaml::IO &io, MultilibSetSerialization &M) { io.mapRequired("MultilibVersion", M.MultilibVersion); io.mapRequired("Variants", M.Multilibs); io.mapOptional("Groups", M.Groups); + llvm::SmallSet<std::string, 32> NameSet; + io.mapOptionalWithContext("Flags", M.CustomFlagDeclarations, NameSet); io.mapOptional("Mappings", M.FlagMatchers); } static std::string validate(IO &io, MultilibSetSerialization &M) { @@ -288,10 +348,6 @@ template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> { } }; -LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization) -LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization) -LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher) - llvm::ErrorOr<MultilibSet> MultilibSet::parseYaml(llvm::MemoryBufferRef Input, llvm::SourceMgr::DiagHandlerTy DiagHandler, @@ -319,7 +375,8 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input, } } - return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers)); + return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers), + std::move(MS.CustomFlagDeclarations)); } LLVM_DUMP_METHOD void MultilibSet::dump() const { diff --git a/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml b/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml new file mode 100644 index 00000000000000..dc4d1b31a71165 --- /dev/null +++ b/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml @@ -0,0 +1,133 @@ +# RUN: split-file %s %t + +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-without-extra-build-args.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-with-extra-build-args.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s +# CHECK-NOT: error: + +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-name.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-NAME +# CHECK-MISSING-FLAG-NAME: error: custom flag requires a name + +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-values.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-VALUES +# CHECK-MISSING-FLAG-VALUES: error: custom flag must have at least one value + +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-value-default.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-VALUE-DEFAULT +# CHECK-MISSING-FLAG-VALUE-DEFAULT: error: custom flag must have a default value + +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-value-name.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-VALUE-NAME +# CHECK-MISSING-FLAG-VALUE-NAME: error: custom flag value requires a name + +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/duplicate-flag-value-name.yaml %s -### -o /dev/null 2>&1 \ +# RUN: | FileCheck %s --check-prefix=CHECK-DUPLICATE-FLAG-VALUE-NAME +# CHECK-DUPLICATE-FLAG-VALUE-NAME: error: duplicate custom flag value name: "value-name" +# CHECK-DUPLICATE-FLAG-VALUE-NAME-NEXT: - Name: value-name + +#--- multilib-without-extra-build-args.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=a] + +Flags: + - Name: flag + Values: + - Name: a + - Name: b + Default: a + +#--- multilib-with-extra-build-args.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=a] + +Flags: + - Name: flag + Values: + - Name: a + DriverArgs: [-DFEATURE_A] + - Name: b + DriverArgs: [-DFEATURE_B] + Default: a + +#--- missing-flag-name.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=a] + +Flags: + - Values: + - Name: a + Default: a + +#--- missing-flag-values.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=a] + +Flags: + - Name: flag + Values: + Default: a + +#--- missing-flag-value-default.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=a] + +Flags: + - Name: flag + Values: + - Name: a + Default: + +#--- missing-flag-value-name.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=a] + +Flags: + - Name: flag + Values: + - Name: + Default: a + +#--- duplicate-flag-value-name.yaml +--- +MultilibVersion: 1.0 + +Variants: +- Dir: libc + Flags: [-fmultilib-flag=value-name] + +Flags: + - Name: a + Values: + - Name: value-name + - Name: value-a + Default: value-name + - Name: b + Values: + - Name: value-name + Default: value-name >From 96e089e5b075cf5376109cae6eadea9b4b9a054d Mon Sep 17 00:00:00 2001 From: Victor Campos <victor.cam...@arm.com> Date: Mon, 9 Dec 2024 09:36:52 +0000 Subject: [PATCH 2/2] Change in specification: DriverArgs now MacroDefines We've decided to restrict the driver arguments that can be specified to only macro definitions. Hence this patch adjusts the YAML parsing for that. --- clang/include/clang/Driver/Multilib.h | 2 +- clang/lib/Driver/Multilib.cpp | 2 +- .../baremetal-multilib-custom-flags-parsing.yaml | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h index 69d0b9b31714ff..1dab45c062aeec 100644 --- a/clang/include/clang/Driver/Multilib.h +++ b/clang/include/clang/Driver/Multilib.h @@ -107,7 +107,7 @@ using DeclarationPtr = std::shared_ptr<Declaration>; struct ValueDetail { std::string Name; - std::optional<SmallVector<std::string>> DriverArgs; + std::optional<SmallVector<std::string>> MacroDefines; DeclarationPtr Decl; }; diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp index 0d7d590a0eed8b..b4b5dbd1bdb5e3 100644 --- a/clang/lib/Driver/Multilib.cpp +++ b/clang/lib/Driver/Multilib.cpp @@ -269,7 +269,7 @@ struct llvm::yaml::MappingContextTraits<custom_flag::ValueDetail, static void mapping(llvm::yaml::IO &io, custom_flag::ValueDetail &V, llvm::SmallSet<std::string, 32> &) { io.mapRequired("Name", V.Name); - io.mapOptional("DriverArgs", V.DriverArgs); + io.mapOptional("MacroDefines", V.MacroDefines); } static std::string validate(IO &io, custom_flag::ValueDetail &V, llvm::SmallSet<std::string, 32> &NameSet) { diff --git a/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml b/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml index dc4d1b31a71165..fe6a9a8d7f1ee7 100644 --- a/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml +++ b/clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml @@ -1,8 +1,8 @@ # RUN: split-file %s %t -# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-without-extra-build-args.yaml %s -### -o /dev/null 2>&1 \ +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-without-macro-defines.yaml %s -### -o /dev/null 2>&1 \ # RUN: | FileCheck %s -# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-with-extra-build-args.yaml %s -### -o /dev/null 2>&1 \ +# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-with-macro-defines.yaml %s -### -o /dev/null 2>&1 \ # RUN: | FileCheck %s # CHECK-NOT: error: @@ -27,7 +27,7 @@ # CHECK-DUPLICATE-FLAG-VALUE-NAME: error: duplicate custom flag value name: "value-name" # CHECK-DUPLICATE-FLAG-VALUE-NAME-NEXT: - Name: value-name -#--- multilib-without-extra-build-args.yaml +#--- multilib-without-macro-defines.yaml --- MultilibVersion: 1.0 @@ -42,7 +42,7 @@ Flags: - Name: b Default: a -#--- multilib-with-extra-build-args.yaml +#--- multilib-with-macro-defines.yaml --- MultilibVersion: 1.0 @@ -54,9 +54,9 @@ Flags: - Name: flag Values: - Name: a - DriverArgs: [-DFEATURE_A] + MacroDefines: [FEATURE_A] - Name: b - DriverArgs: [-DFEATURE_B] + MacroDefines: [FEATURE_B] Default: a #--- missing-flag-name.yaml _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
