diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h --- a/clang/include/clang/Driver/Multilib.h +++ b/clang/include/clang/Driver/Multilib.h @@ -16,6 +16,7 @@ #include "llvm/Support/Compiler.h" #include #include +#include #include #include #include @@ -27,19 +28,20 @@ /// by a command line flag class Multilib { public: - using flags_list = std::vector; + using flags_list = std::set; + using arg_list = std::vector; 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 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); diff --git a/clang/include/clang/Driver/MultilibBuilder.h b/clang/include/clang/Driver/MultilibBuilder.h --- a/clang/include/clang/Driver/MultilibBuilder.h +++ b/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, diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp --- a/clang/lib/Driver/Multilib.cpp +++ b/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 +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 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::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 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; diff --git a/clang/lib/Driver/MultilibBuilder.cpp b/clang/lib/Driver/MultilibBuilder.cpp --- a/clang/lib/Driver/MultilibBuilder.cpp +++ b/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) { diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/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, diff --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp b/clang/lib/Driver/ToolChains/Fuchsia.cpp --- a/clang/lib/Driver/ToolChains/Fuchsia.cpp +++ b/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()); } diff --git a/clang/unittests/Driver/MultilibBuilderTest.cpp b/clang/unittests/Driver/MultilibBuilderTest.cpp --- a/clang/unittests/Driver/MultilibBuilderTest.cpp +++ b/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()); +} diff --git a/clang/unittests/Driver/MultilibTest.cpp b/clang/unittests/Driver/MultilibTest.cpp --- a/clang/unittests/Driver/MultilibTest.cpp +++ b/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 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()); +}