Multilib YAML parsing

[michaelplatings]

Summary

The format includes a ClangMinimumVersion entry to avoid a potential
source of subtle errors if an older version of Clang were to be used
with a multilib.yaml that requires a newer Clang to work correctly.
This feature is comparable to CMake's cmake_minimum_required.

[miyuki]

clang/lib/Driver/Multilib.cpp

93

Please use the LLVM regular expression engine (llvm/Support/Regex.h) instead of std::regex. It's more portable, e.g., I've seen bugs and performance issues with MSVC implementation that don't manifest on Linux.

[miyuki]

clang/lib/Driver/Multilib.cpp

94

Please use llvm::find_if(InFlags, ...)

149

I think it would make sense to validate regular expression syntax as well.

[michaelplatings]

Incorporated changes requested by @miyuki

[michaelplatings]

Move the regular expression flag matching functionality into MutlilibSet. This (a) simplifies the code for loading from multilib.yaml; and (b) makes the flag matching functionality easier to access.

[michaelplatings]

Rebase

[phosek]

Do you know if regular expressions are necessary to cover the existing use cases? In our experience, while regular expressions are powerful, they also tend to be error prone and more difficult to reason about. Would glob patterns that are implemented by https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/GlobPattern.h be sufficient?

[michaelplatings]

In D142932#4111925, @phosek wrote:

Do you know if regular expressions are necessary to cover the existing use cases?

Yes. One use case is mapping later architecture versions to the last architecture version supported by the multilib. For example, this would map armv8 versions after 8.5 to 8.5:

- regex: target=armv8\.([6-9]|[1-9][0-9]+)-none-eabi
  matchFlags: [target=armv8.5-none-eabi]

[michaelplatings]

Make "variants" required

[MaskRay]

Adding new APIs is usually not considered NFC. Just remove the tag.

clang/lib/Driver/Multilib.cpp

127

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

130

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

146

anonymous namespace

151

anonymous namespace

[phosek]

For the existing YAML files within LLVM, we most commonly used the PascalCase for field names and less commonly lisp-case. This change uses camelCase which is unusual and unless there's a particular reason for using this scheme such as compatibility with an existing format, I'd prefer keeping the format consistent with the rest of LLVM and using PascalCase or lisp-case.

[michaelplatings]

• Squash D143763 into this change as suggested by @MaskRay
• Use PascalCase field names as requested by @phosek
• Applied improvements to error messages suggested by @peter.smith
• Removed some braces as requested by @MaskRay
• Moved serialization structs to anonymous namespace as requested by @MaskRay

[michaelplatings]

PrintArgs -> PrintOptions

[michaelplatings]

flags -> tags

