Re: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker

2020-07-26 Thread Fāng-ruì Sòng via Gcc-patches
On Sun, Jul 26, 2020 at 4:02 AM Andreas Schwab  wrote:
>
> On Jul 25 2020, Fāng-ruì Sòng wrote:
>
> > On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab  
> > wrote:
> >>
> >> On Jul 24 2020, Fangrui Song via Gcc-patches wrote:
> >>
> >> > This is to mimick nearly lines. collect2 should filter out options like 
> >> > -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 
> >> > 'f'. --ld-path needs its own `else if`.
> >>
> >> If you handle --ld-path as -fld-path then you don't need that.
> >>
> >
> > --ld-path is a deliberate design choice. -f* is not suitable
> > (https://sourceware.org/pipermail/gcc/2020-July/233089.html )
> > clang has several other --*-path= (e.g. --cuda-path)
>
> I don't agree with that opinion.  --foo and -ffoo are equivalent and
> interchangeable.
>
> Andreas.

For options only affecting linking, -f* are minority.

-fgnu-tm enables language-level constructs.
-flto/-fopenmp/-fprofile-arcs/-fsanitize=*/etc affect code generation.
-fuse-ld= is an exception. The other options (the majority) don't
start with -f.


Re: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker

2020-07-26 Thread Andreas Schwab
On Jul 25 2020, Fāng-ruì Sòng wrote:

> On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab  wrote:
>>
>> On Jul 24 2020, Fangrui Song via Gcc-patches wrote:
>>
>> > This is to mimick nearly lines. collect2 should filter out options like 
>> > -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 
>> > 'f'. --ld-path needs its own `else if`.
>>
>> If you handle --ld-path as -fld-path then you don't need that.
>>
>
> --ld-path is a deliberate design choice. -f* is not suitable
> (https://sourceware.org/pipermail/gcc/2020-July/233089.html )

I don't agree with that opinion.  --foo and -ffoo are equivalent and
interchangeable.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


Re: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker

2020-07-26 Thread Fāng-ruì Sòng via Gcc-patches
On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab  wrote:
>
> On Jul 24 2020, Fangrui Song via Gcc-patches wrote:
>
> > This is to mimick nearly lines. collect2 should filter out options like 
> > -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 
> > 'f'. --ld-path needs its own `else if`.
>
> If you handle --ld-path as -fld-path then you don't need that.
>

--ld-path is a deliberate design choice. -f* is not suitable
(https://sourceware.org/pipermail/gcc/2020-July/233089.html )
clang has several other --*-path= (e.g. --cuda-path)


Re: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker

2020-07-25 Thread Andreas Schwab
On Jul 24 2020, Fangrui Song via Gcc-patches wrote:

> This is to mimick nearly lines. collect2 should filter out options like 
> -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. 
> --ld-path needs its own `else if`.

If you handle --ld-path as -fld-path then you don't need that.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


[PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker

2020-07-25 Thread Fangrui Song via Gcc-patches

Attached v3 to address nits.

On 2020-07-23, Martin Liška wrote:

On 7/21/20 6:07 AM, Fangrui Song wrote:

If the value does not contain any path component separator (e.g. a
slash), the linker will be searched for using COMPILER_PATH followed by
PATH. Otherwise, it is either an absolute path or a path relative to the
current working directory.

--ld-path= complements and overrides -fuse-ld={bfd,gold,lld}. If in the
future, we want to make dfferent linker option decisions we can let
-fuse-ld= represent the linker flavor and --ld-path= the linker path.


Hello.

I have just few nits:

=== ERROR type #3: trailing operator (1 error(s)) ===
gcc/collect2.c:1155:14: ld_file_name =



PR driver/93645
* common.opt (--ld-path=): Add --ld-path=
* opts.c (common_handle_option): Handle OPT__ld_path_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
* doc/invoke.texi: Document --ld-path=.

---
Changes in v2:
* Renamed -fld-path= to --ld-path= (clang 12.0.0 new option).
  The option does not affect code generation and is not a language feature,
  -f* is not suitable. Additionally, clang has other similar --*-path=
  options, e.g. --cuda-path=.
---
 gcc/collect2.c  | 63 +++--
 gcc/common.opt  |  4 +++
 gcc/doc/invoke.texi |  9 +++
 gcc/gcc.c   |  2 +-
 gcc/opts.c  |  1 +
 5 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..caa1b96ab52 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -844,6 +844,7 @@ main (int argc, char **argv)
   const char **ld1;
   bool use_plugin = false;
   bool use_collect_ld = false;
+  const char *ld_path = NULL;
   /* The kinds of symbols we will have to consider when scanning the
  outcome of a first pass link.  This is ALL to start with, then might
@@ -961,12 +962,21 @@ main (int argc, char **argv)
if (selected_linker == USE_DEFAULT_LD)
  selected_linker = USE_PLUGIN_LD;
  }
-   else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
- selected_linker = USE_BFD_LD;
-   else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
- selected_linker = USE_GOLD_LD;
-   else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
- selected_linker = USE_LLD_LD;
+   else if (strncmp (argv[i], "-fuse-ld=", 9) == 0
+&& selected_linker != USE_LD_MAX)
+ {
+   if (strcmp (argv[i] + 9, "bfd") == 0)
+ selected_linker = USE_BFD_LD;
+   else if (strcmp (argv[i] + 9, "gold") == 0)
+ selected_linker = USE_GOLD_LD;
+   else if (strcmp (argv[i] + 9, "lld") == 0)
+ selected_linker = USE_LLD_LD;
+ }
+   else if (strncmp (argv[i], "--ld-path=", 10) == 0)
+ {
+   ld_path = argv[i] + 10;
+   selected_linker = USE_LD_MAX;
+ }
else if (strncmp (argv[i], "-o", 2) == 0)
  {
/* Parse the output filename if it's given so that we can make
@@ -1117,14 +1127,34 @@ main (int argc, char **argv)
   ld_file_name = find_a_file (, collect_ld_suffix, X_OK);
   use_collect_ld = ld_file_name != 0;
 }
-  /* 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 (, 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 (, full_ld_suffixes[selected_linker], 
X_OK);
+  if (selected_linker == USE_LD_MAX)
+{
+  /* If --ld-path= does not contain a path component separator, search for
+the command using cpath, then using path.  Otherwise find the linker
+relative to the current working directory.  */
+  if (lbasename (ld_path) == ld_path)
+   {
+ ld_file_name = find_a_file (, ld_path, X_OK);
+ if (ld_file_name == 0)
+   ld_file_name = find_a_file (, ld_path, X_OK);
+   }
+  else if (file_exists (ld_path))
+   {


^^^ these braces are not needed.


Usually, if the 'if' branch has braces, I'd add braces to 'else' as
well.  Updated to follow your advice anyway.


+ ld_file_name = ld_path;
+   }
+}
+  else
+{
+  /* Search the compiler directories for `ld'.  We have protection against
+recursive calls in find_a_file.  */
+  if (ld_file_name == 0)


I would prefer '== NULL'.


Done.


+   ld_file_name = find_a_file (, 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 (, full_ld_suffixes[selected_linker], X_OK);
+}
 #ifdef REAL_NM_FILE_NAME
   nm_file_name = find_a_file (,