[PATCH] Refactor GCCInsatllationDetector to better support multiarch

Jonathan Roelofs jonathan at codesourcery.com
Sat Dec 14 14:07:10 PST 2013


Hi,

Here's a patch I'd like to get reviewed.  As the title suggests, I'm 
refactoring in preparation for extending Clang's bi-arch support to 
handle more variations of GCC multilibs.

Thanks,
Jon

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded
-------------- next part --------------
Index: lib/Driver/ToolChains.cpp
===================================================================
--- lib/Driver/ToolChains.cpp	(revision 195793)
+++ lib/Driver/ToolChains.cpp	(working copy)
@@ -1008,6 +1008,15 @@
   return GCC_INSTALL_PREFIX;
 }
 
+Generic_GCC::GCCMultiarchLib::GCCMultiarchLib()
+  : GCCSuffix(""), GCCMIPSABIDirSuffix("") {
+}
+
+Generic_GCC::GCCMultiarchLib::GCCMultiarchLib(std::string BiarchSuffix,
+                                      std::string MIPSABIDirSuffix)
+  : GCCSuffix(BiarchSuffix), GCCMIPSABIDirSuffix(MIPSABIDirSuffix) {
+}
+
 /// \brief Construct a GCCInstallationDetector from the driver.
 ///
 /// This performs all of the autodetection and sets up the various paths.
@@ -1363,26 +1372,6 @@
   return llvm::sys::fs::exists(Path + "/crtbegin.o");
 }
 
-static bool findTargetBiarchSuffix(std::string &Suffix, StringRef Path,
-                                   llvm::Triple::ArchType TargetArch,
-                                   const ArgList &Args) {
-  // FIXME: This routine was only intended to model bi-arch toolchains which
-  // use -m32 and -m64 to swap between variants of a target. It shouldn't be
-  // doing ABI-based builtin location for MIPS.
-  if (hasMipsN32ABIArg(Args))
-    Suffix = "/n32";
-  else if (TargetArch == llvm::Triple::x86_64 ||
-           TargetArch == llvm::Triple::ppc64 ||
-           TargetArch == llvm::Triple::systemz ||
-           TargetArch == llvm::Triple::mips64 ||
-           TargetArch == llvm::Triple::mips64el)
-    Suffix = "/64";
-  else
-    Suffix = "/32";
-
-  return hasCrtBeginObj(Path + Suffix);
-}
-
 void Generic_GCC::GCCInstallationDetector::findMIPSABIDirSuffix(
     std::string &Suffix, llvm::Triple::ArchType TargetArch, StringRef Path,
     const llvm::opt::ArgList &Args) {
@@ -1518,28 +1507,8 @@
       if (CandidateVersion <= Version)
         continue;
 
-      std::string MIPSABIDirSuffix;
-      if (isMipsArch(TargetArch))
-        findMIPSABIDirSuffix(MIPSABIDirSuffix, TargetArch, LI->path(), Args);
-
-      // Some versions of SUSE and Fedora on ppc64 put 32-bit libs
-      // in what would normally be GCCInstallPath and put the 64-bit
-      // libs in a subdirectory named 64. The simple logic we follow is that
-      // *if* there is a subdirectory of the right name with crtbegin.o in it,
-      // we use that. If not, and if not a biarch triple alias, we look for
-      // crtbegin.o without the subdirectory.
-
-      std::string BiarchSuffix;
-      if (findTargetBiarchSuffix(BiarchSuffix,
-                                 LI->path() + MIPSABIDirSuffix,
-                                 TargetArch, Args)) {
-        GCCBiarchSuffix = BiarchSuffix;
-      } else if (NeedsBiarchSuffix ||
-                 !hasCrtBeginObj(LI->path() + MIPSABIDirSuffix)) {
+      if (!findMultiarchLib(TargetArch, LI->path(), Args, NeedsBiarchSuffix))
         continue;
-      } else {
-        GCCBiarchSuffix.clear();
-      }
 
       Version = CandidateVersion;
       GCCTriple.setTriple(CandidateTriple);
@@ -1548,12 +1517,63 @@
       // Linux.
       GCCInstallPath = LibDir + LibSuffixes[i] + "/" + VersionText.str();
       GCCParentLibPath = GCCInstallPath + InstallSuffixes[i];
-      GCCMIPSABIDirSuffix = MIPSABIDirSuffix;
       IsValid = true;
     }
   }
 }
 
