[Driver] Change multilib selection algorithm

[michaelplatings]

Summary

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.

[michaelplatings]

Rebase

[michaelplatings]

Set both fexceptions and fno-exceptions flags for Fuchsia multilib. Previously that tweak had slipped into D142878, which is now abandoned.

[peter.smith]

A couple of small stylisitic suggestions.

clang/include/clang/Driver/Multilib.h

33–34

would flags_set be a better name. I'm guessing we're using set as we want uniqueness and ordering?

If we don't need ordering then we may be able to use https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringset-h

112

Worth using multilib_list for consistency with the rest of the file?

Is this meant to be an overload for bool select below? I mention it as it has a different return type. Perhaps use selectCompatible?

[michaelplatings]

Rebase

[michaelplatings]

Apply improvements suggested by @peter.smith

[michaelplatings]

clang/include/clang/Driver/Multilib.h

33–34

Renamed to flag_set.

Yes, we want uniqueness and ordering. I don't think ordering matters in this change, but later changes rely on it.

112

Changed to use multilib_list now, thanks.

The overloading goes away in D143059.

[michaelplatings]

Rebase

[peter.smith]

Thanks for the updates, I'm happy with the changes and don't have any more comments. Happy to give my implicit approval.

[MaskRay]

Use [Driver] for pure driver patches.

[MaskRay]

clang/lib/Driver/Multilib.cpp

53

remove braces

99

remove braces

[MaskRay]

Mention MultilibSet::select in the summary.

[michaelplatings]

Remove braces as requested by @MaskRay

[phosek]

clang/include/clang/Driver/Multilib.h

42

This is just a suggestion, but GCC documentation refers to these as either "options" or "switches", not "args" so I think it might be helpful to use the same nomenclature. This would also avoid confusion since the term "args" is used extensively throughout the driver but refers to input arguments.

clang/lib/Driver/MultilibBuilder.cpp

87–90

If I understand this correctly, we might end up with duplicate arguments in the case when Flags contains duplicate elements. Is that desirable? Wouldn't it be better to construct the list of arguments from the set of fags inside Multilib?

[michaelplatings]

• Rename args to options as suggested by @phosek
• Add more comments to hopefully make the difference between flags and options clearer.

clang/include/clang/Driver/Multilib.h

42

OK, I've gone with "options" just because switch_list would be an unintuitive name.

clang/lib/Driver/MultilibBuilder.cpp

87–90

Yes, you can have duplicate options if you write code to do that. The Multilib class has never previously had any functionality to remove duplicate options so I'd rather not change that. Previously if you wrote code that added the same option twice then clang -print-multi-lib would reflect that. This behaviour remains unchanged.

No, you can't construct the list of options from the set of flags inside Multilib because Multilib's flags are not command line options. (They will typically look similar to command line options but there's no requirement for that to be the case). I've added comments here and for the flags() method to hopefully make this clearer. Hopefully the docs in D143587 will also make things clearer for anyone else trying to reason about the multilib system.

[peter.smith]

I've set approved from the Arm side. Please leave some time for people in the US time zone to leave any final comments or ask for extensions. I've left a suggestion for a comment, but this can be applied prior to commit if you want to take it up.

clang/include/clang/Driver/Multilib.h

68

Would be good to reference the Multilib Design document's definition and examples of (what will be renamed in D145567 to tags).

This may require moving the design doc down the stack, probably not necessary if everything is committed at once.

[michaelplatings]

clang/include/clang/Driver/Multilib.h

68

I've changed D143587 to update the comment to reference the docs.

[phosek]

I have just one comment related to efficiency, but if it turns out to be too difficult to address, I'd be also fine landing this change as is and addressing that issue in a follow up change.

clang/lib/Driver/MultilibBuilder.cpp

91–97

I think we're optimizing for the wrong case here since PrintOptions is only needed for -print-multi-lib, which is infrequently used flag, but we always have pay the cost of constructing and keeping PrintOptions in memory, even when unused (which is going to vast majority of Driver uses). I think that constructing it lazily in getPrintOptions() (and using that in print()) would be more efficient.

[michaelplatings]

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 it's now about 2 microseconds faster per clang invocation.

[michaelplatings]

fix typo in comment

[michaelplatings]

@phosek ping.

[phosek]

clang/lib/Driver/MultilibBuilder.cpp

85

Would it be possible to move this method directly into Multilib to avoid having to pass around the callback?

[michaelplatings]

Add enum to Multilib class for how print options should be calculated.

[michaelplatings]

Move change to print options into D146757

[phosek]

LGTM

clang/include/clang/Driver/Multilib.h

 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include <cassert>
 #include <functional>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 namespace clang {
 namespace driver {
 
 /// This corresponds to a single GCC Multilib, or a segment of one controlled
 /// by a command line flag.
 /// This class is designed to be immutable but see also MultilibBuilder for
 /// building a multilib incrementally.
 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:
   /// 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,
-           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
   const std::string &gccSuffix() const { return GCCSuffix; }
 
   /// Get the detected os path suffix for the multi-arch
   /// target variant. Always starts with a '/', unless empty
   const std::string &osSuffix() const { return OSSuffix; }
 
   /// Get the include directory suffix. Always starts with a '/', unless
   /// 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
   void print(raw_ostream &OS) const;
 
   /// Check whether the default is selected
   bool isDefault() const
   { return GCCSuffix.empty() && OSSuffix.empty() && IncludeSuffix.empty(); }
 
public:
   MultilibSet &FilterOut(FilterCallback F);
 
   /// Add a completed Multilib to the set
   void push_back(const Multilib &M);
 
   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;
 
   unsigned size() const { return Multilibs.size(); }
 
   LLVM_DUMP_METHOD void dump() const;
   void print(raw_ostream &OS) const;
 
   MultilibSet &setIncludeDirsCallback(IncludeDirsFunc F) {
     IncludeCallback = std::move(F);
     return *this;
   }
 
   const IncludeDirsFunc &includeDirsCallback() const { return IncludeCallback; }
 
   MultilibSet &setFilePathsCallback(IncludeDirsFunc F) {
     FilePathsCallback = std::move(F);
     return *this;
   }
 
   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);
 
 } // namespace driver
 } // namespace clang
 
 #endif // LLVM_CLANG_DRIVER_MULTILIB_H

