<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 2, 2020 at 2:19 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jun 1, 2020 at 9:47 PM James Y Knight via cfe-dev<br>
<<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, May 20, 2020 at 3:10 PM Fangrui Song via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
>><br>
>> In <a href="https://reviews.llvm.org/D80225" rel="noreferrer" target="_blank">https://reviews.llvm.org/D80225</a> I intended to update the semantics of -fuse-ld=<br>
>> Some context (from my archaeology) about the option<br>
>><br>
>> * <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470" rel="noreferrer" target="_blank">https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470</a> added support for -fuse-ld=bfd to mean ld.bfd and -fuse-ld=gold to mean ld.gold<br>
>> * rL194328 ported the feature and made -fuse-ld= available for other values (with the ld. prefix)<br>
>> * D78290 changed the prefix to ld64. on Darwin.<br>
>> * GCC added support for -fuse-ld=lld. No other value than bfd,gold,lld is allowed (so our<br>
>>    behavior change here will not be incompatible with GCC).<br>
>><br>
>> Our existing behavior is cumbersome:<br>
>><br>
>> * Inconsistency between an absolute<br>
>>    path (which does not get a ld.  prefix) and a relative path (which gets<br>
>>    a ld. prefix) For a relative path, -fuse-ld=dir/foo currently tries to<br>
>>    access x86_64-unknown-linux-gnu-ld.dir/foo (if<br>
>>    LLVM_DEFAULT_TARGET_TRIPLE is x86_64-unknown-linux-gnu).<br>
>> * lld's new Mach-O port needs to pick an executable name. It would be<br>
>>    nice if -fuse-ld= simply accepts a program name, not necessarily prefixed by `ld64.`<br>
>><br>
>> I suggest we just hard code the currently used values which intend to get<br>
>> a prefix: bfd, gold, lld. For all other values, don't add a prefix.<br>
>> (PS4 seems to use -fuse-ld=ps4 but it overrides ToolChain::GetLinkerPath, thus unaffected)<br>
>><br>
>> Thoughts?<br>
><br>
><br>
> It concerns me that we currently claim to accept arbitrary file paths for -fuse-ld=, yet, we sometimes hardcode special behavior for certain argument values. (Special behavior beyond simply changing which executable name to search for, that is). If we're going to have such conditionals, GCC's behavior of allowing only certain enumerated values would seem to me to be better.<br>
><br>
> However, currently, all of the cases in llvm appear to be either related to Windows toolchains or PS4 toolchain. E.g. here <a href="https://github.com/llvm/llvm-project/blob/3bb0d95fdc2fffd193d39d14f2ef421d4b468617/clang/lib/Driver/Driver.cpp#L4890" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/blob/3bb0d95fdc2fffd193d39d14f2ef421d4b468617/clang/lib/Driver/Driver.cpp#L4890</a> and <a href="https://github.com/llvm/llvm-project/blob/3bb0d95fdc2fffd193d39d14f2ef421d4b468617/clang/lib/Driver/ToolChains/MinGW.cpp#L424" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/blob/3bb0d95fdc2fffd193d39d14f2ef421d4b468617/clang/lib/Driver/ToolChains/MinGW.cpp#L424</a> (just search for OPT_fuse_ld_EQ to find others.)<br>
<br>
Sounds like fixing those is independent of the change you're<br>
proposing, though? I'm not sure I see it as related to this proposal.</blockquote><div><br></div><div>Yes, this is somewhat independent. I bring it up because this proposal is to change the existing behavior of -fuse-ld=word, in order to make it more consistent with -fuse-ld=path. And also there was a suggestion that GCC ought to adopt our behavior.</div><div><br></div><div>But, if there's a requirement to understand which kind of linker the driver is invoking -- which apparently there is -- this seems a wrong direction to go in. And maybe we should instead be deprecating -fuse-ld=PATH. It doesn't seem clear to me that there <i>is</i> really a fix for this. If the driver needs to know what kind of linker it's invoking, then I don't see how it can ever work properly to allow arbitrary values.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> And the PS4 toolchain driver code already prohibits any -fuse-ld= other than the exact strings "ps4" or "gold", so there's no weird behavior changes, only a clear (yet perhaps unexpected) error message. So, maybe we should do the same for windows? But it's rather unfortunate for clang to inconsistently sometimes allow paths as the argument to -fuse-ld= and sometimes not...<br>
<br>
Not sure I follow these pieces. Re: PS4, it's good to know it's not an<br>
issue/wouldn't be affected by this change. But that doesn't mean other<br>
users out there aren't benefiting from/relying on the ld. prefix -<br>
it's functionality LLVM has shipped, users can use, etc, and we would<br>
be changing that out from under them which I think is always something<br>
that should be considered carefully as to what we gain by changing<br>
user-facing behavior.<br>
<br>
Windows: Not sure what part of this discussion would motivate changing<br>
windows to disallow the arbitrary linker path compared to allowing it<br>
on Linux.</blockquote><div><br></div><div>The code in LLVM for Windows and PS4 currently needs to know what linker it's invoking, for correct behavior. Thus, users who specify -fuse-ld=PATH will get incorrect behavior today, even if PATH represents the same binary as -fuse-ld=lld would choose.</div></div></div>