+bool Generic_GCC::GCCInstallationDetector::findMultiarchLib(
+  llvm::Triple::ArchType TargetArch, std::string Path,
+  const ArgList &Args, bool NeedsBiarchSuffix) {
+  
+  if (isMipsArch(TargetArch)) {
+    std::string MIPSABIDirSuffix;
+    findMIPSABIDirSuffix(MIPSABIDirSuffix, TargetArch, Path, Args);
+
+    std::string BiarchSuffix;
+    if (hasMipsN32ABIArg(Args))
+      BiarchSuffix = "/n32";
+    else if (TargetArch == llvm::Triple::mips64 ||
+             TargetArch == llvm::Triple::mips64el)
+      BiarchSuffix = "/64";
+    else
+      BiarchSuffix = "/32";
+
+    if (hasCrtBeginObj(Path + MIPSABIDirSuffix + BiarchSuffix))
+      MultiarchLib = GCCMultiarchLib(BiarchSuffix, MIPSABIDirSuffix);
+    else if (hasCrtBeginObj(Path + MIPSABIDirSuffix))
+      MultiarchLib = GCCMultiarchLib("", MIPSABIDirSuffix);
+    else
+      return false;
+  } else {
+    // Some versions of SUSE and Fedora on ppc64 put 32-bit libs
+    // in what would normally be GCCInstallPath and put the 64-bit
+    // libs in a subdirectory named 64. The simple logic we follow is that
+    // *if* there is a subdirectory of the right name with crtbegin.o in it,
+    // we use that. If not, and if not a biarch triple alias, we look for
+    // crtbegin.o without the subdirectory.
+
+    std::string BiarchSuffix;
+    if (TargetArch == llvm::Triple::x86_64 ||
+        TargetArch == llvm::Triple::ppc64 ||
+        TargetArch == llvm::Triple::systemz)
+      BiarchSuffix = "/64";
+    else
+      BiarchSuffix = "/32";
+
+    if (hasCrtBeginObj(Path + BiarchSuffix))
+      MultiarchLib = GCCMultiarchLib(BiarchSuffix, "");
+    else if (NeedsBiarchSuffix)
+      return false;
+    else if (hasCrtBeginObj(Path))
+      MultiarchLib = GCCMultiarchLib("", "");
+    else
+      return false;
+  }
+
+  return true;
+}
+
 Generic_GCC::Generic_GCC(const Driver &D, const llvm::Triple& Triple,
                          const ArgList &Args)
   : ToolChain(D, Triple, Args), GCCInstallation(getDriver(), Triple, Args) {
@@ -2324,6 +2344,8 @@
   if (llvm::sys::fs::exists(Path)) Paths.push_back(Path.str());
 }
 
