This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd30bc9e91241: [Driver] Change multilib selection algorithm
(authored by michaelplatings).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142905/new/
https://reviews.llvm.org/D142905
Files:
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
Index: clang/unittests/Driver/MultilibTest.cpp
===================================================================
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -33,14 +33,14 @@
}
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, OpEqInequivalence2) {
Multilib M1;
- Multilib M2({}, {}, {}, 0, {"+foo"});
+ Multilib M2({}, {}, {}, {"+foo"});
ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
}
@@ -124,7 +124,7 @@
}
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, 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 @@
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());
+}
Index: clang/lib/Driver/ToolChains/OHOS.cpp
===================================================================
--- clang/lib/Driver/ToolChains/OHOS.cpp
+++ clang/lib/Driver/ToolChains/OHOS.cpp
@@ -39,14 +39,16 @@
// -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;
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -263,33 +263,33 @@
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 @@
});
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",
Index: clang/lib/Driver/MultilibBuilder.cpp
===================================================================
--- clang/lib/Driver/MultilibBuilder.cpp
+++ clang/lib/Driver/MultilibBuilder.cpp
@@ -41,9 +41,8 @@
}
}
-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 @@
}
Multilib MultilibBuilder::makeMultilib() const {
- return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Priority, Flags);
+ return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Flags);
}
MultilibSetBuilder &MultilibSetBuilder::Maybe(const MultilibBuilder &M) {
Index: clang/lib/Driver/Multilib.cpp
===================================================================
--- clang/lib/Driver/Multilib.cpp
+++ clang/lib/Driver/Multilib.cpp
@@ -26,10 +26,9 @@
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 @@
}
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 @@
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;
Index: clang/include/clang/Driver/MultilibBuilder.h
===================================================================
--- clang/include/clang/Driver/MultilibBuilder.h
+++ clang/include/clang/Driver/MultilibBuilder.h
@@ -28,11 +28,10 @@
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 @@
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,
Index: clang/include/clang/Driver/Multilib.h
===================================================================
--- clang/include/clang/Driver/Multilib.h
+++ clang/include/clang/Driver/Multilib.h
@@ -36,14 +36,13 @@
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 @@
/// 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 @@
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 @@
}
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