clang/include/clang/Driver/MultilibBuilder.h

 public:
   using flags_list = std::vector<std::string>;
 
 private:
   std::string GCCSuffix;
   std::string OSSuffix;
   std::string IncludeSuffix;
   flags_list Flags;
-  int Priority;
 
 public:
   MultilibBuilder(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
   const std::string &gccSuffix() const {
     assert(GCCSuffix.empty() ||
            (StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
     return GCCSuffix;
   }
public:
   /// Set the include directory suffix
   MultilibBuilder &includeSuffix(StringRef S);
 
   /// Get the flags that indicate or contraindicate this multilib's use
   /// All elements begin with either '+' or '-'
   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,
   ///     and replaced with either:
   ///       '-' which contraindicates using this multilib with that flag
   ///     or:
   ///       '+' which promotes using this multilib in the presence of that flag
   ///     otherwise '-print-multi-lib' will not emit them correctly.

clang/lib/Driver/Multilib.cpp

 #include <cassert>
 #include <string>
 
 using namespace clang;
 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,
+                   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() ||
          (StringRef(OSSuffix).front() == '/' && OSSuffix.size() > 1));
   assert(IncludeSuffix.empty() ||
          (StringRef(IncludeSuffix).front() == '/' && IncludeSuffix.size() > 1));
 }
 
 LLVM_DUMP_METHOD void Multilib::dump() const {
   print(llvm::errs());
 }
 
 void Multilib::print(raw_ostream &OS) const {
   if (GCCSuffix.empty())
     OS << ".";
   else {
     OS << StringRef(GCCSuffix).drop_front();
   }
   OS << ";";
-  for (StringRef Flag : Flags) {
-    if (Flag.front() == '+')
-      OS << "@" << Flag.substr(1);
+  for (StringRef Flag : PrintArgs) {
   }
 }
 
 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)
 
   if (osSuffix() != Other.osSuffix())
     return false;
 
   if (gccSuffix() != Other.gccSuffix())
     return false;
 
   if (includeSuffix() != Other.includeSuffix())
     return false;
 
   return true;
 }
 
 raw_ostream &clang::driver::operator<<(raw_ostream &OS, const Multilib &M) {
   M.print(OS);
   return OS;
 }
 
 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 == '+';
-}
-
-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);
+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;
 
-  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;
-  }
+  Selected = Result.back();
 
-  // TODO: We should consider returning llvm::Error rather than aborting.
-  assert(false && "More than one multilib with the same priority");
-  return false;
-}
-
 LLVM_DUMP_METHOD void MultilibSet::dump() const {
   print(llvm::errs());
 }
 
 void MultilibSet::print(raw_ostream &OS) const {
   for (const auto &M : *this)
     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;
 }

clang/lib/Driver/MultilibBuilder.cpp

static void normalizePathSegment(std::string &Segment) {
   // Add leading '/'
   if (seg.front() != '/') {
     Segment = "/" + seg.str();
   } else {
     Segment = std::string(seg);
   }
 }
 