+// NB: don't be confused on the name of this with GCC's use of the word multilib.
+// Here we are referring to bi-arch lib directories outside of the gcc installation
 static StringRef getMultilibDir(const llvm::Triple &Triple,
                                 const ArgList &Args) {
   if (isMipsArch(Triple.getArch())) {
@@ -2338,7 +2360,7 @@
   // It happens that only x86 and PPC use the 'lib32' variant of multilib, and
   // using that variant while targeting other architectures causes problems
   // because the libraries are laid out in shared system roots that can't cope
-  // with a 'lib32' multilib search path being considered. So we only enable
+  // with a 'lib32' library search path being considered. So we only enable
   // them when we know we may need it.
   //
   // FIXME: This is a bit of a hack. We should really unify this code for
@@ -2434,13 +2456,13 @@
     // FIXME: It would be cleaner to model this as a variant of bi-arch. IE,
     // instead of a '64' biarch suffix it would be 'el' or something.
     if (IsAndroid && IsMips && isMips32r2(Args)) {
-      assert(GCCInstallation.getBiarchSuffix().empty() &&
+      assert(GCCInstallation.getMultiarchLib().getSuffix().empty() &&
              "Unexpected bi-arch suffix");
       addPathIfExists(GCCInstallation.getInstallPath() + "/mips-r2", Paths);
     } else {
       addPathIfExists((GCCInstallation.getInstallPath() +
-                       GCCInstallation.getMIPSABIDirSuffix() +
-                       GCCInstallation.getBiarchSuffix()),
+                       GCCInstallation.getMultiarchLib().getMIPSABIDirSuffix() +
+                       GCCInstallation.getMultiarchLib().getSuffix()),
                       Paths);
     }
 
@@ -2463,7 +2485,7 @@
     // Note that this matches the GCC behavior. See the below comment for where
     // Clang diverges from GCC's behavior.
     addPathIfExists(LibPath + "/../" + GCCTriple.str() + "/lib/../" + Multilib +
-                    GCCInstallation.getMIPSABIDirSuffix(),
+                    GCCInstallation.getMultiarchLib().getMIPSABIDirSuffix(),
                     Paths);
 
     // If the GCC installation we found is inside of the sysroot, we want to
@@ -2494,14 +2516,14 @@
     // Add the non-multilib suffixed paths (if potentially different).
     const std::string &LibPath = GCCInstallation.getParentLibPath();
     const llvm::Triple &GCCTriple = GCCInstallation.getTriple();
-    if (!GCCInstallation.getBiarchSuffix().empty())
+    if (!GCCInstallation.getMultiarchLib().getSuffix().empty())
       addPathIfExists(GCCInstallation.getInstallPath() +
-                      GCCInstallation.getMIPSABIDirSuffix(), Paths);
+                      GCCInstallation.getMultiarchLib().getMIPSABIDirSuffix(), Paths);
 
     // See comments above on the multilib variant for details of why this is
     // included even from outside the sysroot.
     addPathIfExists(LibPath + "/../" + GCCTriple.str() +
-                    "/lib" + GCCInstallation.getMIPSABIDirSuffix(), Paths);
+                    "/lib" + GCCInstallation.getMultiarchLib().getMIPSABIDirSuffix(), Paths);
 
     // See comments above on the multilib variant for details of why this is
     // only included from within the sysroot.
@@ -2554,7 +2576,7 @@
 
   const StringRef InstallDir = GCCInstallation.getInstallPath();
   const StringRef TripleStr = GCCInstallation.getTriple().str();
-  const StringRef MIPSABIDirSuffix = GCCInstallation.getMIPSABIDirSuffix();
+  const StringRef MIPSABIDirSuffix = GCCInstallation.getMultiarchLib().getMIPSABIDirSuffix();
 
   std::string Path = (InstallDir + "/../../../../" + TripleStr + "/libc" +
                       MIPSABIDirSuffix).str();
@@ -2757,8 +2779,8 @@
   StringRef LibDir = GCCInstallation.getParentLibPath();
   StringRef InstallDir = GCCInstallation.getInstallPath();
   StringRef TripleStr = GCCInstallation.getTriple().str();
