Re: RFA/RFC: Enable both gold and ld in a single toolchain

[Ralf Wildenhues <Ralf dot Wildenhues at gmx dot de>]

Hello Nick,

just curious, what does your patch offer over H.J.'s patch?

A few nits:

* Nick Clifton wrote on Thu, Mar 04, 2010 at 08:21:45PM CET:
>   In order to build both linkers it is necessary to run the top level
>   configure script with the switch:
> 
>     --enable-gold=both
> 
>   Then once the linkers are built/installed you can add:
> 
>     -Luse-gold

FWIW, I don't consider abusing existing APIs like this to be a good
idea.  Collect2 is something GCC controls, so you can adjust it; that
doesn't justify in-band signaling in user APIs if you ask me.

BTW, installation order requirements can (and should IMVHO) be
formalized in the toplevel Makefile.def, so parallel make does not fail.

> --- configure.ac	(revision 157226)
> +++ configure.ac	(working copy)
> @@ -320,7 +320,7 @@
>  [  --enable-gold           use gold instead of ld],
>  ENABLE_GOLD=$enableval,
>  ENABLE_GOLD=no)
> -if test "${ENABLE_GOLD}" = "yes"; then
> +if test "${ENABLE_GOLD}" = "yes" -o "${ENABLE_GOLD}" = "both"; then

test -o is not POSIX, please use test || test instead.

> --- gcc/exec-tool.in	(revision 157226)
> +++ gcc/exec-tool.in	(working copy)

> @@ -35,15 +37,47 @@
>      dir=gas
>      ;;
>    collect-ld)
> -    # when using a linker plugin, gcc will always pass '-plugin' as the
> -    # first option to the linker.
> -    if test x"$1" = "x-plugin"; then
> -      original=$ORIGINAL_PLUGIN_LD_FOR_TARGET
> -    else
> -      original=$ORIGINAL_LD_FOR_TARGET
> +    prog=ld-new$exeext
> +    # Look for the magic library paths "-Luse-gold" and
> +    # "-Luse-ld".  If present then select the indicated
> +    # linker.  Otherwise if -plugin is specified then
> +    # choose a plugin-capable linker, otherwise use the
> +    # default.
> +    case "${1+$@} " in

${1+$@} is only necessary in places where zero arguments need to be
detected as such, and it is only correct if the quotes are around the $@
alone.  In a case statement, plain $* is sufficient (and more portable):
      case " $* " in ...

but I would usually prefer iterating over the positional parameters:
      for arg
      do
        case $arg in
          -plugin) ...
          ;;
        esac
      done

> +      *-Luse-gold\ *)

Without leading space, this also matches unrelated arguments that only
happen to end in this string.

> +        original=$ORIGINAL_GOLD_FOR_TARGET
> +	dir=gold
> +        ;;
[...]

> +    # If the selected linker has not been configured then
> +    # try using the others, in the order PLUGIN-LD, LD, GOLD.
> +    if test x"$original" == x; then

test == is a bashism, = is portable.

> @@ -52,36 +86,65 @@
>      ;;
>  esac
>  
> -case "$original" in
> -  ../*)
> -    # compute absolute path of the location of this script
> +case x"$original" in

Why that x here?  The x used in "test" is to avoid leading hyphens
to let old test programs confuse them with options like -f.  This is not
needed for case.

> +  x../*)
> +    # Compute absolute path to the location of this script.
>      tdir=`dirname "$0"`
>      scriptdir=`cd "$tdir" && pwd`
>  
>      if test -x $scriptdir/../$dir/$prog; then
> -      test "$fast_install" = yes || exec $scriptdir/../$dir/$prog ${1+"$@"}
> +      if test "$fast_install" = yes; then
> +        # If libtool did everything it needs to do, there's a fast path.
> +        lt_prog=$scriptdir/../$dir/$objdir/lt-$prog 
>  
> -      # if libtool did everything it needs to do, there's a fast path
> -      lt_prog=$scriptdir/../$dir/$objdir/lt-$prog 
> -      test -x $lt_prog && exec $lt_prog ${1+"$@"}
> -
> -      # libtool has not relinked ld-new yet, but we cannot just use the
> -      # previous stage (because then the relinking would just never happen!).
> -      # So we take extra care to use prev-ld/ld-new *on recursive calls*.
> -      test x"$LT_RCU" = x"1" && exec $scriptdir/../prev-$dir/$prog ${1+"$@"}
> -
> -      LT_RCU=1; export LT_RCU
> -      $scriptdir/../$dir/$prog ${1+"$@"}
> -      result=$?
> -      exit $result
> -
> +	if test -x $lt_prog; then
> +	  original=$lt_prog
> +        else
> +          # Libtool has not relinked ld-new yet, but we cannot just use the
> +          # previous stage (because then the relinking would just never happen!).
> +          # So we take extra care to use prev-ld/ld-new *on recursive calls*.
> +          if test x"$LT_RCU" = x"1"; then
> +	    original=$scriptdir/../prev-$dir/$prog
> +          else
> +            LT_RCU=1; export LT_RCU
> +            case "${1+$@} " in
> +              *-v\ *)

See above.

> +               echo "$invoked version $version"

This is not a GCS-compatible version output.

> +               echo $scriptdir/../$dir/$prog ${1+"$@"}
> +               ;;
> +            esac
> +            $scriptdir/../$dir/$prog ${1+"$@"}
> +            result=$?
> +            exit $result
> +          fi
> +        fi
> +      else
> +	original=$scriptdir/../$dir/$prog
> +      fi
>      else
> -      exec $scriptdir/../prev-$dir/$prog ${1+"$@"}
> +      original=$scriptdir/../prev-$dir/$prog
>      fi
>      ;;
> -  *)
> -    exec "$original" ${1+"$@"}
> +  x)
> +    echo "$invoked: executable not configured"
> +    exit -1

I don't know whether -1 is portable for shell exit.

>      ;;
>  esac
>  
> +# If -v has been used then display our version number
> +# and then echo the command we are about to invoke.
> +case "${1+$@} " in
> +  *-v\ *)

See above.

> +    echo "$invoked version $version"
> +    echo $original ${1+"$@"}
> +    ;;
> +esac
>  
> +if test -x $original; then
> +  exec "$original" ${1+"$@"}
> +else
> +  echo "$invoked: unable to locate executable: $original"
> +  exit -1
> +fi

Cheers,
Ralf