[Aarch64] Fix multi-arch support in ILP32 mode

Message ID 1513882582.5026.74.camel@cavium.com
State New
Headers show
Series
  • [Aarch64] Fix multi-arch support in ILP32 mode
Related show

Commit Message

Steve Ellcey Dec. 21, 2017, 6:56 p.m.
This one line patch for multi-arch support on Aarch64 and ILP32 was
submitted over a year ago and pinged a number of time since then,
since no one has objected and since it is only one line I am going
to check it in as an obvious fix.

Steve Ellcey
sellcey@cavium.com


2017-12-21  Andrew Pinski  <apinski@cavium.com>
	    Steve Ellcey  <sellcey@cavium.com>

	* config/aarch64/t-aarch64-linux (MULTILIB_OSDIRNAMES): Handle
	multi-arch for ilp32.

Comments

James Greenhalgh Dec. 21, 2017, 8:55 p.m. | #1
On Thu, Dec 21, 2017 at 06:56:22PM +0000, Steve Ellcey wrote:
> This one line patch for multi-arch support on Aarch64 and ILP32 was

> submitted over a year ago and pinged a number of time since then,

> since no one has objected and since it is only one line I am going

> to check it in as an obvious fix.


I think this is not obvious, and worse incorrect. I contacted Andrew
off-list back in September to ask about this patch, the problem is the
triplet used here:

> -MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32

> +MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32$(call if_multiarch,:aarch64$(AARCH_BE)_ilp32-linux-gnu)


As far as I understand it from Linaro connect and conversations with
Debian/Ubuntu port maintainers, the correct triplet should be:

  aarch64-none-linux-gnu_ilp32

If you're revisiting this patch, it is OK to go in, but please use the
agreed triplet.

Thanks,
James
Steve Ellcey Dec. 21, 2017, 9:59 p.m. | #2
On Thu, 2017-12-21 at 20:55 +0000, James Greenhalgh wrote:
> On Thu, Dec 21, 2017 at 06:56:22PM +0000, Steve Ellcey wrote:

> > 

> > This one line patch for multi-arch support on Aarch64 and ILP32 was

> > submitted over a year ago and pinged a number of time since then,

> > since no one has objected and since it is only one line I am going

> > to check it in as an obvious fix.

> I think this is not obvious, and worse incorrect. I contacted Andrew

> off-list back in September to ask about this patch, the problem is

> the

> triplet used here:

> 

> > 

> > -MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32

> > +MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32$(call

> > if_multiarch,:aarch64$(AARCH_BE)_ilp32-linux-gnu)

> As far as I understand it from Linaro connect and conversations with

> Debian/Ubuntu port maintainers, the correct triplet should be:

> 

>   aarch64-none-linux-gnu_ilp32

> 

> If you're revisiting this patch, it is OK to go in, but please use the

> agreed triplet.

> 

> Thanks,

> James


OK, I fixed the triplet and checked it in.

Steve Ellcey
Matthias Klose Dec. 21, 2017, 10:54 p.m. | #3
On 21.12.2017 22:59, Steve Ellcey wrote:
> On Thu, 2017-12-21 at 20:55 +0000, James Greenhalgh wrote:

>> On Thu, Dec 21, 2017 at 06:56:22PM +0000, Steve Ellcey wrote:

>>>

>>> This one line patch for multi-arch support on Aarch64 and ILP32 was

>>> submitted over a year ago and pinged a number of time since then,

>>> since no one has objected and since it is only one line I am going

>>> to check it in as an obvious fix.

>> I think this is not obvious, and worse incorrect. I contacted Andrew

>> off-list back in September to ask about this patch, the problem is

>> the

>> triplet used here:

>>

>>>

>>> -MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32

>>> +MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32$(call

>>> if_multiarch,:aarch64$(AARCH_BE)_ilp32-linux-gnu)

>> As far as I understand it from Linaro connect and conversations with

>> Debian/Ubuntu port maintainers, the correct triplet should be:

>>

>>   aarch64-none-linux-gnu_ilp32

>>

>> If you're revisiting this patch, it is OK to go in, but please use the

>> agreed triplet.


well, it's called triplet, not quadruplet. No other multiarch tuple contains an
explicit vendor field. Do you really want to use that?

>> Thanks,

>> James

> 

> OK, I fixed the triplet and checked it in.


hmm ...

Matthias
Steve Ellcey Dec. 21, 2017, 11:18 p.m. | #4
On Thu, 2017-12-21 at 23:54 +0100, Matthias Klose wrote:
> On 21.12.2017 22:59, Steve Ellcey wrote:

> > 

> > > As far as I understand it from Linaro connect and conversations with

> > > Debian/Ubuntu port maintainers, the correct triplet should be:

> > > 

> > >   aarch64-none-linux-gnu_ilp32

> > > 

> > > If you're revisiting this patch, it is OK to go in, but please use the

> > > agreed triplet.

> well, it's called triplet, not quadruplet. No other multiarch tuple contains an

> explicit vendor field. Do you really want to use that?

> > > Thanks,

> > > James

> > OK, I fixed the triplet and checked it in.

> hmm ...

> 

> Matthias


Actually, I got so focused on moving where the _ilp32 was I forgot
about the '-none-' part so what I actually checked in was:

MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32$(call if_multiarch,:aarch64$(AARCH_BE)-linux-gnu_ilp32)

This seems to match the dynamic linker name in gcc and glibc:

gcc/gcc/config/aarch64/aarch64-linux.h:
#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux-aarch64%{mbig-endian:_be}%{mabi=ilp32:_ilp32}.so.1"

This doesn't have a '-none-' in it but does have the _ilp32 at the end.

Steve Ellcey
sellcey@cavium.com

Patch

diff --git a/gcc/config/aarch64/t-aarch64-linux b/gcc/config/aarch64/t-aarch64-linux
index ab064ab..348fcda 100644
--- a/gcc/config/aarch64/t-aarch64-linux
+++ b/gcc/config/aarch64/t-aarch64-linux
@@ -25,4 +25,4 @@  AARCH_BE = $(if $(findstring TARGET_BIG_ENDIAN_DEFAULT=1, $(tm_defines)),_be)
 MULTILIB_OSDIRNAMES = mabi.lp64=../lib64$(call if_multiarch,:aarch64$(AARCH_BE)-linux-gnu)
 MULTIARCH_DIRNAME = $(call if_multiarch,aarch64$(AARCH_BE)-linux-gnu)
 
-MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32
+MULTILIB_OSDIRNAMES += mabi.ilp32=../libilp32$(call if_multiarch,:aarch64$(AARCH_BE)_ilp32-linux-gnu)