Re: [ada, build] host/target configuration

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Wed, 29 May 2013 16:21:38 +0200, Paolo Bonzini <bonzini@gnu.org> wrote:
> Il 29/05/2013 12:50, Thomas Schwinge ha scritto:
> >>> How about we use something like the following [...] patch?  In 
> >>> essence, replace the manual parsing in
> >>> gcc/ada/gcc-interface/Makefile.in by using the values the
> >>> configure script already computed for us.  This is largely
> >>> straightforward (I would hope); please especially review the 
> >>> more involved cases such as the one where I'm now using
> >>> linux%eabi -- for example, does that do the right thing or
> >>> interfere with the Android configurations?

> I agree that using $(target_os) and linux% matches is a better way to
> strip out the "-gnu"s.
> 
> The patch is long, but I think I reviewed it carefully enough.  It's
> okay for mainline.

Thanks for the review.  Before I commit however:

> Regarding the android change:
> 
> > -ifeq ($(strip $(filter-out arm%-linux,$(arch)-$(osys)) $(if $(findstring eabi,$(word 4,$(targ))),,$(word 4,$(targ)))),)
> > +ifeq ($(strip $(filter-out arm% linux%eabi,$(target_cpu) $(target_os))),)
> 
> This is okay, it will match arm-none-linux-androideabi both before and
> after

Well, upon more carful inspection I came to realize that the old/still
current code accepts arm-*-linux-*eabi but also plain arm-*-linux, which
my new code no longer accepts.  Is the old/still current behavior
intentional here, or a bug?  It has been introduced in commit
f49eb158fa5618dbd1130e47281e0bae13798ec6 (r192475):

gcc/ada/
2012-10-15  Matthias Klose  <doko@ubuntu.com>

	* gcc-interface/Makefile.in: Match arm*-*-linux-*eabi* for
	ARM Linux/GNU.

    -ifeq ($(strip $(filter-out arm% linux-gnueabi,$(arch) $(osys)-$(word 4,$(targ)))),)
    +ifeq ($(strip $(filter-out arm%-linux,$(arch)-$(osys)) $(if $(findstring eabi,$(word 4,$(targ))),,$(word 4,$(targ)))),)

From the description, I understand that change to mean any ARM,
Linux-kernel, EABI configuration with any kind of user-land, so my new
interpretation seems correct, and the current code wrong to also accept
plain arm-*-linux.  That change just cite should just have replaced
linux-gnueabi with linux-%eabi?

> (perhaps linux-%eabi would be more readable).

As soon as we've clarified the previous question, I'll do that change.


> However, it seems that the first androideabi snippet was dead code.
> Can you delete it in a follow-up?

That has been added in commit 7a5ee35f18bc945ec8abbcd1377c446352504e6e
(r193238):

2012-11-06  Arnaud Charlet  <charlet@adacore.com>

	[...]
	* gcc-interface/Makefile.in: Add runtime pairs for Android.
	Rework handling of s-oscons.ads.
	* s-osinte-android.ads, s-osinte-android.adb: New files.

    +ifeq ($(strip $(filter-out arm% linux-androideabi,$(arch) $(osys)-$(word 4,$(targ)))),)
    +  LIBGNAT_TARGET_PAIRS = \
    +  [...]
    +  s-osinte.adb<s-osinte-android.adb \
    +  s-osinte.ads<s-osinte-android.ads \
    +  [...]

Indeed I agree that the second snippet in the Makefile.in (as changed by
Matthias on 2012-10-15) triggers anytime the first one (as added by
Arnaud on 2012-11-06) triggered, so Arnaud's LIBGNAT_TARGET_PAIRS setting
will always be overwritten by Matthias', so to speak, and thus the new
s-osinte-android.* files never be used in the current code.  Arnaud,
Matthias, please clarify the intended behavior?  Of course I would assume
that for *-android* configurations the new s-osinte-android.* files be
used, but as Arnaud's change has gone in after Matthias', so this can
never have worked (unless I'm confused)?


GrÃÃe,
 Thomas

Attachment: pgpiI47AGi4Uf.pgp
Description: PGP signature