Re: [PATCH] Do not prepend target triple to -fuse-ld=lld,mold.

[Tatsuyuki Ishi <ishitatsuyuki@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6237 bytes --]

> On Nov 7, 2023, at 23:37, Richard Biener <rguenther@suse.de> wrote:
> 
> On Tue, 7 Nov 2023, Tatsuyuki Ishi wrote:
> 
>>> On Oct 16, 2023, at 18:16, Richard Biener <rguenther@suse.de> wrote:
>>> 
>>> On Mon, 16 Oct 2023, Tatsuyuki Ishi wrote:
>>> 
>>>> 
>>>> 
>>>>> On Oct 16, 2023, at 17:55, Richard Biener <rguenther@suse.de> wrote:
>>>>> 
>>>>> On Mon, 16 Oct 2023, Tatsuyuki Ishi wrote:
>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> On Oct 16, 2023, at 17:39, Richard Biener <rguenther@suse.de> wrote:
>>>>>>> 
>>>>>>> On Mon, 16 Oct 2023, Tatsuyuki Ishi wrote:
>>>>>>> 
>>>>>>>> lld and mold are platform-agnostic and not prefixed with target triple.
>>>>>>>> Prepending the target triple makes it less likely to find the intended
>>>>>>>> linker executable.
>>>>>>>> 
>>>>>>>> A potential breaking change is that we no longer try to search for
>>>>>>>> triple-prefixed lld/mold binaries anymore. However, since there doesn't
>>>>>>>> seem to be support to build LLVM or mold with triple-prefixed executable
>>>>>>>> names, it seems better to just not bother with that case.
>>>>>>>> 
>>>>>>>> 	PR driver/111605
>>>>>>>> 
>>>>>>>> gcc/Changelog:
>>>>>>>> 
>>>>>>>> 	* collect2.cc (main): Do not prepend target triple to
>>>>>>>> 	-fuse-ld=lld,mold.
>>>>>>>> ---
>>>>>>>> gcc/collect2.cc | 13 ++++++++-----
>>>>>>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>>>>>> 
>>>>>>>> diff --git a/gcc/collect2.cc b/gcc/collect2.cc
>>>>>>>> index 63b9a0c233a..c943f9f577c 100644
>>>>>>>> --- a/gcc/collect2.cc
>>>>>>>> +++ b/gcc/collect2.cc
>>>>>>>> @@ -865,12 +865,15 @@ main (int argc, char **argv)
>>>>>>>> int i;
>>>>>>>> 
>>>>>>>> for (i = 0; i < USE_LD_MAX; i++)
>>>>>>>> -    full_ld_suffixes[i]
>>>>>>>> #ifdef CROSS_DIRECTORY_STRUCTURE
>>>>>>>> -      = concat (target_machine, "-", ld_suffixes[i], NULL);
>>>>>>>> -#else
>>>>>>>> -      = ld_suffixes[i];
>>>>>>>> -#endif
>>>>>>>> +    /* lld and mold are platform-agnostic and not prefixed with target
>>>>>>>> +       triple.  */
>>>>>>>> +    if (!(i == USE_LLD_LD || i == USE_MOLD_LD))
>>>>>>>> +      full_ld_suffixes[i] = concat (target_machine, "-", ld_suffixes[i],
>>>>>>>> +				    NULL);
>>>>>>>> +    else
>>>>>>>> +#endif
>>>>>>>> +      full_ld_suffixes[i] = ld_suffixes[i];
>>>>>>>> 
>>>>>>>> p = argv[0] + strlen (argv[0]);
>>>>>>>> while (p != argv[0] && !IS_DIR_SEPARATOR (p[-1]))
>>>>>>> 
>>>>>>> Since we later do
>>>>>>> 
>>>>>>> /* Search the compiler directories for `ld'.  We have protection against
>>>>>>>  recursive calls in find_a_file.  */
>>>>>>> if (ld_file_name == 0)
>>>>>>> ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker], 
>>>>>>> X_OK);
>>>>>>> /* Search the ordinary system bin directories
>>>>>>>  for `ld' (if native linking) or `TARGET-ld' (if cross).  */
>>>>>>> if (ld_file_name == 0)
>>>>>>> ld_file_name = find_a_file (&path, full_ld_suffixes[selected_linker], 
>>>>>>> X_OK);
>>>>>>> 
>>>>>>> I wonder how having full_ld_suffixes[LLD|MOLD] == ld_suffixes[LLD|MOLD]
>>>>>>> fixes anything?
>>>>>> 
>>>>>> Per the linked PR, the intended use case for this is when one wants to use their system lld/mold with a separately packaged cross toolchain, without requiring them to symlink their system lld/mold into the cross toolchain bin directory.
>>>>>> 
>>>>>> (Note that the first search is against COMPILER_PATH while the latter is 
>>>>>> against PATH).
>>>>> 
>>>>> Ah.  So what about instead adding here
>>>>> 
>>>>> /* Search the ordinary system bin directories for mold/lld even in
>>>>>    a cross configuration.  */
>>>>> if (ld_file_name == 0
>>>>>     && selected_linker == ...)
>>>>>   ld_file_name = find_a_file (&path, ld_suffixes[selected_linker], X_OK);
>>>>> 
>>>>> instead?  That would keep things working in case the user has a
>>>>> xyz-arch-mold in the system dir but uses GNU ld on the host
>>>>> otherwise, lacking a 'mold' binary there?
>>>>> 
>>>>> That is, we'd only add, not change what we search for.
>>>> 
>>>> I considered that, but as described in commit message, it doesn?t seem anyone has created stuff named xyz-arch-lld or xyz-arch-mold. Closest is Gentoo?s symlink mentioned in this thread, but that?s xyz-arch-ld -> ld.lld/mold.
>>>> As such, this feels like a quirk, not something we need to keep compatibility for.
>>> 
>>> I don't have a good idea whether this is the case or not unfortunately
>>> so if it's my call I would err on the safe side.
>>> 
>>> We seem to recognize mold and lld only since GCC 12 which both are
>>> still maintained so I think we might want to do the change on all
>>> those branches?
>>> 
>>> If you feel confident there's indeed no such installs then let's go
>>> with your original patch.
>>> 
>>> Thus, OK for trunk and the affected branches after a while of no
>>> reported issues.
>> 
>> Hi,
>> 
>> Can I consider this an approval for this patch to be applied to trunk?
> 
> Yes.
> 
>> I would appreciate if this patch could be tested in GCC 14 prereleases.
>> 
>> I suppose backporting after no reported issues in GCC 14 would be the plan here?
>> 
>> Please let me know in case of misunderstandings.
> 
> You understood correctly.
> 
> Richard.

Hi Richard,

I forgot to mention that I don’t have committer / write access.

Could you help me get this patch committed? Thanks.

Tatsuyuki.

>> Thanks,
>> Tatsuyuki.
>> 
>>> Thanks,
>>> Richard.
>>> 
>>>> The proposed change seems simple enough though, so if you consider this 
>>>> a compatibility issue I can go for that way as well.
>>> 
>>>> Tatsuyuki.
>>>> 
>>>>> 
>>>>> Thanks,
>>>>> Richard.
>>>> 
>>>> 
>>> 
>>> -- 
>>> Richard Biener <rguenther@suse.de <mailto:rguenther@suse.de> <mailto:rguenther@suse.de>>
>>> SUSE Software Solutions Germany GmbH,
>>> Frankenstrasse 146, 90461 Nuernberg, Germany;
>>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de <mailto:rguenther@suse.de>>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)