On Fri, Dec 4, 2020 at 5:45 AM Martin Liška <mli...@suse.cz> wrote:
>
> PING
>
> May I please ping the patch, it's waiting here for a review
> for quite some time.
>
> Thanks,
> Martin
Ping. I think Martin LGTMed this patch and was waiting for a
maintainer to merge it
> On 7/23/20 12:17 PM, 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 (&cpath, 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 (&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);
> >> + 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 (&cpath, ld_path, X_OK);
> >> + if (ld_file_name == 0)
> >> + ld_file_name = find_a_file (&path, ld_path, X_OK);
> >> + }
> >> + else if (file_exists (ld_path))
> >> + {
> >
> > ^^^ these braces are not needed.
> >
> >> + 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'.
> >
> >> + 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);
> >> + }
> >> #ifdef REAL_NM_FILE_NAME
> >> nm_file_name = find_a_file (&path, REAL_NM_FILE_NAME, X_OK);
> >> @@ -1461,6 +1491,11 @@ main (int argc, char **argv)
> >> ld2--;
> >> #endif
> >> }
> >> + else if (strncmp (arg, "--ld-path=", 10) == 0)
> >> + {
> >> + ld1--;
> >> + ld2--;
> >> + }
> >
> > Can you please explain more this change?
> >
> > Thank you,
> > Martin
> >
> >> else if (strncmp (arg, "--sysroot=", 10) == 0)
> >> target_system_root = arg + 10;
> >> else if (strcmp (arg, "--version") == 0)
> >> diff --git a/gcc/common.opt b/gcc/common.opt
> >> index a3893a4725e..5adbd8c18a3 100644
> >> --- a/gcc/common.opt
> >> +++ b/gcc/common.opt
> >> @@ -2940,6 +2940,10 @@ fuse-ld=lld
> >> Common Driver Negative(fuse-ld=lld)
> >> Use the lld LLVM linker instead of the default linker.
> >> +-ld-path=
> >> +Common Driver Joined
> >> +Use the specified executable instead of the default linker.
> >> +
> >> fuse-linker-plugin
> >> Common Undocumented Var(flag_use_linker_plugin)
> >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> >> index ba18e05fb1a..e185e40ffac 100644
> >> --- a/gcc/doc/invoke.texi
> >> +++ b/gcc/doc/invoke.texi
> >> @@ -14718,6 +14718,15 @@ Use the @command{gold} linker instead of the
> >> default linker.
> >> @opindex fuse-ld=lld
> >> Use the LLVM @command{lld} linker instead of the default linker.
> >> +@item --ld-path=@var{linker}
> >> +@opindex -ld-path=linker
> >> +Use the specified executable named @var{linker} instead of the default
> >> linker.
> >> +If @var{linker} does not contain any path component separator (e.g. a
> >> slash),
> >> +the linker will be searched for using @env{COMPILER_PATH} followed by
> >> +@env{PATH}. Otherwise, it is either an absolute path or a path relative
> >> to the
> >> +current working directory. If @option{-fuse-ld=} is also specified, the
> >> linker
> >> +path is specified by @option{--ld-path=}.
> >> +
> >> @cindex Libraries
> >> @item -l@var{library}
> >> @itemx -l @var{library}
> >> diff --git a/gcc/gcc.c b/gcc/gcc.c
> >> index c0eb3c10cfd..05fa5375f06 100644
> >> --- a/gcc/gcc.c
> >> +++ b/gcc/gcc.c
> >> @@ -1077,7 +1077,7 @@ proper position among the other output files. */
> >> LINK_PLUGIN_SPEC \
> >> "%{flto|flto=*:%<fcompare-debug*} \
> >> %{flto} %{fno-lto} %{flto=*} %l " LINK_PIE_SPEC \
> >> - "%{fuse-ld=*:-fuse-ld=%*} " LINK_COMPRESS_DEBUG_SPEC \
> >> + "%{fuse-ld=*:-fuse-ld=%*} %{-ld-path=*:--ld-path=%*} "
> >> LINK_COMPRESS_DEBUG_SPEC \
> >> "%X %{o*} %{e*} %{N} %{n} %{r}\
> >> %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!r:%{!nostartfiles:%S}}} \
> >> %{static|no-pie|static-pie:} %@{L*} %(mfwrap) %(link_libgcc) " \
> >> diff --git a/gcc/opts.c b/gcc/opts.c
> >> index 499eb900643..5cbf9b85549 100644
> >> --- a/gcc/opts.c
> >> +++ b/gcc/opts.c
> >> @@ -2755,6 +2755,7 @@ common_handle_option (struct gcc_options *opts,
> >> dc->max_errors = value;
> >> break;
> >> + case OPT__ld_path_:
> >> case OPT_fuse_ld_bfd:
> >> case OPT_fuse_ld_gold:
> >> case OPT_fuse_ld_lld:
> >>
> >
>
--
宋方睿
