[clang] d30bc9e - [Driver] Change multilib selection algorithm
Michael Platings via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 23 23:58:51 PDT 2023
Author: Michael Platings
Date: 2023-03-24T06:58:07Z
New Revision: d30bc9e91241d69410fe1a878a66438dd752014f
URL: https://github.com/llvm/llvm-project/commit/d30bc9e91241d69410fe1a878a66438dd752014f
DIFF: https://github.com/llvm/llvm-project/commit/d30bc9e91241d69410fe1a878a66438dd752014f.diff
LOG: [Driver] Change multilib selection algorithm
The new algorithm is:
1. Find all multilibs with flags that are a subset of the requested
flags.
2. If more than one multilib matches, choose the last.
In addition a new selection mechanism is permitted via an overload of
MultilibSet::select() for which multiple multilibs are returned.
This allows layering multilibs on top of each other.
Since multilibs are now ordered within a list, they no longer need a
Priority field.
The new algorithm is different to the old algorithm, but in practise
the old algorithm was always used in such a way that the effect is the
same.
The old algorithm was to find the set intersection of the requested
flags (with the first character of each removed) with each multilib's
flags (ditto), and for that intersection check whether the first
character matched. However, ignoring the first characters, the
requested flags were always a superset of all the multilibs flags.
Therefore the new algorithm can be used as a drop-in replacement.
The exception is Fuchsia, which needs adjusting slightly to set both
fexceptions and fno-exceptions flags.
Differential Revision: https://reviews.llvm.org/D142905
Added:
Modified:
clang/include/clang/Driver/Multilib.h
clang/include/clang/Driver/MultilibBuilder.h
clang/lib/Driver/Multilib.cpp
clang/lib/Driver/MultilibBuilder.cpp
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/lib/Driver/ToolChains/OHOS.cpp
clang/unittests/Driver/MultilibTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h
index feb12f3638d34..9d6f1d23696b8 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -36,14 +36,13 @@ class Multilib {
std::string OSSuffix;
std::string IncludeSuffix;
flags_list Flags;
- int Priority;
public:
/// GCCSuffix, OSSuffix & IncludeSuffix will be appended directly to the
/// sysroot string so they must either be empty or begin with a '/' character.
/// This is enforced with an assert in the constructor.
Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
- StringRef IncludeSuffix = {}, int Priority = 0,
+ StringRef IncludeSuffix = {},
const flags_list &Flags = flags_list());
/// Get the detected GCC installation path suffix for the multi-arch
@@ -62,10 +61,6 @@ class Multilib {
/// All elements begin with either '+' or '-'
const flags_list &flags() const { return Flags; }
- /// Returns the multilib priority. When more than one multilib matches flags,
- /// the one with the highest priority is selected, with 0 being the default.
- int priority() const { return Priority; }
-
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
@@ -108,6 +103,9 @@ class MultilibSet {
const_iterator begin() const { return Multilibs.begin(); }
const_iterator end() const { return Multilibs.end(); }
+ /// Select compatible variants
+ multilib_list select(const Multilib::flags_list &Flags) const;
+
/// Pick the best multilib in the set, \returns false if none are compatible
bool select(const Multilib::flags_list &Flags, Multilib &M) const;
@@ -129,13 +127,6 @@ class MultilibSet {
}
const IncludeDirsFunc &filePathsCallback() const { return FilePathsCallback; }
-
-private:
- /// Apply the filter to Multilibs and return the subset that remains
- static multilib_list filterCopy(FilterCallback F, const multilib_list &Ms);
-
- /// Apply the filter to the multilib_list, removing those that don't match
- static void filterInPlace(FilterCallback F, multilib_list &Ms);
};
raw_ostream &operator<<(raw_ostream &OS, const MultilibSet &MS);
diff --git a/clang/include/clang/Driver/MultilibBuilder.h b/clang/include/clang/Driver/MultilibBuilder.h
index cf84c456152b1..f4875f2e03f8a 100644
--- a/clang/include/clang/Driver/MultilibBuilder.h
+++ b/clang/include/clang/Driver/MultilibBuilder.h
@@ -28,11 +28,10 @@ class MultilibBuilder {
std::string OSSuffix;
std::string IncludeSuffix;
flags_list Flags;
- int Priority;
public:
MultilibBuilder(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, int Priority = 0);
+ StringRef IncludeSuffix);
/// Initializes GCCSuffix, OSSuffix & IncludeSuffix to the same value.
MultilibBuilder(StringRef Suffix = {});
@@ -75,10 +74,6 @@ class MultilibBuilder {
const flags_list &flags() const { return Flags; }
flags_list &flags() { return Flags; }
- /// Returns the multilib priority. When more than one multilib matches flags,
- /// the one with the highest priority is selected, with 0 being the default.
- int priority() const { return Priority; }
-
/// Add a flag to the flags list
/// \p Flag must be a flag accepted by the driver with its leading '-'
/// removed,
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index d1ab0c7b114e9..06bab74898616 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -26,10 +26,9 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, int Priority,
- const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags), Priority(Priority) {
+ Flags(Flags) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -84,56 +83,36 @@ raw_ostream &clang::driver::operator<<(raw_ostream &OS, const Multilib &M) {
}
MultilibSet &MultilibSet::FilterOut(FilterCallback F) {
- filterInPlace(F, Multilibs);
+ llvm::erase_if(Multilibs, F);
return *this;
}
void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); }
-static bool isFlagEnabled(StringRef Flag) {
- char Indicator = Flag.front();
- assert(Indicator == '+' || Indicator == '-');
- return Indicator == '+';
+MultilibSet::multilib_list
+MultilibSet::select(const Multilib::flags_list &Flags) const {
+ llvm::StringSet<> FlagSet;
+ for (const auto &Flag : Flags)
+ FlagSet.insert(Flag);
+
+ multilib_list Result;
+ llvm::copy_if(Multilibs, std::back_inserter(Result),
+ [&FlagSet](const Multilib &M) {
+ for (const std::string &F : M.flags())
+ if (!FlagSet.contains(F))
+ return false;
+ return true;
+ });
+ return Result;
}
-bool MultilibSet::select(const Multilib::flags_list &Flags, Multilib &M) const {
- llvm::StringMap<bool> FlagSet;
-
- // Stuff all of the flags into the FlagSet such that a true mappend indicates
- // the flag was enabled, and a false mappend indicates the flag was disabled.
- for (StringRef Flag : Flags)
- FlagSet[Flag.substr(1)] = isFlagEnabled(Flag);
-
- multilib_list Filtered = filterCopy([&FlagSet](const Multilib &M) {
- for (StringRef Flag : M.flags()) {
- llvm::StringMap<bool>::const_iterator SI = FlagSet.find(Flag.substr(1));
- if (SI != FlagSet.end())
- if (SI->getValue() != isFlagEnabled(Flag))
- return true;
- }
- return false;
- }, Multilibs);
-
- if (Filtered.empty())
+bool MultilibSet::select(const Multilib::flags_list &Flags,
+ Multilib &Selected) const {
+ multilib_list Result = select(Flags);
+ if (Result.empty())
return false;
- if (Filtered.size() == 1) {
- M = Filtered[0];
- return true;
- }
-
- // Sort multilibs by priority and select the one with the highest priority.
- llvm::sort(Filtered, [](const Multilib &a, const Multilib &b) -> bool {
- return a.priority() > b.priority();
- });
-
- if (Filtered[0].priority() > Filtered[1].priority()) {
- M = Filtered[0];
- return true;
- }
-
- // TODO: We should consider returning llvm::Error rather than aborting.
- assert(false && "More than one multilib with the same priority");
- return false;
+ Selected = Result.back();
+ return true;
}
LLVM_DUMP_METHOD void MultilibSet::dump() const {
@@ -145,17 +124,6 @@ void MultilibSet::print(raw_ostream &OS) const {
OS << M << "\n";
}
-MultilibSet::multilib_list MultilibSet::filterCopy(FilterCallback F,
- const multilib_list &Ms) {
- multilib_list Copy(Ms);
- filterInPlace(F, Copy);
- return Copy;
-}
-
-void MultilibSet::filterInPlace(FilterCallback F, multilib_list &Ms) {
- llvm::erase_if(Ms, F);
-}
-
raw_ostream &clang::driver::operator<<(raw_ostream &OS, const MultilibSet &MS) {
MS.print(OS);
return OS;
diff --git a/clang/lib/Driver/MultilibBuilder.cpp b/clang/lib/Driver/MultilibBuilder.cpp
index 83ebc31d8eb99..f6351ae4b5278 100644
--- a/clang/lib/Driver/MultilibBuilder.cpp
+++ b/clang/lib/Driver/MultilibBuilder.cpp
@@ -41,9 +41,8 @@ static void normalizePathSegment(std::string &Segment) {
}
}
-MultilibBuilder::MultilibBuilder(StringRef GCC, StringRef OS, StringRef Include,
- int Priority)
- : GCCSuffix(GCC), OSSuffix(OS), IncludeSuffix(Include), Priority(Priority) {
+MultilibBuilder::MultilibBuilder(StringRef GCC, StringRef OS, StringRef Include)
+ : GCCSuffix(GCC), OSSuffix(OS), IncludeSuffix(Include) {
normalizePathSegment(GCCSuffix);
normalizePathSegment(OSSuffix);
normalizePathSegment(IncludeSuffix);
@@ -87,7 +86,7 @@ bool MultilibBuilder::isValid() const {
}
Multilib MultilibBuilder::makeMultilib() const {
- return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Priority, Flags);
+ return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Flags);
}
MultilibSetBuilder &MultilibSetBuilder::Maybe(const MultilibBuilder &M) {
diff --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 3a3f7043a795f..b8bb000391b91 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -263,33 +263,33 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
Multilibs.push_back(Multilib());
// Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
- Multilibs.push_back(MultilibBuilder("noexcept", {}, {}, 1)
+ Multilibs.push_back(MultilibBuilder("noexcept", {}, {})
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
// ASan has higher priority because we always want the instrumentated version.
- Multilibs.push_back(MultilibBuilder("asan", {}, {}, 2)
+ Multilibs.push_back(MultilibBuilder("asan", {}, {})
.flag("+fsanitize=address")
.makeMultilib());
// Use the asan+noexcept variant with ASan and -fno-exceptions.
- Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {}, 3)
+ Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {})
.flag("+fsanitize=address")
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
// HWASan has higher priority because we always want the instrumentated
// version.
- Multilibs.push_back(MultilibBuilder("hwasan", {}, {}, 4)
+ Multilibs.push_back(MultilibBuilder("hwasan", {}, {})
.flag("+fsanitize=hwaddress")
.makeMultilib());
// Use the hwasan+noexcept variant with HWASan and -fno-exceptions.
- Multilibs.push_back(MultilibBuilder("hwasan+noexcept", {}, {}, 5)
+ Multilibs.push_back(MultilibBuilder("hwasan+noexcept", {}, {})
.flag("+fsanitize=hwaddress")
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
// Use Itanium C++ ABI for the compat multilib.
- Multilibs.push_back(MultilibBuilder("compat", {}, {}, 6)
+ Multilibs.push_back(MultilibBuilder("compat", {}, {})
.flag("+fc++-abi=itanium")
.makeMultilib());
@@ -299,9 +299,10 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
});
Multilib::flags_list Flags;
- addMultilibFlag(
- Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, true),
- "fexceptions", Flags);
+ bool Exceptions =
+ Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, true);
+ addMultilibFlag(Exceptions, "fexceptions", Flags);
+ addMultilibFlag(!Exceptions, "fno-exceptions", Flags);
addMultilibFlag(getSanitizerArgs(Args).needsAsanRt(), "fsanitize=address",
Flags);
addMultilibFlag(getSanitizerArgs(Args).needsHwasanRt(), "fsanitize=hwaddress",
diff --git a/clang/lib/Driver/ToolChains/OHOS.cpp b/clang/lib/Driver/ToolChains/OHOS.cpp
index 71a4ccd042ac8..bd0409d282084 100644
--- a/clang/lib/Driver/ToolChains/OHOS.cpp
+++ b/clang/lib/Driver/ToolChains/OHOS.cpp
@@ -39,14 +39,16 @@ static bool findOHOSMuslMultilibs(const Multilib::flags_list &Flags,
// -mcpu=cortex-a7
// -mfloat-abi=soft -mfloat-abi=softfp -mfloat-abi=hard
// -mfpu=neon-vfpv4
- Multilibs.push_back(Multilib("/a7_soft", {}, {}, 1,
- {"+mcpu=cortex-a7", "+mfloat-abi=soft"}));
+ Multilibs.push_back(
+ Multilib("/a7_soft", {}, {}, {"+mcpu=cortex-a7", "+mfloat-abi=soft"}));
- Multilibs.push_back(Multilib("/a7_softfp_neon-vfpv4", {}, {}, 1,
- {"+mcpu=cortex-a7", "+mfloat-abi=softfp", "+mfpu=neon-vfpv4"}));
+ Multilibs.push_back(
+ Multilib("/a7_softfp_neon-vfpv4", {}, {},
+ {"+mcpu=cortex-a7", "+mfloat-abi=softfp", "+mfpu=neon-vfpv4"}));
- Multilibs.push_back(Multilib("/a7_hard_neon-vfpv4", {}, {}, 1,
- {"+mcpu=cortex-a7", "+mfloat-abi=hard", "+mfpu=neon-vfpv4"}));
+ Multilibs.push_back(
+ Multilib("/a7_hard_neon-vfpv4", {}, {},
+ {"+mcpu=cortex-a7", "+mfloat-abi=hard", "+mfpu=neon-vfpv4"}));
if (Multilibs.select(Flags, Result.SelectedMultilib)) {
Result.Multilibs = Multilibs;
diff --git a/clang/unittests/Driver/MultilibTest.cpp b/clang/unittests/Driver/MultilibTest.cpp
index 2e729a5051734..6a066f6b0f5a6 100644
--- a/clang/unittests/Driver/MultilibTest.cpp
+++ b/clang/unittests/Driver/MultilibTest.cpp
@@ -33,14 +33,14 @@ TEST(MultilibTest, OpEqReflexivity2) {
}
TEST(MultilibTest, OpEqReflexivity3) {
- Multilib M1({}, {}, {}, 0, {"+foo"});
- Multilib M2({}, {}, {}, 0, {"+foo"});
+ Multilib M1({}, {}, {}, {"+foo"});
+ Multilib M2({}, {}, {}, {"+foo"});
ASSERT_TRUE(M1 == M2) << "Multilibs with the same flag should be the same";
}
TEST(MultilibTest, OpEqInequivalence1) {
- Multilib M1({}, {}, {}, 0, {"+foo"});
- Multilib M2({}, {}, {}, 0, {"-foo"});
+ Multilib M1({}, {}, {}, {"+foo"});
+ Multilib M2({}, {}, {}, {"-foo"});
ASSERT_FALSE(M1 == M2) << "Multilibs with conflicting flags are not the same";
ASSERT_FALSE(M2 == M1)
<< "Multilibs with conflicting flags are not the same (commuted)";
@@ -48,7 +48,7 @@ TEST(MultilibTest, OpEqInequivalence1) {
TEST(MultilibTest, OpEqInequivalence2) {
Multilib M1;
- Multilib M2({}, {}, {}, 0, {"+foo"});
+ Multilib M2({}, {}, {}, {"+foo"});
ASSERT_FALSE(M1 == M2) << "Flags make Multilibs
diff erent";
}
@@ -124,7 +124,7 @@ TEST(MultilibTest, Construction2) {
}
TEST(MultilibTest, Construction3) {
- Multilib M({}, {}, {}, 0, {"+f1", "+f2", "-f3"});
+ Multilib M({}, {}, {}, {"+f1", "+f2", "-f3"});
for (Multilib::flags_list::const_iterator I = M.flags().begin(),
E = M.flags().end();
I != E; ++I) {
@@ -149,8 +149,8 @@ TEST(MultilibTest, SetPushback) {
TEST(MultilibTest, SetPriority) {
MultilibSet MS({
- Multilib("/foo", {}, {}, 1, {"+foo"}),
- Multilib("/bar", {}, {}, 2, {"+bar"}),
+ Multilib("/foo", {}, {}, {"+foo"}),
+ Multilib("/bar", {}, {}, {"+bar"}),
});
Multilib::flags_list Flags1 = {"+foo", "-bar"};
Multilib Selection1;
@@ -166,3 +166,24 @@ TEST(MultilibTest, SetPriority) {
ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
<< "Selection picked " << Selection2 << " which was not expected";
}
+
+TEST(MultilibTest, SelectMultiple) {
+ MultilibSet MS({
+ Multilib("/a", {}, {}, {"x"}),
+ Multilib("/b", {}, {}, {"y"}),
+ });
+ std::vector<Multilib> Selection;
+
+ Selection = MS.select({"x"});
+ ASSERT_EQ(1u, Selection.size());
+ EXPECT_EQ("/a", Selection[0].gccSuffix());
+
+ Selection = MS.select({"y"});
+ ASSERT_EQ(1u, Selection.size());
+ EXPECT_EQ("/b", Selection[0].gccSuffix());
+
+ Selection = MS.select({"y", "x"});
+ ASSERT_EQ(2u, Selection.size());
+ EXPECT_EQ("/a", Selection[0].gccSuffix());
+ EXPECT_EQ("/b", Selection[1].gccSuffix());
+}
More information about the cfe-commits
mailing list