Re: powerpc*le-linux support

[Gary V. Vaughan]

On Aug 22, 2013, at 2:59 PM, Peter Rosin <address@hidden> wrote:
> Hi!
> 
> Ok, Gary pushed something while I wrote this, but I'm sending it
> anyway since what he pushed didn't look quite right to me…

Argh.  I pasted the patch from my browser to a terminal to save hunting
my archives for the original email, and mangled the hunks during application.
I pushed a fix too, so that the last two commits do match the original
patch.

> On 2013-06-06 07:18, Alan Modra wrote:
>> On Thu, Jun 06, 2013 at 11:31:34AM +0930, Alan Modra wrote:
>>> This adds support for little-endian powerpc linux, and tidies the
>>> existing host match for powerpc.  config.sub won't return ppc*-*linux*
>>> so there isn't much point in matching that.
>> 
>>> -     ppc*-*linux*|powerpc*-*linux*)
>>> +     powerpcle*)
>>> +       LD="${LD-ld} -m elf64lppc"
>>> +       ;;
>>> +     powerpc*)
>>>         LD="${LD-ld} -m elf64ppc"
>>>         ;;
>> 
>> I didn't get that quite right.  'powerpc*' in the above matches too
>> much, for example when your host is powerpc64-linux and target
>> powerpc64le-linux you'll get -melf64ppc added to LD.  Since
>> powerpc64le-linux-ld wants -melf64lppc (or nothing) that will fail.
>> Revised as follows.
>> 
>>      * m4/libtool.m4 (ld -m flags): Remove non-canonical ppc host match.
> 
> The macro/function you are changing is _LT_ENABLE_LOCK, so that should be
> 
>       * m4/libtool.m4 (_LT_ENABLE_LOCK): …

Agreed.  I should have checked and changed that on commit, my bad.

>>      Support little-endian powerpc linux host.
>> 
>> diff --git a/m4/libtool.m4 b/m4/libtool.m4
>> index d7013c5..501246d 100644
>> --- a/m4/libtool.m4
>> +++ b/m4/libtool.m4
>> @@ -1307,7 +1307,7 @@ ia64-*-hpux*)
>>   rm -rf conftest*
>>   ;;
>> 
>> -x86_64-*kfreebsd*-gnu|x86_64-*linux*|ppc*-*linux*|powerpc*-*linux*| \
>> +x86_64-*kfreebsd*-gnu|x86_64-*linux*|powerpc*-*linux*| \
>> s390*-*linux*|s390*-*tpf*|sparc*-*linux*)
>>   # Find out which ABI we are using.
>>   echo 'int i;' > conftest.$ac_ext
>> @@ -1328,7 +1328,10 @@ s390*-*linux*|s390*-*tpf*|sparc*-*linux*)
>>              ;;
>>          esac
>>          ;;
>> -      ppc64-*linux*|powerpc64-*linux*)
>> +      powerpc64le-*)
>> +        LD="${LD-ld} -m elf32lppclinux"
>> +        ;;
>> +      powerpc64-*)
>>          LD="${LD-ld} -m elf32ppclinux"
>>          ;;
>>        s390x-*linux*)
> 
> All other inner cases match one of the outer or-ed expressions (i.e.
> from the first hunk) quite closely, but the outer match is still
> powerpc*-*linux* while the inner match has dropped the trailing
> -*linux* part. I would have kept them in sync.

The other branches are inconsistent here too, so either is fine IMHO.

> This also made me think
> about the 32-bit case; is there no le variant for 32-bit powerpc?
> Compare with the x86_64 case just above this hunk. To me, it seems as
> if -m elf32lppclinux should be added to $LD at least in some case?

That's only because I fumbled the initial commit.  The original patch
catches ppcle on 32 and 64 bit legs of that case statement.

> When you build 32-bit output and $host is 64-bit, you need to specify
> endianess (elf32lppclinux or elf32ppclinux). When you build 64-bit
> output and $host is 64-bit, you need to specify endianess (elf64lppc
> or elf64ppc). I miss the case when you build 32-bit output and $host
> is 32-bit, i.e. something like the below (assuming $host is
> powerpcle-* and powerpc-* for 32-bit) at the end of the second hunk:
> 
> +       powerpcle-*linux*)
> +         LD="${LD-ld} -m elf32lppclinux"
> +         ;;
> +       powerpc-*linux*)
> +         LD="${LD-ld} -m elf32ppclinux"
> +         ;;
> 
> If there is no 32-bit le powerpc variant (why wouldn't there be?), then
> the subject is somewhat misleading when le is only handled for 64-bit
> hosts.
> 
>> @@ -1347,7 +1350,10 @@ s390*-*linux*|s390*-*tpf*|sparc*-*linux*)
>>        x86_64-*linux*)
>>          LD="${LD-ld} -m elf_x86_64"
>>          ;;
>> -      ppc*-*linux*|powerpc*-*linux*)
>> +      powerpcle-*)
>> +        LD="${LD-ld} -m elf64lppc"
>> +        ;;
>> +      powerpc-*)
>>          LD="${LD-ld} -m elf64ppc"
>>          ;;
>>        s390*-*linux*|s390*-*tpf*)
>> 
>> 
> 
> Or what am I not getting? I'm probably just ignorant...ö

Without access to ppc linux compilers and cross compilers, I only ran
the test suite on my Mac before committing, which doesn't exercise any
of the changes, but is at least a sanity check against syntax errors or
similar breakage.

Alan, your patch is already a step forward in providing the support you
need without breaking anything else, so I'm happy to have committed it
finally.  Peter, your point about cross compiling between the 32bit and
64bit APIs is valid too, so I'd be even happier to apply a follow-up that
handles those cases too.

Unfortunately, we still need to figure out a plan to handle the patch
backlog problem that left the original in limbo for so long.

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)

signature.asc
Description: Message signed with OpenPGP using GPGMail