[PATCH] D37727: Driver: Remove custom MinGW linker detection

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

In https://reviews.llvm.org/D37727#867627, @mstorsjo wrote:

> I'm not sure I like this direction. In my setups, a build-time default isn't right since I'm using the same clang builds for both native-on-linux building (with the default `ld` as linker) and cross-building targeting windows (using `ldd`), and keeping the code that reads `-fuse-ld=` is essential.


The `GetLinkerPath` function in `lib/Driver/ToolChain.cpp` that I replaced this with already checks `-fuse-ld` and does everything we need correctly.

> I haven't followed the changes closely on how this works with the build-time `CLANG_DEFAULT_LINKER`, but can't we just make the current code check this define if `-fuse-ld` isn't set?

If you specifically want a toolchain that uses lld by default for MINGW but ld for the platform and or host you should override the `getDefaultLinker` function in the Mingw driver with an out of tree patch.
This is now what I do in my own builds since applying this patch. Hopefully when the MinGW driver becomes more stable such a patch can land in tree :)

If you are happy with all being lld or all ld you should set CLANG_DEFAULT_LINKER.
Note that `GetLinkerPath` also gives us the extra option to pass `-fuse-ld=platform` to use the platform default instead of the compile time default.



================
Comment at: lib/Driver/ToolChains/MinGW.cpp:107
 
-  StringRef LinkerName = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "lld");
-  if (LinkerName.equals_lower("lld")) {
----------------
mstorsjo wrote:
> This diff isn't based on the current master version, where this defaults to `ld`
Yeah the line was supposed to read
`StringRef LinkerName = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "ld");`
It was applied on top a commit for clang that changed ld -> lld here.
The reason I want to merge this is so that I can stop doing that :)
For the review because I am removing the line it does not matter much, I will update the diff to remove the extra `l` in the next revision.



================
Comment at: lib/Driver/ToolChains/MinGW.cpp:236
         CmdArgs.push_back("--end-group");
-      else if (!LinkerName.equals_lower("lld"))
+      else
         AddLibGCC(Args, CmdArgs);
----------------
mstorsjo wrote:
> So if you do a clang build that defaults to `lld`, this should suddenly be included?
The previous check was if not lld so it would include it if it was ld or anything else.
The MINGW lld driver does not complain about this, the behaviour should be the same for both.


Repository:
  rL LLVM

https://reviews.llvm.org/D37727