[NFC][Driver] Change MultilibBuilder interface

Decouple the interface of the MultilibBuilder flag method from how flags
are stored internally. Likewise change the addMultilibFlag function.

Currently a multilib flag like "-fexceptions" means a multilib is
*incompatible* with the -fexceptions command line option, which is
counter-intuitive. This change is a step towards changing this scheme.

Differential Revision: https://reviews.llvm.org/D151437

clang/include/clang/Driver/MultilibBuilder.h

@@ -75,18 +75,9 @@ class MultilibBuilder {
   flags_list &flags() { return Flags; }
 
   /// 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.
-  MultilibBuilder &flag(StringRef F) {
-    assert(F.front() == '+' || F.front() == '-');
-    Flags.push_back(std::string(F));
-    return *this;
-  }
+  /// \p Required defines whether the flag is required or disallowed.
+  /// \p Flag must be a flag accepted by the driver.
+  MultilibBuilder &flag(bool Required, StringRef Flag);
 
   Multilib makeMultilib() const;
 

clang/lib/Driver/MultilibBuilder.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/MultilibBuilder.h"
+#include "ToolChains/CommonArgs.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Path.h"
@@ -85,6 +86,11 @@ bool MultilibBuilder::isValid() const {
   return true;
 }
 
+MultilibBuilder &MultilibBuilder::flag(bool Required, StringRef Flag) {
+  tools::addMultilibFlag(Required, Flag, Flags);
+  return *this;
+}
+
 Multilib MultilibBuilder::makeMultilib() const {
   return Multilib(GCCSuffix, OSSuffix, IncludeSuffix, Flags);
 }

clang/lib/Driver/ToolChains/BareMetal.cpp

@@ -37,54 +37,56 @@ static bool findRISCVMultilibs(const Driver &D,
   StringRef Abi = tools::riscv::getRISCVABI(Args, TargetTriple);
 
   if (TargetTriple.isRISCV64()) {
-    MultilibBuilder Imac =
-        MultilibBuilder().flag("+march=rv64imac").flag("+mabi=lp64");
+    MultilibBuilder Imac = MultilibBuilder()
+                               .flag(true, "-march=rv64imac")
+                               .flag(true, "-mabi=lp64");
     MultilibBuilder Imafdc = MultilibBuilder("/rv64imafdc/lp64d")
-                                 .flag("+march=rv64imafdc")
-                                 .flag("+mabi=lp64d");
+                                 .flag(true, "-march=rv64imafdc")
+                                 .flag(true, "-mabi=lp64d");
 
     // Multilib reuse
     bool UseImafdc =
         (Arch == "rv64imafdc") || (Arch == "rv64gc"); // gc => imafdc
 
-    addMultilibFlag((Arch == "rv64imac"), "march=rv64imac", Flags);
-    addMultilibFlag(UseImafdc, "march=rv64imafdc", Flags);
-    addMultilibFlag(Abi == "lp64", "mabi=lp64", Flags);
-    addMultilibFlag(Abi == "lp64d", "mabi=lp64d", Flags);
+    addMultilibFlag((Arch == "rv64imac"), "-march=rv64imac", Flags);
+    addMultilibFlag(UseImafdc, "-march=rv64imafdc", Flags);
+    addMultilibFlag(Abi == "lp64", "-mabi=lp64", Flags);
+    addMultilibFlag(Abi == "lp64d", "-mabi=lp64d", Flags);
 
     Result.Multilibs =
         MultilibSetBuilder().Either(Imac, Imafdc).makeMultilibSet();
     return Result.Multilibs.select(Flags, Result.SelectedMultilib);
   }
   if (TargetTriple.isRISCV32()) {
-    MultilibBuilder Imac =
-        MultilibBuilder().flag("+march=rv32imac").flag("+mabi=ilp32");
+    MultilibBuilder Imac = MultilibBuilder()
+                               .flag(true, "-march=rv32imac")
+                               .flag(true, "-mabi=ilp32");
     MultilibBuilder I = MultilibBuilder("/rv32i/ilp32")
-                            .flag("+march=rv32i")
-                            .flag("+mabi=ilp32");
+                            .flag(true, "-march=rv32i")
+                            .flag(true, "-mabi=ilp32");
     MultilibBuilder Im = MultilibBuilder("/rv32im/ilp32")
-                             .flag("+march=rv32im")
-                             .flag("+mabi=ilp32");
+                             .flag(true, "-march=rv32im")
+                             .flag(true, "-mabi=ilp32");
     MultilibBuilder Iac = MultilibBuilder("/rv32iac/ilp32")
-                              .flag("+march=rv32iac")
-                              .flag("+mabi=ilp32");
+                              .flag(true, "-march=rv32iac")
+                              .flag(true, "-mabi=ilp32");
     MultilibBuilder Imafc = MultilibBuilder("/rv32imafc/ilp32f")
-                                .flag("+march=rv32imafc")
-                                .flag("+mabi=ilp32f");
+                                .flag(true, "-march=rv32imafc")
+                                .flag(true, "-mabi=ilp32f");
 
     // Multilib reuse
     bool UseI = (Arch == "rv32i") || (Arch == "rv32ic");    // ic => i
     bool UseIm = (Arch == "rv32im") || (Arch == "rv32imc"); // imc => im
     bool UseImafc = (Arch == "rv32imafc") || (Arch == "rv32imafdc") ||
                     (Arch == "rv32gc"); // imafdc,gc => imafc
 
-    addMultilibFlag(UseI, "march=rv32i", Flags);
-    addMultilibFlag(UseIm, "march=rv32im", Flags);
-    addMultilibFlag((Arch == "rv32iac"), "march=rv32iac", Flags);
-    addMultilibFlag((Arch == "rv32imac"), "march=rv32imac", Flags);
-    addMultilibFlag(UseImafc, "march=rv32imafc", Flags);
-    addMultilibFlag(Abi == "ilp32", "mabi=ilp32", Flags);
-    addMultilibFlag(Abi == "ilp32f", "mabi=ilp32f", Flags);
+    addMultilibFlag(UseI, "-march=rv32i", Flags);
+    addMultilibFlag(UseIm, "-march=rv32im", Flags);
+    addMultilibFlag((Arch == "rv32iac"), "-march=rv32iac", Flags);
+    addMultilibFlag((Arch == "rv32imac"), "-march=rv32imac", Flags);
+    addMultilibFlag(UseImafc, "-march=rv32imafc", Flags);
+    addMultilibFlag(Abi == "ilp32", "-mabi=ilp32", Flags);
+    addMultilibFlag(Abi == "ilp32f", "-mabi=ilp32f", Flags);
 
     Result.Multilibs =
         MultilibSetBuilder().Either(I, Im, Iac, Imac, Imafc).makeMultilibSet();

clang/lib/Driver/ToolChains/CommonArgs.cpp

@@ -1905,9 +1905,14 @@ SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
   return StatsFile;
 }
 
-void tools::addMultilibFlag(bool Enabled, const char *const Flag,
+void tools::addMultilibFlag(bool Enabled, const StringRef Flag,
                             Multilib::flags_list &Flags) {
-  Flags.push_back(std::string(Enabled ? "+" : "-") + Flag);
+  assert(Flag.front() == '-');
+  if (Enabled) {
+    Flags.push_back(("+" + Flag.substr(1)).str());
+  } else {
+    Flags.push_back(Flag.str());
+  }
 }
 
 void tools::addX86AlignBranchArgs(const Driver &D, const ArgList &Args,

clang/lib/Driver/ToolChains/CommonArgs.h

@@ -198,9 +198,8 @@ SmallString<128> getStatsFileName(const llvm::opt::ArgList &Args,
                                   const InputInfo &Output,
                                   const InputInfo &Input, const Driver &D);
 
-/// \p Flag must be a flag accepted by the driver with its leading '-' removed,
-//     otherwise '-print-multi-lib' will not emit them correctly.
-void addMultilibFlag(bool Enabled, const char *const Flag,
+/// \p Flag must be a flag accepted by the driver.
+void addMultilibFlag(bool Enabled, const StringRef Flag,
                      Multilib::flags_list &Flags);
 
 void addX86AlignBranchArgs(const Driver &D, const llvm::opt::ArgList &Args,

clang/lib/Driver/ToolChains/Fuchsia.cpp

@@ -265,33 +265,33 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
   Multilibs.push_back(Multilib());
   // Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
   Multilibs.push_back(MultilibBuilder("noexcept", {}, {})
-                          .flag("-fexceptions")
-                          .flag("+fno-exceptions")
+                          .flag(false, "-fexceptions")
+                          .flag(true, "-fno-exceptions")
                           .makeMultilib());
   // ASan has higher priority because we always want the instrumentated version.
   Multilibs.push_back(MultilibBuilder("asan", {}, {})
-                          .flag("+fsanitize=address")
+                          .flag(true, "-fsanitize=address")
                           .makeMultilib());
   // Use the asan+noexcept variant with ASan and -fno-exceptions.
   Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {})
-                          .flag("+fsanitize=address")
-                          .flag("-fexceptions")
-                          .flag("+fno-exceptions")
+                          .flag(true, "-fsanitize=address")
+                          .flag(false, "-fexceptions")
+                          .flag(true, "-fno-exceptions")
                           .makeMultilib());
   // HWASan has higher priority because we always want the instrumentated
   // version.
   Multilibs.push_back(MultilibBuilder("hwasan", {}, {})
-                          .flag("+fsanitize=hwaddress")
+                          .flag(true, "-fsanitize=hwaddress")
                           .makeMultilib());
   // Use the hwasan+noexcept variant with HWASan and -fno-exceptions.
   Multilibs.push_back(MultilibBuilder("hwasan+noexcept", {}, {})
-                          .flag("+fsanitize=hwaddress")
-                          .flag("-fexceptions")
-                          .flag("+fno-exceptions")
+                          .flag(true, "-fsanitize=hwaddress")
+                          .flag(false, "-fexceptions")
+                          .flag(true, "-fno-exceptions")
                           .makeMultilib());
   // Use Itanium C++ ABI for the compat multilib.
   Multilibs.push_back(MultilibBuilder("compat", {}, {})
-                          .flag("+fc++-abi=itanium")
+                          .flag(true, "-fc++-abi=itanium")
                           .makeMultilib());
 
   Multilibs.FilterOut([&](const Multilib &M) {
@@ -302,15 +302,15 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
   Multilib::flags_list 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",
+  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);
+                  "-fc++-abi=itanium", Flags);
 
   Multilibs.setFilePathsCallback(FilePaths);