michaelplatings updated this revision to Diff 505279.
michaelplatings marked an inline comment as done.
michaelplatings added a comment.
Calculate the output for -print-multi-lib lazily.
This necessitated returning to using std::vector to store flags to avoid
reordering them.
In theory the big-O time to select a multilib is larger now but in practise the
number of flags is small enough that in practice this is about 2 microseconds
faster per clang invocation.
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/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
@@ -207,3 +207,14 @@
<< "Selection picked " << Selection << " which was not expected ";
}
}
+
+TEST(MultilibBuilderTest, PrintOptions) {
+ Multilib M = MultilibBuilder()
+ .flag("+x")
+ .flag("-y")
+ .flag("+a")
+ .flag("-b")
+ .flag("+c")
+ .makeMultilib();
+ ASSERT_EQ(Multilib::flags_list({"-x", "-a", "-c"}), M.getPrintOptions());
+}
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);
@@ -86,8 +85,23 @@
return true;
}
+static Multilib::flags_list getPrintOptions(const Multilib::flags_list &Flags) {
+ // Derive print options from flags.
+ // In general, flags in the Multilib class are not required to be valid
+ // command line options, but for the MultilibBuilder class flags are expected
+ // to form valid command line options when their first character is replaced
+ // with '-'.
+ Multilib::flags_list PrintOptions;
+ for (StringRef Flag : Flags) {
+ if (Flag.front() == '+')
+ PrintOptions.push_back(("-" + Flag.substr(1)).str());
+ }
+ return PrintOptions;
+}
+
Multilib MultilibBuilder::makeMultilib() const {
- return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Priority, Flags);
+ return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Flags,
+ Multilib::flags_list(), &::getPrintOptions);
}
MultilibSetBuilder &MultilibSetBuilder::Maybe(const MultilibBuilder &M) {
Index: clang/lib/Driver/Multilib.cpp
===================================================================
--- clang/lib/Driver/Multilib.cpp
+++ clang/lib/Driver/Multilib.cpp
@@ -26,16 +26,28 @@
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 flags_list &PrintOptions,
+ print_options_callback PrintOptionsCallback)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags), Priority(Priority) {
+ Flags(Flags), PrintOptions(PrintOptions),
+ PrintOptionsCallback(PrintOptionsCallback) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
(StringRef(OSSuffix).front() == '/' && OSSuffix.size() > 1));
assert(IncludeSuffix.empty() ||
(StringRef(IncludeSuffix).front() == '/' && IncludeSuffix.size() > 1));
+ // PrintOptionsCallback should not be provided at the same time as a non-empty
+ // PrintOptions. They are mutually exlcusive.
+ assert(PrintOptions.empty() || PrintOptionsCallback == nullptr);
+}
+
+Multilib::flags_list Multilib::getPrintOptions() const {
+ if (PrintOptionsCallback) {
+ return (*PrintOptionsCallback)(Flags);
+ }
+ return PrintOptions;
}
LLVM_DUMP_METHOD void Multilib::dump() const {
@@ -45,14 +57,11 @@
void Multilib::print(raw_ostream &OS) const {
if (GCCSuffix.empty())
OS << ".";
- else {
+ else
OS << StringRef(GCCSuffix).drop_front();
- }
OS << ";";
- for (StringRef Flag : Flags) {
- if (Flag.front() == '+')
- OS << "@" << Flag.substr(1);
- }
+ for (StringRef Option : getPrintOptions())
+ OS << "@" << Option.substr(1);
}
bool Multilib::operator==(const Multilib &Other) const {
@@ -84,56 +93,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 +134,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
@@ -30,21 +30,28 @@
class Multilib {
public:
using flags_list = std::vector<std::string>;
+ using print_options_callback = flags_list (*)(const flags_list &);
private:
std::string GCCSuffix;
std::string OSSuffix;
std::string IncludeSuffix;
flags_list Flags;
- int Priority;
+ flags_list PrintOptions;
+ print_options_callback PrintOptionsCallback;
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.
+ /// PrintOptions and PrintOptionsCallback are mutually exclusive.
+ /// If PrintOptionsCallback is non-null then it will be used when needed for
+ /// -print-multi-lib functionality. Otherwise -print-multi-lib will use the
+ /// content from PrintOptions.
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 flags_list &PrintOptions = flags_list(),
+ print_options_callback PrintOptionsCallback = nullptr);
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -58,13 +65,14 @@
/// 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 set of flags that indicate this multilib's use.
+ /// Flags are arbitrary strings although typically they will look similar to
+ /// command line options. A multilib is considered compatible if its flags are
+ /// a subset of the flags derived from the Clang command line options.
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 options that should be used for clang -print-multi-lib
+ flags_list getPrintOptions() const;
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
@@ -108,6 +116,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 +140,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
