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

[Richard Biener <rguenther@suse.de>]

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.

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>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)