-  StringRef MIPSABIDirSuffix = GCCInstallation.getMIPSABIDirSuffix();
-  StringRef BiarchSuffix = GCCInstallation.getBiarchSuffix();
+  StringRef MIPSABIDirSuffix = GCCInstallation.getMultiarchLib().getMIPSABIDirSuffix();
+  StringRef BiarchSuffix = GCCInstallation.getMultiarchLib().getSuffix();
   const GCCVersion &Version = GCCInstallation.getVersion();
 
   if (addLibStdCXXIncludePaths(LibDir.str() + "/../include",
Index: lib/Driver/ToolChains.h
===================================================================
--- lib/Driver/ToolChains.h	(revision 195793)
+++ lib/Driver/ToolChains.h	(working copy)
@@ -67,7 +67,33 @@
     bool operator>=(const GCCVersion &RHS) const { return !(*this < RHS); }
   };
 
+  /// This corresponds to what GCC calls a 'multilib'
+  class GCCMultiarchLib {
+    std::string GCCSuffix;
+    std::string GCCMIPSABIDirSuffix;
+  public:
+    GCCMultiarchLib();
+    GCCMultiarchLib(std::string Suffix, std::string MIPSABIDirSuffix);
 
+    /// \brief Get the detected GCC installation path suffix for the multi-arch
+    /// target variant.
+    StringRef getSuffix() const { return GCCSuffix; }
+
+    /// \brief Get the detected GCC MIPS ABI directory suffix.
+    ///
+    /// This is used as a suffix both to the install directory of GCC and as
+    /// a suffix to its parent lib path in order to select a MIPS ABI-specific
+    /// subdirectory.
+    ///
+    /// This will always be empty for any non-MIPS target.
+    ///
+    // FIXME: This probably shouldn't exist at all, and should be factored
+    // into the multiarch and/or biarch support. Please don't add more uses of
+    // this interface, it is meant as a legacy crutch for the MIPS driver
+    // logic.
+    StringRef getMIPSABIDirSuffix() const { return GCCMIPSABIDirSuffix; }
+  };
+
   /// \brief This is a class to find a viable GCC installation for Clang to
   /// use.
   ///
@@ -80,10 +106,10 @@
 
     // FIXME: These might be better as path objects.
     std::string GCCInstallPath;
-    std::string GCCBiarchSuffix;
     std::string GCCParentLibPath;
-    std::string GCCMIPSABIDirSuffix;
 
+    GCCMultiarchLib MultiarchLib;
+
     GCCVersion Version;
 
     // We retain the list of install paths that were considered and rejected in
@@ -103,26 +129,11 @@
     /// \brief Get the detected GCC installation path.
     StringRef getInstallPath() const { return GCCInstallPath; }
 
-    /// \brief Get the detected GCC installation path suffix for the bi-arch
-    /// target variant.
-    StringRef getBiarchSuffix() const { return GCCBiarchSuffix; }
-
     /// \brief Get the detected GCC parent lib path.
     StringRef getParentLibPath() const { return GCCParentLibPath; }
 
-    /// \brief Get the detected GCC MIPS ABI directory suffix.
-    ///
-    /// This is used as a suffix both to the install directory of GCC and as
-    /// a suffix to its parent lib path in order to select a MIPS ABI-specific
-    /// subdirectory.
-    ///
-    /// This will always be empty for any non-MIPS target.
-    ///
-    // FIXME: This probably shouldn't exist at all, and should be factored
-    // into the multiarch and/or biarch support. Please don't add more uses of
-    // this interface, it is meant as a legacy crutch for the MIPS driver
-    // logic.
-    StringRef getMIPSABIDirSuffix() const { return GCCMIPSABIDirSuffix; }
+    /// \brief Get the detected multilib
+    const GCCMultiarchLib &getMultiarchLib() const { return MultiarchLib; }
 
     /// \brief Get the detected GCC version string.
     const GCCVersion &getVersion() const { return Version; }
@@ -145,6 +156,11 @@
                                 StringRef CandidateTriple,
                                 bool NeedsBiarchSuffix = false);
 
+    bool findMultiarchLib(llvm::Triple::ArchType TargetArch,
+                          std::string Path,
+                          const llvm::opt::ArgList &Args,
+                          bool NeedsBiarchSuffix);
+
     void findMIPSABIDirSuffix(std::string &Suffix,
                               llvm::Triple::ArchType TargetArch, StringRef Path,
                               const llvm::opt::ArgList &Args);


More information about the cfe-commits mailing list