[PATCH] D51464: clang: fix MIPS/N32 triple and paths

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan added a comment.

Could you please include more context to patches sent for review?
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface



================
Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:109
 
+  if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+    ABIName = "n32";
----------------
If target triple is mips-mti-linux-gnuabin32 the code above set (incorrectly) the `ABIName` to `n64` and this statement will be `false`.


================
Comment at: lib/Driver/ToolChains/Linux.cpp:47
   bool IsAndroid = TargetTriple.isAndroid();
+  std::string MipsCpu = "", Mips64Abi = "gnuabi64";
+  if (TargetEnvironment == llvm::Triple::GNUABIN32)
----------------
- Do you need `MipsCpu` variable?
- Is it possible to use any lightweight type like `StringRef` for the `Mips64Abi`?


================
Comment at: lib/Driver/ToolChains/Linux.cpp:118
   case llvm::Triple::mips64:
     if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnu"))
       return "mips64-linux-gnu";
----------------
If a user has two toolchains installed into "/lib/mips64-linux-gnu" and "/lib/mips64-linux-gnuabin32", this code always selects mips64-linux-gnu even if N32 ABI is requested. Is it intended?


Repository:
  rC Clang

https://reviews.llvm.org/D51464