[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 some comments that can be applied prior to commit if you want to take them up.

clang/include/clang/Driver/Multilib.h

63

With the context of https://discourse.llvm.org/t/rfc-multilib/67494 and potential other formats. I think it will be worth making this comment not specific to multilib.yaml. For example if there is another non-YAML DSL that has tags.

For example multilib DSL rather than multilib.yaml

133

With reference to https://discourse.llvm.org/t/rfc-multilib/67494 perhaps rename this to parseMultilibYaml as parse is generic, yet there is no indication it is working on a yaml file here.

[michaelplatings]

parse -> parseYaml

clang/include/clang/Driver/Multilib.h

63

If I were a newcomer to this code I think I'd find it easier to grep for multilib.yaml so on that basis I'd prefer to stick with the current wording. Should another file format be adopted then the comment could be updated at that point.

133

Agreed. Renamed to parseYaml.

[phosek]

clang/lib/Driver/Multilib.cpp

152

Can we use a separate versioning scheme rather than tying the version to Clang? That is start at version 1.0 and bump it every time we make backwards incompatible change. That's the strategy we've for YAML formats used in other tools such as llvm-ifs.

The reason I think that would be preferable is that Clang will be at version N.0.0 for ~9 months; we bump the version when we branch of the release, then spent 6 months developing and then typically another 3 months stabilizing the release branch. So for example, we bump the version to 16.0.0 in July 2022, and that version will get released in March 2023. However, during those 9 months, we may want to make more than one breaking change to the format.

This may not be an issue for projects that only follow stable releases, but there's plenty of major projects that live at HEAD, such as Android, Chromium or Fuchsia, for which this could cause an issue. Using a dedicated versioning scheme allows us to decouple the format evolution from Clang releases.

152

Use llvm::VersionTuple rather than std::string which is intended for this purpose.

[michaelplatings]

tags->flags

[michaelplatings]

Decouple multilib versioning scheme from the Clang version

[michaelplatings]

Rebase

[michaelplatings]

Rebase

[phosek]

clang/include/clang/Driver/Multilib.h

133–135

Rather than parseYaml modifying the state of MultilibSet, can we instead have a static factory method or a standalone function that parses, constructs and returns llvm::Expected<MultilibSet>. That's more in line with how we handle YAML deserialization elsewhere in LLVM, and also follows the notion of MultilibSet being immutable.

clang/lib/Driver/Multilib.cpp

171

This is just a nit, but the error messages in this file are inconsistent. Sometimes they are full sentences (capitalized, ending with a dot), sometimes they are not (not-capitalized, no dot at the end) and in this particular case it's a mixture (not-capitalized, ending with a dot, although it seems like dot here is only used as a separator). I don't have a strong preference for one style over the other, but I think we should pick one and be consistent.

[michaelplatings]

Rebase on D151438

[michaelplatings]

clang/include/clang/Driver/Multilib.h

133–135

Done, although I used ErrorOr instead of Expected to allow passing the error directly from the yaml::Input object.

[michaelplatings]

Update unit tests

[phosek]

The only remaining concern I have is about the YAML schema, specifically the naming of Regex, MatchFlags and NoMatchFlags fields. Specifically, I think it something like this might be cleaner:

FlagMap:
- Match: --target=thumbv8(\.[0-9]+)?m\.base-none-unknown-eabi
  Flags: [--target=thumbv6m-none-unknown-eabi]

The fact that the value is a regular expression is implied by the name of the field Match since the term "matching operation" is commonly used with regular expression. For the negative case we could use NoMatch.

I'm open to other ideas though, this is just a suggestion.

[michaelplatings]

@phosek thanks for your suggestion, that's now implemented. In practise for LLVM Embedded Toolchain for Arm we haven't yet needed NoMatchFlags so I've removed that feature.

[phosek]

In D142932#4402981, @michaelplatings wrote:

@phosek thanks for your suggestion, that's now implemented. In practise for LLVM Embedded Toolchain for Arm we haven't yet needed NoMatchFlags so I've removed that feature.

Great, that simplifies things even further. The only other suggestion I have is to maybe rename FlagMap to Mappings for a better consistency with Variants, otherwise LGTM.

[michaelplatings]

FlagMap->Mappings

clang/include/clang/Driver/Multilib.h

 //===- Multilib.h -----------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_DRIVER_MULTILIB_H
 #define LLVM_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/ADT/StringSet.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/SourceMgr.h"
 #include <cassert>
 #include <functional>
 #include <string>
 #include <utility>
 #include <vector>
 
 namespace clang {
 namespace driver {
public:
   /// 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 '!'
   const flags_list &flags() const { return Flags; }
 
   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(); }
 
   bool operator==(const Multilib &Other) const;
 };
 
 raw_ostream &operator<<(raw_ostream &OS, const Multilib &M);
 
 /// See also MultilibSetBuilder for combining multilibs into a set.
 class MultilibSet {
 public:
   using multilib_list = std::vector<Multilib>;
   using const_iterator = multilib_list::const_iterator;
   using IncludeDirsFunc =
       std::function<std::vector<std::string>(const Multilib &M)>;
   using FilterCallback = llvm::function_ref<bool(const Multilib &)>;
 
+  /// Uses regular expressions to simplify flags used for multilib selection.
+  /// For example, we may wish both -mfloat-abi=soft and -mfloat-abi=softfp to
+  /// be treated as -mfloat-abi=soft.
+  struct FlagMatcher {
+    std::string Match;
+    std::vector<std::string> Flags;
+  };
+
 private:
   multilib_list Multilibs;
+  std::vector<FlagMatcher> FlagMatchers;
   IncludeDirsFunc IncludeCallback;
   IncludeDirsFunc FilePathsCallback;
 
 public:
   MultilibSet() = default;
-  MultilibSet(multilib_list &&Multilibs) : Multilibs(Multilibs) {}
+  MultilibSet(multilib_list &&Multilibs,
+              std::vector<FlagMatcher> &&FlagMatchers = {})
+      : Multilibs(Multilibs), FlagMatchers(FlagMatchers) {}
 
   const multilib_list &getMultilibs() { return Multilibs; }
 
   /// Filter out some subset of the Multilibs using a user defined callback
   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
   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;
 
   unsigned size() const { return Multilibs.size(); }
 
+  /// Get the given flags plus flags found by matching them against the
+  /// FlagMatchers and choosing the Flags of each accordingly. The select method
+  /// calls this method so in most cases it's not necessary to call it directly.
+  llvm::StringSet<> expandFlags(const Multilib::flags_list &) const;
+
   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; }
+
+  static llvm::ErrorOr<MultilibSet>
+  parseYaml(llvm::MemoryBufferRef, llvm::SourceMgr::DiagHandlerTy = nullptr,
+            void *DiagHandlerCtxt = nullptr);
 };
 
 raw_ostream &operator<<(raw_ostream &OS, const MultilibSet &MS);
 
 } // namespace driver
 } // namespace clang
 
 #endif // LLVM_CLANG_DRIVER_MULTILIB_H

