[PATCH] Support Sourcery CodeBench MIPS toolchain

[Simon Atanasyan]

  Thanks for your review. I will send a new patch soon.


================
Comment at: lib/Driver/ToolChains.cpp:1125
@@ -1125,1 +1124,3 @@
+    "mipsel-linux-android",
+    "mips-linux-gnu"
   };
----------------
Rafael Ávila de Espíndola wrote:
> mips-linux-gnu is used for both LE and BE?
> 
Yes, some Mips toolchains put everything into the different directories under `mips-linux-gnu` root. For example, big-endian stuff go to the `mips-linux-gnu/<version>/` folder, while little-endian stuff go to the `mips-linux-gnu/<version>/el/` folder

================
Comment at: lib/Driver/ToolChains.cpp:1301
@@ -1269,6 +1300,3 @@
 
-static StringRef getTargetMultiarchSuffix(llvm::Triple::ArchType TargetArch,
-                                          const ArgList &Args) {
-  if (TargetArch == llvm::Triple::x86_64 ||
-      TargetArch == llvm::Triple::ppc64)
-    return "/64";
+static void getMipsTargetSuffix(SmallString<32> &Path,
+                                llvm::Triple::ArchType TargetArch,
----------------
Rafael Ávila de Espíndola wrote:
> s/get/append/ since this modifies the Path.
Agree. Fixed in the new patch.

================
Comment at: lib/Driver/ToolChains.cpp:1304
@@ -1275,1 +1303,3 @@
+                                const ArgList &Args) {
+  Path.clear();
 
----------------
Rafael Ávila de Espíndola wrote:
> Why?
Because it was a //getter// function. I remove this code in the new patch.

================
Comment at: lib/Driver/ToolChains.cpp:1319
@@ +1318,3 @@
+
+static bool getTargetMultiarchSuffix(SmallString<32> &Suffix,
+                                     StringRef Path,
----------------
Rafael Ávila de Espíndola wrote:
> s/get/append/
I suggest s/get/find/. This function searches valid path suffix and returns the search result.

================
Comment at: lib/Driver/ToolChains.cpp:1326
@@ +1325,3 @@
+
+    if (TargetArch == llvm::Triple::mips64 ||
+        TargetArch == llvm::Triple::mips64el)
----------------
Rafael Ávila de Espíndola wrote:
> Cant you merge this to getMipsTargetSuffix?
Unfortunately no. Some paths require //suffix// without //64// tail.

================
Comment at: lib/Driver/ToolChains.cpp:1330
@@ +1329,3 @@
+
+    if (llvm::sys::fs::exists(Path + Suffix.str() + "/crtbegin.o"))
+      return true;
----------------
Rafael Ávila de Espíndola wrote:
> Just
> 
> return llvm::sys::fs::exists(Path + Suffix.str() + "/crtbegin.o";
> 
> no?
No, because if the path does not exist we need to check another //suffix//.

================
Comment at: lib/Driver/ToolChains.cpp:1339
@@ +1338,3 @@
+           TargetArch == llvm::Triple::mips64el)
+    Suffix = hasMipsN32ABIArg(Args) ? "/n32" : "/64";
+  else
----------------
Rafael Ávila de Espíndola wrote:
> Can you please merge this with the above so that the entire funciton looks like
> 
> getTargetMultiarchSuffix(...) {
> ​  if (isMipsArch(TargetArch)) {
>      getMipsTargetSuffix(..);
>     return ...;
>  }
>   // not mips case.
> }
Done in the new patch.

================
Comment at: lib/Driver/ToolChains.cpp:2172
@@ +2171,3 @@
+
+  if (IsMips && !SysRoot.empty() && D.SysRoot.empty())
+    ExtraOpts.push_back("--sysroot=" + SysRoot);
----------------
Rafael Ávila de Espíndola wrote:
> Checking both Sysroot and D.SysRoot is redundant given
> 
> 
> 	​std::string Linux::computeSysRoot(const ArgList &Args) const {
> 		​  if (!getDriver().SysRoot.empty())
> 		​    return getDriver().SysRoot;
> 
> no?
Good point. Fixed.


http://llvm-reviews.chandlerc.com/D644