[clang] [Multilib] Custom flags YAML parsing (PR #110657)
Victor Campos via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 11 05:30:08 PST 2024
https://github.com/vhscampos updated https://github.com/llvm/llvm-project/pull/110657
>From 3914f0f5afbc8549dbc3557b70abb21f96be2db7 Mon Sep 17 00:00:00 2001
From: Victor Campos <victor.campos at 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 42c7ca165d9a6dc82395ce729629c9167b52850e Mon Sep 17 00:00:00 2001
From: Victor Campos <victor.campos at 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
More information about the cfe-commits
mailing list