clang/lib/Driver/Multilib.cpp

 //===- Multilib.cpp - Multilib Implementation -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/Multilib.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/Support/VersionTuple.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <string>
 
 using namespace clang;
 using namespace driver;
 using namespace llvm::sys;
raw_ostream &clang::driver::operator<<(raw_ostream &OS, const Multilib &M) {
   M.print(OS);
   return OS;
 }
 
 MultilibSet &MultilibSet::FilterOut(FilterCallback F) {
   llvm::erase_if(Multilibs, F);
   return *this;
 }
 
 void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); }
 
 MultilibSet::multilib_list
 MultilibSet::select(const Multilib::flags_list &Flags) const {
-  llvm::StringSet<> FlagSet;
-  for (const auto &Flag : Flags)
-    FlagSet.insert(Flag);
-
+  llvm::StringSet<> FlagSet(expandFlags(Flags));
   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 &Selected) const {
   multilib_list Result = select(Flags);
   if (Result.empty())
     return false;
   Selected = Result.back();
   return true;
 }
 
+llvm::StringSet<>
+MultilibSet::expandFlags(const Multilib::flags_list &InFlags) const {
+  llvm::StringSet<> Result;
+  for (const auto &F : InFlags)
+    Result.insert(F);
+  for (const FlagMatcher &M : FlagMatchers) {
+    std::string RegexString(M.Match);
+
+    // Make the regular expression match the whole string.
+    if (!StringRef(M.Match).starts_with("^"))
+      RegexString.insert(RegexString.begin(), '^');
+    if (!StringRef(M.Match).ends_with("$"))
+      RegexString.push_back('$');
+
+    const llvm::Regex Regex(RegexString);
+    assert(Regex.isValid());
+    if (llvm::find_if(InFlags, [&Regex](StringRef F) {
+          return Regex.match(F);
+        }) != InFlags.end()) {
+      Result.insert(M.Flags.begin(), M.Flags.end());
+    }
+  }
+  return Result;
+}
+
+namespace {
+
+// When updating this also update MULTILIB_VERSION in MultilibTest.cpp
+static const VersionTuple MultilibVersionCurrent(1, 0);
+
+struct MultilibSerialization {
+  std::string Dir;
+  std::vector<std::string> Flags;
+};
+
+struct MultilibSetSerialization {
+  llvm::VersionTuple MultilibVersion;
+  std::vector<MultilibSerialization> Multilibs;
+  std::vector<MultilibSet::FlagMatcher> FlagMatchers;
+};
+
+} // end anonymous namespace
+
+template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
+  static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
+    io.mapRequired("Dir", V.Dir);
+    io.mapRequired("Flags", V.Flags);
+  }
+  static std::string validate(IO &io, MultilibSerialization &V) {
+    if (StringRef(V.Dir).starts_with("/"))
+      return "paths must be relative but \"" + V.Dir + "\" starts with \"/\"";
+    return std::string{};
+  }
+};
+
+template <> struct llvm::yaml::MappingTraits<MultilibSet::FlagMatcher> {
+  static void mapping(llvm::yaml::IO &io, MultilibSet::FlagMatcher &M) {
+    io.mapRequired("Match", M.Match);
+    io.mapRequired("Flags", M.Flags);
+  }
+  static std::string validate(IO &io, MultilibSet::FlagMatcher &M) {
+    llvm::Regex Regex(M.Match);
+    std::string RegexError;
+    if (!Regex.isValid(RegexError))
+      return RegexError;
+    if (M.Flags.empty())
+      return "value required for 'Flags'";
+    return std::string{};
+  }
+};
+
+template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
+  static void mapping(llvm::yaml::IO &io, MultilibSetSerialization &M) {
+    io.mapRequired("MultilibVersion", M.MultilibVersion);
+    io.mapRequired("Variants", M.Multilibs);
+    io.mapOptional("Mappings", M.FlagMatchers);
+  }
+  static std::string validate(IO &io, MultilibSetSerialization &M) {
+    if (M.MultilibVersion.empty())
+      return "missing required key 'MultilibVersion'";
+    if (M.MultilibVersion.getMajor() != MultilibVersionCurrent.getMajor())
+      return "multilib version " + M.MultilibVersion.getAsString() +
+             " is unsupported";
+    if (M.MultilibVersion.getMinor() > MultilibVersionCurrent.getMinor())
+      return "multilib version " + M.MultilibVersion.getAsString() +
+             " is unsupported";
+    return std::string{};
+  }
+};
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
+LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
+
+llvm::ErrorOr<MultilibSet>
+MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
+                       llvm::SourceMgr::DiagHandlerTy DiagHandler,
+                       void *DiagHandlerCtxt) {
+  MultilibSetSerialization MS;
+  llvm::yaml::Input YamlInput(Input, nullptr, DiagHandler, DiagHandlerCtxt);
+  YamlInput >> MS;
+  if (YamlInput.error())
+    return YamlInput.error();
+
+  multilib_list Multilibs;
+  Multilibs.reserve(MS.Multilibs.size());
+  for (const auto &M : MS.Multilibs) {
+    std::string Dir;
+    if (M.Dir != ".")
+      Dir = "/" + M.Dir;
+    Multilibs.emplace_back(Dir, Dir, Dir, M.Flags);
+  }
+
+  return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));
+}
+
 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";
 }
 
 raw_ostream &clang::driver::operator<<(raw_ostream &OS, const MultilibSet &MS) {
   MS.print(OS);
   return OS;
 }

