Page MenuHomePhabricator

[Driver] Recognize -fuse-ld={bfd,gold,lld} but don't prepend "ld." or "ld64." for other values
AbandonedPublic

Authored by MaskRay on May 19 2020, 11:04 AM.

Details

Summary

However, the prefix (ld. or ld64.) is actually cumbersome:

  • For a relative path, -fuse-ld=dir/foo currently tries to access x86_64-unknown-linux-gnu-ld.dir/foo (if LLVM_DEFAULT_TARGET_TRIPLE is x86_64-unknown-linux-gnu).
  • wasm and Windows do not seem to need the ld. convention.

We could teach -fuse-ld= to check whether there is a path separator, and
omit the ld. or ld64. prefix, but then a relative path will be
unnecessarily inconsistent with a single path component.

Let's hard code the currently used values which intend to get a prefix:
bfd, gold, lld. For all other values, don't add a prefix.

GCC currently hard codes -fuse-ld={bfd,gold,lld} but does not support
other values. I am going to make it support arbitrary values.
https://sourceware.org/pipermail/gcc-patches/2020-May/546277.html

Diff Detail

Unit TestsFailed

TimeTest
590 msClang.Driver::Unknown Unit Message ("")
Script: -- : 'RUN: at line 4'; c:\ws\beta\llvm-project\build\bin\clang.exe C:\ws\beta\llvm-project\clang\test\Driver\fuse-ld-windows.c -### -o C:\ws\beta\llvm-project\build\tools\clang\test\Driver\Output\fuse-ld-windows.c.tmp.o -target i386-unknown-linux -B C:\ws\beta\llvm-project\clang\test\Driver/Inputs/fuse_ld_windows -fuse-ld=foo 2>&1 | c:\ws\beta\llvm-project\build\bin\filecheck.exe C:\ws\beta\llvm-project\clang\test\Driver\fuse-ld-windows.c

Event Timeline

MaskRay created this revision.May 19 2020, 11:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2020, 11:04 AM
MaskRay updated this revision to Diff 264995.May 19 2020, 12:36 PM

Fix fuse-ld-windows.c (I don't have Windows (REQUIRES: system-windows), so I failed to catch the issue earlier)

MaskRay updated this revision to Diff 265026.May 19 2020, 1:43 PM

Fix fuse-ld-windows.c

I'm a bit nervous about this. I'm aware of at least one out-of-tree toolchain that uses this mechanism to select their proprietary linker. I'd expect an RFC and for this to be highlighted in LLVM Weekly before I'm happy that this won't break downstream consumers.

I'm a bit nervous about this. I'm aware of at least one out-of-tree toolchain that uses this mechanism to select their proprietary linker. I'd expect an RFC and for this to be highlighted in LLVM Weekly before I'm happy that this won't break downstream consumers.

Created http://lists.llvm.org/pipermail/cfe-dev/2020-May/065518.html

rnk added a comment.May 20 2020, 2:27 PM

Personally, I support this. I never liked the magic ld. prefixing. If we want a way to select a proprietary linker, it seems reasonable to ask the user to name the complete binary basename (ld.lld, lld, ld64, myld, etc). The driver shouldn't force the linker to use some special executable naming scheme.

It's worrying to me that there number of places in LLVM that at the exact argument value of "-fuse-ld=". E.g. in the windows and PS4 toolchains. We already claim to support arbitrary values and full paths, but if you specify "-fuse-ld=/path/to/lld-link" on Windows today, you end up with different behavior than "-fuse-ld=lld" (which gets translated to searching for an "lld-link" binary, but also triggers other conditions).

That's not a reason to not make this particular change, but if conditionals on the flavor of linker are going to be used, that seems like perhaps a reason why we should not accept arbitrary values at all?

keith added a subscriber: keith.May 27 2020, 1:53 PM

I've submitted a related change to accept relative paths for -fuse-ld https://reviews.llvm.org/D80660

MaskRay added a comment.EditedMay 28 2020, 6:07 PM

@theraven Are you ok with this change? You seem to have a use case not using ld.bfd, ld.gold or ld.lld . This change will require you to adapt.

It's worrying to me that there number of places in LLVM that at the exact argument value of "-fuse-ld=". E.g. in the windows and PS4 toolchains. We already claim to support arbitrary values and full paths, but if you specify "-fuse-ld=/path/to/lld-link" on Windows today, you end up with different behavior than "-fuse-ld=lld" (which gets translated to searching for an "lld-link" binary, but also triggers other conditions).

That's not a reason to not make this particular change, but if conditionals on the flavor of linker are going to be used, that seems like perhaps a reason why we should not accept arbitrary values at all?

This is indeed unfortunate, but I can see the reasons people customize linker options for different linkers. An arbitrary value is a bit convenient when people want to compare behavior differences between different versions of a linker. As a linker developer I do this a lot. Some ClangBuiltLinux folks do this. (In addition, an absolute path can sometimes give extra confidence that a particular executable is picked -> -fuse-ld=lld may pick one from PATH if ld.lld does not sit beside clang.) Now @keith also expresses interest in such a feature. I think it is really difficult for us to drop it now.

MaskRay edited the summary of this revision. (Show Details)May 28 2020, 6:07 PM
MaskRay abandoned this revision.Jul 2 2020, 11:24 AM

Abandon in favor of D83015