[clang] [Multilib] Custom flags YAML parsing (PR #110657)
Victor Campos via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 24 02:14:02 PDT 2024
https://github.com/vhscampos updated https://github.com/llvm/llvm-project/pull/110657
>From e194bdad39ea7f719e1a133eca94f9ce6ef3e881 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] [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 | 30 +++-
clang/lib/Driver/Multilib.cpp | 80 +++++++++--
...remetal-multilib-custom-flags-parsing.yaml | 133 ++++++++++++++++++
3 files changed, 230 insertions(+), 13 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..333b1d2b555bd9 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 CustomFlagDeclaration;
+using CustomFlagDeclarationPtr = std::shared_ptr<CustomFlagDeclaration>;
+
+struct CustomFlagValueDetail {
+ std::string Name;
+ std::optional<SmallVector<std::string>> ExtraBuildArgs;
+ CustomFlagDeclarationPtr Decl;
+};
+
+struct CustomFlagDeclaration {
+ std::string Name;
+ SmallVector<CustomFlagValueDetail> ValueList;
+ size_t DefaultValueIdx = ~0UL;
+};
+
+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::CustomFlagDeclarationPtr> CustomFlagDecls;
IncludeDirsFunc IncludeCallback;
IncludeDirsFunc FilePathsCallback;
public:
MultilibSet() = default;
- MultilibSet(multilib_list &&Multilibs,
- std::vector<FlagMatcher> &&FlagMatchers = {})
- : Multilibs(Multilibs), FlagMatchers(FlagMatchers) {}
+ MultilibSet(
+ multilib_list &&Multilibs, SmallVector<FlagMatcher> &&FlagMatchers = {},
+ SmallVector<custom_flag::CustomFlagDeclarationPtr> &&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 c56417c6f6d0b0..81fe97517b0f91 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -11,7 +11,7 @@
#include "clang/Basic/Version.h"
#include "clang/Driver/Driver.h"
#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/Error.h"
@@ -205,13 +205,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::CustomFlagDeclarationPtr> 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::CustomFlagValueDetail)
+LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::CustomFlagDeclarationPtr)
+
template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
io.mapOptional("Dir", V.Dir);
@@ -259,11 +266,69 @@ template <> struct llvm::yaml::MappingTraits<MultilibSet::FlagMatcher> {
}
};
+template <>
+struct llvm::yaml::MappingContextTraits<custom_flag::CustomFlagValueDetail,
+ llvm::SmallSet<std::string, 32>> {
+ static void mapping(llvm::yaml::IO &io, custom_flag::CustomFlagValueDetail &V,
+ llvm::SmallSet<std::string, 32> &) {
+ io.mapRequired("Name", V.Name);
+ io.mapOptional("ExtraBuildArgs", V.ExtraBuildArgs);
+ }
+ static std::string validate(IO &io, custom_flag::CustomFlagValueDetail &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::CustomFlagDeclarationPtr,
+ llvm::SmallSet<std::string, 32>> {
+ static void mapping(llvm::yaml::IO &io,
+ custom_flag::CustomFlagDeclarationPtr &V,
+ llvm::SmallSet<std::string, 32> &NameSet) {
+ assert(!V);
+ V = std::make_shared<custom_flag::CustomFlagDeclaration>();
+ 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 == ~0UL);
+ V->DefaultValueIdx = Idx;
+ }
+ }
+ }
+ static std::string validate(IO &io, custom_flag::CustomFlagDeclarationPtr &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 >= V->ValueList.size())
+ return "custom flag must have a default value";
+ if (llvm::any_of(V->ValueList, [&V](const auto &Value) {
+ return !Value.Decl || Value.Decl != V;
+ }))
+ return "custom flag value missing reference to its custom flag "
+ "declaration";
+ 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) {
@@ -292,10 +357,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,
@@ -323,7 +384,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..4d751cfe7c9edd
--- /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
+ ExtraBuildArgs: [-DFEATURE_A]
+ - Name: b
+ ExtraBuildArgs: [-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
More information about the cfe-commits
mailing list