clang/unittests/Driver/MultilibTest.cpp

 //===- unittests/Driver/MultilibTest.cpp --- Multilib tests ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Unit tests for Multilib and MultilibSet
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/Multilib.h"
 #include "../../lib/Driver/ToolChains/CommonArgs.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
 using namespace clang::driver;
 using namespace clang;
TEST(MultilibTest, SelectMultiple) {
   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());
 }
+
+static void diagnosticCallback(const llvm::SMDiagnostic &D, void *Out) {
+  *reinterpret_cast<std::string *>(Out) = D.getMessage();
+}
+
+static bool parseYaml(MultilibSet &MS, std::string &Diagnostic,
+                      const char *Data) {
+  auto ErrorOrMS = MultilibSet::parseYaml(llvm::MemoryBufferRef(Data, "TEST"),
+                                          diagnosticCallback, &Diagnostic);
+  if (ErrorOrMS.getError())
+    return false;
+  MS = std::move(ErrorOrMS.get());
+  return true;
+}
+
+static bool parseYaml(MultilibSet &MS, const char *Data) {
+  auto ErrorOrMS = MultilibSet::parseYaml(llvm::MemoryBufferRef(Data, "TEST"));
+  if (ErrorOrMS.getError())
+    return false;
+  MS = std::move(ErrorOrMS.get());
+  return true;
+}
+
+// When updating this version also update MultilibVersionCurrent in Multilib.cpp
+#define YAML_PREAMBLE "MultilibVersion: 1.0\n"
+
+TEST(MultilibTest, ParseInvalid) {
+  std::string Diagnostic;
+
+  MultilibSet MS;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, R"(
+Variants: []
+)"));
+  EXPECT_TRUE(
+      StringRef(Diagnostic).contains("missing required key 'MultilibVersion'"))
+      << Diagnostic;
+
+  // Reject files with a different major version
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+                         R"(
+MultilibVersion: 2.0
+Variants: []
+)"));
+  EXPECT_TRUE(
+      StringRef(Diagnostic).contains("multilib version 2.0 is unsupported"))
+      << Diagnostic;
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+                         R"(
+MultilibVersion: 0.1
+Variants: []
+)"));
+  EXPECT_TRUE(
+      StringRef(Diagnostic).contains("multilib version 0.1 is unsupported"))
+      << Diagnostic;
+
+  // Reject files with a later minor version
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+                         R"(
+MultilibVersion: 1.9
+Variants: []
+)"));
+  EXPECT_TRUE(
+      StringRef(Diagnostic).contains("multilib version 1.9 is unsupported"))
+      << Diagnostic;
+
+  // Accept files with the same major version and the same or earlier minor
+  // version
+  EXPECT_TRUE(parseYaml(MS, Diagnostic, R"(
+MultilibVersion: 1.0
+Variants: []
+)")) << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Variants'"))
+      << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: /abc
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("paths must be relative"))
+      << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Dir'"))
+      << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Flags'"))
+      << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants: []
+Mappings:
+- Match: abc
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("value required for 'Flags'"))
+      << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants: []
+Mappings:
+- Dir: .
+  Match: '('
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("parentheses not balanced"))
+      << Diagnostic;
+}
+
+TEST(MultilibTest, Parse) {
+  MultilibSet MS;
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+  Flags: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: abc
+  Flags: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/abc", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: pqr
+  Flags: [-mfloat-abi=soft]
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/pqr", MS.begin()->gccSuffix());
+  EXPECT_EQ(std::vector<std::string>({"-mfloat-abi=soft"}),
+            MS.begin()->flags());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: pqr
+  Flags: [-mfloat-abi=soft, -fno-exceptions]
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ(std::vector<std::string>({"-mfloat-abi=soft", "-fno-exceptions"}),
+            MS.begin()->flags());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: a
+  Flags: []
+- Dir: b
+  Flags: []
+)"));
+  EXPECT_EQ(2U, MS.size());
+}
+
+TEST(MultilibTest, SelectSoft) {
+  MultilibSet MS;
+  Multilib Selected;
+  ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: s
+  Flags: [-mfloat-abi=soft]
+Mappings:
+- Match: -mfloat-abi=softfp
+  Flags: [-mfloat-abi=soft]
+)"));
+  EXPECT_TRUE(MS.select({"-mfloat-abi=soft"}, Selected));
+  EXPECT_TRUE(MS.select({"-mfloat-abi=softfp"}, Selected));
+  EXPECT_FALSE(MS.select({"-mfloat-abi=hard"}, Selected));
+}
+
+TEST(MultilibTest, SelectSoftFP) {
+  MultilibSet MS;
+  Multilib Selected;
+  ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: f
+  Flags: [-mfloat-abi=softfp]
+)"));
+  EXPECT_FALSE(MS.select({"-mfloat-abi=soft"}, Selected));
+  EXPECT_TRUE(MS.select({"-mfloat-abi=softfp"}, Selected));
+  EXPECT_FALSE(MS.select({"-mfloat-abi=hard"}, Selected));
+}
+
+TEST(MultilibTest, SelectHard) {
+  // If hard float is all that's available then select that only if compiling
+  // with hard float.
+  MultilibSet MS;
+  Multilib Selected;
+  ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: h
+  Flags: [-mfloat-abi=hard]
+)"));
+  EXPECT_FALSE(MS.select({"-mfloat-abi=soft"}, Selected));
+  EXPECT_FALSE(MS.select({"-mfloat-abi=softfp"}, Selected));
+  EXPECT_TRUE(MS.select({"-mfloat-abi=hard"}, Selected));
+}
+
+TEST(MultilibTest, SelectFloatABI) {
+  MultilibSet MS;
+  Multilib Selected;
+  ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: s
+  Flags: [-mfloat-abi=soft]
+- Dir: f
+  Flags: [-mfloat-abi=softfp]
+- Dir: h
+  Flags: [-mfloat-abi=hard]
+Mappings:
+- Match: -mfloat-abi=softfp
+  Flags: [-mfloat-abi=soft]
+)"));
+  MS.select({"-mfloat-abi=soft"}, Selected);
+  EXPECT_EQ("/s", Selected.gccSuffix());
+  MS.select({"-mfloat-abi=softfp"}, Selected);
+  EXPECT_EQ("/f", Selected.gccSuffix());
+  MS.select({"-mfloat-abi=hard"}, Selected);
+  EXPECT_EQ("/h", Selected.gccSuffix());
+}
+
+TEST(MultilibTest, SelectFloatABIReversed) {
+  // If soft is specified after softfp then softfp will never be
+  // selected because soft is compatible with softfp and last wins.
+  MultilibSet MS;
+  Multilib Selected;
+  ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: h
+  Flags: [-mfloat-abi=hard]
+- Dir: f
+  Flags: [-mfloat-abi=softfp]
+- Dir: s
+  Flags: [-mfloat-abi=soft]
+Mappings:
+- Match: -mfloat-abi=softfp
+  Flags: [-mfloat-abi=soft]
+)"));
+  MS.select({"-mfloat-abi=soft"}, Selected);
+  EXPECT_EQ("/s", Selected.gccSuffix());
+  MS.select({"-mfloat-abi=softfp"}, Selected);
+  EXPECT_EQ("/s", Selected.gccSuffix());
+  MS.select({"-mfloat-abi=hard"}, Selected);
+  EXPECT_EQ("/h", Selected.gccSuffix());
+}
+
+TEST(MultilibTest, SelectMClass) {
+  const char *MultilibSpec = YAML_PREAMBLE R"(
+Variants:
+- Dir: thumb/v6-m/nofp
+  Flags: [--target=thumbv6m-none-unknown-eabi, -mfpu=none]
+
+- Dir: thumb/v7-m/nofp
+  Flags: [--target=thumbv7m-none-unknown-eabi, -mfpu=none]
+
+- Dir: thumb/v7e-m/nofp
+  Flags: [--target=thumbv7em-none-unknown-eabi, -mfpu=none]
+
+- Dir: thumb/v8-m.main/nofp
+  Flags: [--target=thumbv8m.main-none-unknown-eabi, -mfpu=none]
+
+- Dir: thumb/v8.1-m.main/nofp/nomve
+  Flags: [--target=thumbv8.1m.main-none-unknown-eabi, -mfpu=none]
+
+- Dir: thumb/v7e-m/fpv4_sp_d16
+  Flags: [--target=thumbv7em-none-unknown-eabihf, -mfpu=fpv4-sp-d16]
+
+- Dir: thumb/v7e-m/fpv5_d16
+  Flags: [--target=thumbv7em-none-unknown-eabihf, -mfpu=fpv5-d16]
+
+- Dir: thumb/v8-m.main/fp
+  Flags: [--target=thumbv8m.main-none-unknown-eabihf]
+
+- Dir: thumb/v8.1-m.main/fp
+  Flags: [--target=thumbv8.1m.main-none-unknown-eabihf]
+
+- Dir: thumb/v8.1-m.main/nofp/mve
+  Flags: [--target=thumbv8.1m.main-none-unknown-eabihf, -march=thumbv8.1m.main+mve]
+
+Mappings:
+- Match: --target=thumbv8(\.[0-9]+)?m\.base-none-unknown-eabi
+  Flags: [--target=thumbv6m-none-unknown-eabi]
+- Match: -target=thumbv8\.[1-9]m\.main-none-unknown-eabi
+  Flags: [--target=thumbv8.1m.main-none-unknown-eabi]
+- Match: -target=thumbv8\.[1-9]m\.main-none-unknown-eabihf
+  Flags: [--target=thumbv8.1m.main-none-unknown-eabihf]
+- Match: -march=thumbv8\.[1-9]m\.main.*\+mve($|\+).*
+  Flags: [-march=thumbv8.1m.main+mve]
+)";
+
+  MultilibSet MS;
+  Multilib Selected;
+  ASSERT_TRUE(parseYaml(MS, MultilibSpec));
+
+  ASSERT_TRUE(MS.select({"--target=thumbv6m-none-unknown-eabi", "-mfpu=none"},
+                        Selected));
+  EXPECT_EQ("/thumb/v6-m/nofp", Selected.gccSuffix());
+
+  ASSERT_TRUE(MS.select({"--target=thumbv7m-none-unknown-eabi", "-mfpu=none"},
+                        Selected));
+  EXPECT_EQ("/thumb/v7-m/nofp", Selected.gccSuffix());
+
+  ASSERT_TRUE(MS.select({"--target=thumbv7em-none-unknown-eabi", "-mfpu=none"},
+                        Selected));
+  EXPECT_EQ("/thumb/v7e-m/nofp", Selected.gccSuffix());
+
+  ASSERT_TRUE(MS.select(
+      {"--target=thumbv8m.main-none-unknown-eabi", "-mfpu=none"}, Selected));
+  EXPECT_EQ("/thumb/v8-m.main/nofp", Selected.gccSuffix());
+
+  ASSERT_TRUE(MS.select(
+      {"--target=thumbv8.1m.main-none-unknown-eabi", "-mfpu=none"}, Selected));
+  EXPECT_EQ("/thumb/v8.1-m.main/nofp/nomve", Selected.gccSuffix());
+
+  ASSERT_TRUE(
+      MS.select({"--target=thumbv7em-none-unknown-eabihf", "-mfpu=fpv4-sp-d16"},
+                Selected));
+  EXPECT_EQ("/thumb/v7e-m/fpv4_sp_d16", Selected.gccSuffix());
+
+  ASSERT_TRUE(MS.select(
+      {"--target=thumbv7em-none-unknown-eabihf", "-mfpu=fpv5-d16"}, Selected));
+  EXPECT_EQ("/thumb/v7e-m/fpv5_d16", Selected.gccSuffix());
+
+  ASSERT_TRUE(
+      MS.select({"--target=thumbv8m.main-none-unknown-eabihf"}, Selected));
+  EXPECT_EQ("/thumb/v8-m.main/fp", Selected.gccSuffix());
+
+  ASSERT_TRUE(
+      MS.select({"--target=thumbv8.1m.main-none-unknown-eabihf"}, Selected));
+  EXPECT_EQ("/thumb/v8.1-m.main/fp", Selected.gccSuffix());
+
+  ASSERT_TRUE(MS.select({"--target=thumbv8.1m.main-none-unknown-eabihf",
+                         "-mfpu=none", "-march=thumbv8.1m.main+dsp+mve"},
+                        Selected));
+  EXPECT_EQ("/thumb/v8.1-m.main/nofp/mve", Selected.gccSuffix());
+}