Re: [PATCH] ld: Override default linker emulation at configure time

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

Cupertino,

Thanks for taking the time to look through this patch.  I always
appreciate your feedback.

* Cupertino Miranda <Cupertino.Miranda@synopsys.com> [2017-01-26 13:15:19 +0000]:

> I apologize for just coming with such a question now. I don't really
> know how I missed this thread.
> But why not to specify the triplet with melanox to use the particular
> emulation?
> 
> IMHO, although it is discouraged, as a general rule, to use the vendor
> name to classify a target, in this particular case it makes total sense
> to me.
> It is clear that no one else apart from melanox will ever be using this
> particular linker script.
> 
> Moreover, the required over enginering for the default ARC configure
> process looks to me like a very good reason for to break that rule.

It's not clear which part seems over-engineered, could you explain in
a little more detail what your concerns are please?

When I think of over-engineering I think of introducing overly complex
solutions in order to solve simple problems.  Maybe inventing a new
solution when a solution already exists and can be reused.

As an example, adding a whole new configuration switch like
'--default-linker-emulation=NAME' in order to solve this problem might
be seen as overly complex (if someone was ever silly enough to suggest
such a solution) while reusing an existing solution (say
--with-cpu=NAME) would be much simpler.

> The second thing is that, there is already other target / vendor in
> configure.tgt that does this for the same reason:
>     vax-dec-ultrix* | vax-dec-bsd*) targ_emul=vax ;;
> 
> If we follow the path to check the "with_cpu" flag on the configure.tgt,
> we will also be doing something that certainly should be avoided and
> perhaps lead to future problems.

Can you expand on this too please.  Why should this be avoided?

I and Claudiu have both invested effort in making sure that
--with-cpu=NAME works for the ARC toolchain in both GCC and gas
(though to be clear Claudiu has not done any --with-cpu= work for ld)
however the --with-cpu switch is a well established cpu switch within
binutils and GCC, and given that both Synopsys and others have a
proven interest in this feature working (for ARC) it seems unlikely
that it's going to go away anytime soon.

My two mains dislikes for switching on the vendor string are, first,
it ties us to the vendor string, what if we don't want to use the
vendor name in the target for whatever reason, or what if the vendor
name changes in the future?  Second, having worked so hard to make the
configuration NOT depend on the vendor string in all other parts of
the toolchain, having the linker be the odd-one out feels like a
likely source of confusion.  Some end user builds the toolchain with
--with-cpu=nps400, but miss-spells, or drops the vendor string.
Everything else has configured/built correctly, except for the default
emulation.

> Moreover, ARC will be the only target to do this kind of thing.

There are lots of things that ARC is doing that others are not, like
the table based features that you (and Claudiu) are/have introduced.

Lets think of us as leading the way, hopefully everyone else will
follow our lead!

Thanks,
Andrew


> 
> Best regards,
> Cupertino
> 
> On 01/26/2017 11:41 AM, Andrew Burgess wrote:
> > * Alan Modra <amodra@gmail.com> [2017-01-25 13:22:48 +1030]:
> >
> >> On Tue, Jan 24, 2017 at 10:55:25AM +0000, Andrew Burgess wrote:
> >>> I might end up having to add some code to ld/configure.ac to make
> >>> --with-cpu=NAME visible in the ld configure script.... but how would
> >>> this look to you?
> >> Works for me.
> > Thanks for all your time reviewing this patch.
> >
> > Here's a revised version in line with the previous discussion.
> >
> > Thanks,
> > Andrew
> >
> > ---
> >
> > If we are configuring for an arc/linux target, and --with-cpu=nps400 is
> > used at configure time then change the default linker emulation to the
> > nps specific version.  All of the alternative linker emulations are
> > still available using the -mNAME option for ld.
> >
> > ld/ChangeLog:
> >
> > 	* configure.tgt (arc*-*-linux*): Change the default linker
> > 	emulation based on --with-cpu selection.
> > 	* NEWS: Mention new configuration option.
> > ---
> >  ld/ChangeLog     |  6 ++++++
> >  ld/NEWS          |  3 +++
> >  ld/configure.tgt | 10 ++++++++--
> >  3 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/ld/NEWS b/ld/NEWS
> > index d43f846..23ca25a 100644
> > --- a/ld/NEWS
> > +++ b/ld/NEWS
> > @@ -2,6 +2,9 @@
> >  
> >  * Add support for the Texas Instruments PRU processor.
> >  
> > +* When configuring for arc*-*-linux* targets the default linker emulation will
> > +  change if --with-cpu=nps400 is used at configure time.
> > +
> >  Changes in 2.28:
> >  
> >  * The EXCLUDE_FILE linker script construct can now be applied outside of the
> > diff --git a/ld/configure.tgt b/ld/configure.tgt
> > index 5a68083..cfb2689 100644
> > --- a/ld/configure.tgt
> > +++ b/ld/configure.tgt
> > @@ -80,8 +80,14 @@ alpha*-*-*vms*)		targ_emul=alphavms
> >  arc*-*-elf*)		targ_emul=arcelf
> >  			targ_extra_emuls="arcelf_prof arclinux arclinux_nps arclinux_prof arcv2elf arcv2elfx"
> >              		;;
> > -arc*-*-linux*)		targ_emul=arclinux
> > -			targ_extra_emuls="arclinux_nps arclinux_prof arcelf arcelf_prof arcv2elf arcv2elfx"
> > +arc*-*-linux*)		if test "${with_cpu}" = "nps400"; then
> > +			   targ_emul=arclinux_nps
> > +			   targ_extra_emuls=arclinux
> > +			else
> > +			   targ_emul=arclinux
> > +			   targ_extra_emuls=arclinux_nps
> > +			fi
> > +			targ_extra_emuls="${targ_extra_emuls} arclinux_prof arcelf arcelf_prof arcv2elf arcv2elfx"
> >  			;;
> >  arm-epoc-pe)		targ_emul=arm_epoc_pe ;	targ_extra_ofiles="deffilep.o pe-dll.o" ;;
> >  arm*-*-cegcc*)		targ_emul=arm_wince_pe ; targ_extra_ofiles="deffilep.o pe-dll.o"
> 
>