michaelplatings created this revision.
michaelplatings added a reviewer: phosek.
Herald added subscribers: abrachet, mgrang.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
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, 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.
Repository:
rG LLVM Github Monorepo
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/CommonArgs.cpp
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/unittests/Driver/MultilibBuilderTest.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/unittests/Driver/MultilibBuilderTest.cpp
===================================================================
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -7,7 +7,7 @@
//
//===----------------------------------------------------------------------===//
//
-// Unit tests for Multilib and MultilibBuilder
+// Unit tests for MultilibBuilderVariant and MultilibBuilder
//
//===----------------------------------------------------------------------===//
@@ -185,14 +185,14 @@
bool IsSF = I & 0x2;
Multilib::flags_list Flags;
if (IsEL)
- Flags.push_back("+EL");
+ Flags.insert("+EL");
else
- Flags.push_back("-EL");
+ Flags.insert("-EL");
if (IsSF)
- Flags.push_back("+SF");
+ Flags.insert("+SF");
else
- Flags.push_back("-SF");
+ Flags.insert("-SF");
Multilib Selection;
ASSERT_TRUE(MS2.select(Flags, Selection))
@@ -209,3 +209,11 @@
<< "Selection picked " << Selection << " which was not expected ";
}
}
+
+TEST(MultilibBuilderTest, PrintArgs) {
+ MultilibBuilderVariant MBV =
+ MultilibBuilderVariant().flag("+x").flag("-y").flag("+a").flag("-b").flag(
+ "+c");
+ auto M = MBV.makeMultilib();
+ ASSERT_EQ(Multilib::arg_list({"-x", "-a", "-c"}), M.getPrintArgs());
+}
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -247,27 +247,27 @@
void Fuchsia::configureMultilibs(MultilibSet &Multilibs) {
Multilibs.push_back(Multilib());
// Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
- Multilibs.push_back(MultilibBuilderVariant("noexcept", {}, {}, 1)
+ Multilibs.push_back(MultilibBuilderVariant("noexcept", {}, {})
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
// ASan has higher priority because we always want the instrumentated version.
- Multilibs.push_back(MultilibBuilderVariant("asan", {}, {}, 2)
+ Multilibs.push_back(MultilibBuilderVariant("asan", {}, {})
.flag("+fsanitize=address")
.makeMultilib());
// Use the asan+noexcept variant with ASan and -fno-exceptions.
- Multilibs.push_back(MultilibBuilderVariant("asan+noexcept", {}, {}, 3)
+ Multilibs.push_back(MultilibBuilderVariant("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(MultilibBuilderVariant("hwasan", {}, {}, 4)
+ Multilibs.push_back(MultilibBuilderVariant("hwasan", {}, {})
.flag("+fsanitize=hwaddress")
.makeMultilib());
// Use the hwasan+noexcept variant with HWASan and -fno-exceptions.
- Multilibs.push_back(MultilibBuilderVariant("hwasan+noexcept", {}, {}, 5)
+ Multilibs.push_back(MultilibBuilderVariant("hwasan+noexcept", {}, {})
.flag("+fsanitize=hwaddress")
.flag("-fexceptions")
.flag("+fno-exceptions")
@@ -275,40 +275,39 @@
// Use the relative vtables ABI.
// TODO: Remove these multilibs once relative vtables are enabled by default
// for Fuchsia.
- Multilibs.push_back(MultilibBuilderVariant("relative-vtables", {}, {}, 6)
+ Multilibs.push_back(MultilibBuilderVariant("relative-vtables", {}, {})
.flag("+fexperimental-relative-c++-abi-vtables")
.makeMultilib());
Multilibs.push_back(
- MultilibBuilderVariant("relative-vtables+noexcept", {}, {}, 7)
+ MultilibBuilderVariant("relative-vtables+noexcept", {}, {})
.flag("+fexperimental-relative-c++-abi-vtables")
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
- Multilibs.push_back(MultilibBuilderVariant("relative-vtables+asan", {}, {}, 8)
+ Multilibs.push_back(MultilibBuilderVariant("relative-vtables+asan", {}, {})
.flag("+fexperimental-relative-c++-abi-vtables")
.flag("+fsanitize=address")
.makeMultilib());
Multilibs.push_back(
- MultilibBuilderVariant("relative-vtables+asan+noexcept", {}, {}, 9)
+ MultilibBuilderVariant("relative-vtables+asan+noexcept", {}, {})
.flag("+fexperimental-relative-c++-abi-vtables")
.flag("+fsanitize=address")
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
+ Multilibs.push_back(MultilibBuilderVariant("relative-vtables+hwasan", {}, {})
+ .flag("+fexperimental-relative-c++-abi-vtables")
+ .flag("+fsanitize=hwaddress")
+ .makeMultilib());
Multilibs.push_back(
- MultilibBuilderVariant("relative-vtables+hwasan", {}, {}, 10)
- .flag("+fexperimental-relative-c++-abi-vtables")
- .flag("+fsanitize=hwaddress")
- .makeMultilib());
- Multilibs.push_back(
- MultilibBuilderVariant("relative-vtables+hwasan+noexcept", {}, {}, 11)
+ MultilibBuilderVariant("relative-vtables+hwasan+noexcept", {}, {})
.flag("+fexperimental-relative-c++-abi-vtables")
.flag("+fsanitize=hwaddress")
.flag("-fexceptions")
.flag("+fno-exceptions")
.makeMultilib());
// Use Itanium C++ ABI for the compat multilib.
- Multilibs.push_back(MultilibBuilderVariant("compat", {}, {}, 12)
+ Multilibs.push_back(MultilibBuilderVariant("compat", {}, {})
.flag("+fc++-abi=itanium")
.makeMultilib());
}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1806,7 +1806,7 @@
void tools::addMultilibFlag(bool Enabled, const char *const Flag,
Multilib::flags_list &Flags) {
- Flags.push_back(std::string(Enabled ? "+" : "-") + Flag);
+ Flags.insert(std::string(Enabled ? "+" : "-") + Flag);
}
void tools::addX86AlignBranchArgs(const Driver &D, const ArgList &Args,
Index: clang/lib/Driver/MultilibBuilder.cpp
===================================================================
--- clang/lib/Driver/MultilibBuilder.cpp
+++ clang/lib/Driver/MultilibBuilder.cpp
@@ -42,8 +42,8 @@
}
MultilibBuilderVariant::MultilibBuilderVariant(StringRef GCC, StringRef OS,
- StringRef Include, int Priority)
- : GCCSuffix(GCC), OSSuffix(OS), IncludeSuffix(Include), Priority(Priority) {
+ StringRef Include)
+ : GCCSuffix(GCC), OSSuffix(OS), IncludeSuffix(Include) {
normalizePathSegment(GCCSuffix);
normalizePathSegment(OSSuffix);
normalizePathSegment(IncludeSuffix);
@@ -84,7 +84,13 @@
}
Multilib MultilibBuilderVariant::makeMultilib() const {
- return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Priority, Flags);
+ Multilib::arg_list Args;
+ for (StringRef Flag : Flags) {
+ if (Flag.front() == '+')
+ Args.push_back(("-" + Flag.substr(1)).str());
+ }
+ return Multilib(GCCSuffix, OSSuffix, IncludeSuffix,
+ Multilib::flags_list(Flags.begin(), Flags.end()), Args);
}
MultilibBuilder &MultilibBuilder::Maybe(const MultilibBuilderVariant &M) {
Index: clang/lib/Driver/Multilib.cpp
===================================================================
--- clang/lib/Driver/Multilib.cpp
+++ clang/lib/Driver/Multilib.cpp
@@ -26,10 +26,10 @@
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, int Priority,
- const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ const arg_list &PrintArgs)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags), Priority(Priority) {
+ Flags(Flags), PrintArgs(PrintArgs) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -49,22 +49,14 @@
OS << StringRef(GCCSuffix).drop_front();
}
OS << ";";
- for (StringRef Flag : Flags) {
- if (Flag.front() == '+')
- OS << "@" << Flag.substr(1);
+ for (StringRef Flag : PrintArgs) {
+ OS << "@" << Flag.substr(1);
}
}
bool Multilib::operator==(const Multilib &Other) const {
- // Check whether the flags sets match
- // allowing for the match to be order invariant
- llvm::StringSet<> MyFlags;
- for (const auto &Flag : Flags)
- MyFlags.insert(Flag);
-
- for (const auto &Flag : Other.Flags)
- if (MyFlags.find(Flag) == MyFlags.end())
- return false;
+ if (Flags != Other.Flags)
+ return false;
if (osSuffix() != Other.osSuffix())
return false;
@@ -84,56 +76,31 @@
}
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 == '+';
+std::vector<Multilib>
+MultilibSet::select(const Multilib::flags_list &Flags) const {
+ multilib_list Result;
+ llvm::copy_if(Multilibs, std::back_inserter(Result),
+ [&Flags](const Multilib &M) {
+ return std::includes(Flags.begin(), Flags.end(),
+ M.flags().begin(), M.flags().end());
+ });
+ 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 {
+ std::vector<Multilib> 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 +112,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
@@ -1,4 +1,4 @@
-//===- Multilib.h
+//===- MultilibBuilderVariant.h
//-----------------------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -26,11 +26,10 @@
std::string OSSuffix;
std::string IncludeSuffix;
flags_list Flags;
- int Priority;
public:
MultilibBuilderVariant(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
- StringRef IncludeSuffix = {}, int Priority = 0);
+ StringRef IncludeSuffix = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -70,10 +69,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
@@ -16,6 +16,7 @@
#include "llvm/Support/Compiler.h"
#include <cassert>
#include <functional>
+#include <set>
#include <string>
#include <utility>
#include <vector>
@@ -27,19 +28,20 @@
/// by a command line flag
class Multilib {
public:
- using flags_list = std::vector<std::string>;
+ using flags_list = std::set<std::string>;
+ using arg_list = std::vector<std::string>;
private:
std::string GCCSuffix;
std::string OSSuffix;
std::string IncludeSuffix;
flags_list Flags;
- int Priority;
+ arg_list PrintArgs;
public:
Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
- StringRef IncludeSuffix = {}, int Priority = 0,
- const flags_list &Flags = flags_list());
+ StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(),
+ const arg_list &PrintArgs = arg_list());
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -53,13 +55,11 @@
/// empty
const std::string &includeSuffix() const { return IncludeSuffix; }
- /// Get the flags that indicate or contraindicate this multilib's use
- /// All elements begin with either '+' or '-'
+ /// Get the flags that indicate this multilib's use
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; }
+ /// Returns the arguments that should be used for clang -print-multi-lib
+ const arg_list &getPrintArgs() const { return PrintArgs; }
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
@@ -102,6 +102,9 @@
const_iterator begin() const { return Multilibs.begin(); }
const_iterator end() const { return Multilibs.end(); }
+ /// Select compatible variants
+ std::vector<Multilib> 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;
@@ -123,13 +126,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