-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);
 }
 
 MultilibBuilder &MultilibBuilder::gccSuffix(StringRef S) {
   GCCSuffix = std::string(S);
   normalizePathSegment(GCCSuffix);
for (unsigned I = 0, N = Flags.size(); I != N; ++I) {
     if (SI == FlagSet.end())
       FlagSet[Flag.substr(1)] = I;
     else if (Flags[I] != Flags[SI->getValue()])
       return false;
   }
   return true;
 }
 
 Multilib MultilibBuilder::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);
 }
 
 MultilibSetBuilder &MultilibSetBuilder::Maybe(const MultilibBuilder &M) {
   MultilibBuilder Opposite;
   // Negate any '+' flags
   for (StringRef Flag : M.flags()) {
     if (Flag.front() == '+')
       Opposite.flags().push_back(("-" + Flag.substr(1)).str());
   }
   return Either(M, Opposite);
 }
 
 MultilibSetBuilder &MultilibSetBuilder::Either(const MultilibBuilder &M1,

clang/lib/Driver/ToolChains/CommonArgs.cpp

SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
   StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
   llvm::sys::path::append(StatsFile, BaseName);
   llvm::sys::path::replace_extension(StatsFile, "stats");
   return StatsFile;
 }
 
 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,
                                   ArgStringList &CmdArgs, bool IsLTO,
                                   const StringRef PluginOptPrefix) {
   auto addArg = [&, IsLTO](const Twine &Arg) {
     if (IsLTO) {
       assert(!PluginOptPrefix.empty() && "Cannot have empty PluginOptPrefix!");

clang/lib/Driver/ToolChains/Fuchsia.cpp

for (const std::string &Path : getStdlibPaths()) {
       llvm::sys::path::append(P, M.gccSuffix());
       FP.push_back(std::string(P.str()));
     }
     return FP;
   };
 
   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());
 
   Multilibs.FilterOut([&](const Multilib &M) {
     std::vector<std::string> RD = FilePaths(M);
     return llvm::all_of(RD, [&](std::string P) { return !getVFS().exists(P); });
   });
 
   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",
                   Flags);
 
   addMultilibFlag(Args.getLastArgValue(options::OPT_fcxx_abi_EQ) == "itanium",
                   "fc++-abi=itanium", Flags);
 

clang/unittests/Driver/MultilibBuilderTest.cpp

MultilibSet MS2 = MultilibSetBuilder()
                         .Maybe(MultilibBuilder("sf").flag("+SF"))
                         .makeMultilibSet();
 
   for (unsigned I = 0; I < 4; ++I) {
     bool IsEL = I & 0x1;
     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))
         << "Selection failed for " << (IsEL ? "+EL" : "-EL") << " "
         << (IsSF ? "+SF" : "-SF");
 
     std::string Suffix;
     if (IsEL)
       Suffix += "/el";
     if (IsSF)
       Suffix += "/sf";
 
     ASSERT_EQ(Selection.gccSuffix(), Suffix)
         << "Selection picked " << Selection << " which was not expected ";
   }
 }
+
+TEST(MultilibBuilderTest, PrintArgs) {
+  Multilib M = MultilibBuilder()
+                   .flag("+x")
+                   .flag("-y")
+                   .flag("+a")
+                   .flag("-b")
+                   .flag("+c")
+                   .makeMultilib();
+  ASSERT_EQ(Multilib::arg_list({"-x", "-a", "-c"}), M.getPrintArgs());
+}

clang/unittests/Driver/MultilibTest.cpp

 }
 
 TEST(MultilibTest, OpEqReflexivity2) {
   ASSERT_TRUE(Multilib() == Multilib())
       << "Separately constructed default multilibs are not equal";
 }
 
 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)";
 }
 
 TEST(MultilibTest, OpEqInequivalence2) {
   Multilib M1;
-  Multilib M2({}, {}, {}, 0, {"+foo"});
+  Multilib M2({}, {}, {}, {"+foo"});
   ASSERT_FALSE(M1 == M2) << "Flags make Multilibs different";
 }
 
 TEST(MultilibTest, OpEqEquivalence2) {
   Multilib M1("/64");
   Multilib M2("/64");
   ASSERT_TRUE(M1 == M2)
       << "Constructor argument must match Multilib::gccSuffix()";
ASSERT_TRUE(M1 == M2)
       << "Default arguments to Multilib constructor broken (first argument)";
   ASSERT_TRUE(M1 == M3)
       << "Default arguments to Multilib constructor broken (second argument)";
   ASSERT_TRUE(M1 == M4)
       << "Default arguments to Multilib constructor broken (third argument)";
 }
 
 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) {
     ASSERT_TRUE(llvm::StringSwitch<bool>(*I)
                     .Cases("+f1", "+f2", "-f3", true)
                     .Default(false));
   }
 }
 
 TEST(MultilibTest, SetPushback) {
   MultilibSet MS({
       Multilib("/one"),
       Multilib("/two"),
   });
   ASSERT_TRUE(MS.size() == 2);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
     ASSERT_TRUE(llvm::StringSwitch<bool>(I->gccSuffix())
                     .Cases("/one", "/two", true)
                     .Default(false));
   }
 }
 
 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;
   ASSERT_TRUE(MS.select(Flags1, Selection1))
       << "Flag set was {\"+foo\"}, but selection not found";
   ASSERT_TRUE(Selection1.gccSuffix() == "/foo")
       << "Selection picked " << Selection1 << " which was not expected";
 
   Multilib::flags_list Flags2 = {"+foo", "+bar"};
   Multilib Selection2;
   ASSERT_TRUE(MS.select(Flags2, Selection2))
       << "Flag set was {\"+bar\"}, but selection not found";
   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());
+}