diff mbox

[v2] ARM: Use different linker path for hardfloat ABI

Message ID 4F9EAE9D.3040509@arm.com
State New
Headers show

Commit Message

Richard Earnshaw April 30, 2012, 3:24 p.m. UTC
On 27/04/12 00:27, Michael Hope wrote:
> On 27 April 2012 08:20, Carlos O'Donell <carlos@systemhalted.org> wrote:
>> On Mon, Apr 23, 2012 at 5:36 PM, Michael Hope <michael.hope@linaro.org> wrote:
>>> 2012-04-24  Michael Hope  <michael.hope@linaro.org>
>>>            Richard Earnshaw  <rearnsha@arm.com>
>>>
>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER_SOFT_FLOAT): Define.
>>>        (GLIBC_DYNAMIC_LINKER_HARD_FLOAT): Define.
>>>        (GLIBC_DYNAMIC_LINKER_DEFAULT): Define.
>>>        (GLIBC_DYNAMIC_LINKER): Redefine to use the hard float path.
>>>
>>> diff --git a/gcc/config/arm/linux-eabi.h b/gcc/config/arm/linux-eabi.h
>>> index 80bd825..2ace6f0 100644
>>> --- a/gcc/config/arm/linux-eabi.h
>>> +++ b/gcc/config/arm/linux-eabi.h
>>> @@ -62,7 +62,17 @@
>>>  /* Use ld-linux.so.3 so that it will be possible to run "classic"
>>>    GNU/Linux binaries on an EABI system.  */
>>>  #undef  GLIBC_DYNAMIC_LINKER
>>> -#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.3"
>>> +#define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "/lib/ld-linux.so.3"
>>> +#define GLIBC_DYNAMIC_LINKER_HARD_FLOAT "/lib/ld-linux-armhf.so.3"
>>> +#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>> +#else
>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
>>> +#endif
>>> +#define GLIBC_DYNAMIC_LINKER \
>>> +   "%{mfloat-abi=hard:" GLIBC_DYNAMIC_LINKER_HARD_FLOAT "} \
>>> +    %{mfloat-abi=soft*:" GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "} \
>>> +    %{!mfloat-abi=*:" GLIBC_DYNAMIC_LINKER_DEFAULT "}"
>>>
>>>  /* At this point, bpabi.h will have clobbered LINK_SPEC.  We want to
>>>    use the GNU/Linux version, not the generic BPABI version.  */
>>
>> This patch is broken. Please fix this.
>>
>> You can't use a named enumeration in cpp equality.
>>
>> The type ARM_FLOAT_ABI_HARD is a named enumeration and evaluates to 0
>> as an unknown identifier.
>>
>> Therefore "#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD"
>> evaluates to "#if 0 == 0" and is always true.
>>
>> Watch out that "#define ARM_FLOAT_ABI_HARD ARM_FLOAT_ABI_HARD" for
>> such enums is not conforming C99/C11.
>>
>> I suggest you define the types as macros and then set the named enum
>> to those values, then use the macros in the header equality checks.
>>
>> e.g.
>> #define VAL1 0 then enum FOO { RVAL1 = VAL1, ... }
>>
>> Look at arm.h for the enum definition.
> 
> I've looked further into this and I think the original pre-#if version
> is correct.
> 
> The float ABI comes from these places:
>  * The -mfloat-abi= command line argument, else
>  * The --with-float= configure time argument, else
>  * TARGET_DEFAULT_FLOAT_ABI from linux-eabi.h
> 
> In the first case the ABI is explicit.  In the second
> OPTION_DEFAULT_SPECS turns the configure time argument into an explict
> -mfloat-abi=.
> 
> The patch below covers all cases, keeps the logic in the spec file,
> and adds a comment linking the two #defines.
> 
> Tested by building with no configure flags, --wtih-float=softfp,
> --with-float=hard, and then running with all combinations of
> {,-mfloat-abi=softfp,-mfloat-abi=hard} {,-mglibc,-muclibc,-mbionic}.
> 
> OK?
> 
> -- Michael
> 
> 2012-04-27  Michael Hope  <michael.hope@linaro.org>
> 
> 	* config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
> 	using a spec rule.
> 

Michael,

can you try this patch please.  It should make it possible to then
create linux-eabihf.h containing just

#undef TARGET_DEFAULT_FLOAT_ABI
#define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
#undef GLIBC_DYNAMIC_LINKER_DEFAULT
#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT

Which is not quite as simple as leaving out the second re-define, but
pretty close.

R.

Comments

Michael Hope April 30, 2012, 9:47 p.m. UTC | #1
On 1 May 2012 03:24, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 27/04/12 00:27, Michael Hope wrote:
>> On 27 April 2012 08:20, Carlos O'Donell <carlos@systemhalted.org> wrote:
>>> On Mon, Apr 23, 2012 at 5:36 PM, Michael Hope <michael.hope@linaro.org> wrote:
>>>> 2012-04-24  Michael Hope  <michael.hope@linaro.org>
>>>>            Richard Earnshaw  <rearnsha@arm.com>
>>>>
>>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER_SOFT_FLOAT): Define.
>>>>        (GLIBC_DYNAMIC_LINKER_HARD_FLOAT): Define.
>>>>        (GLIBC_DYNAMIC_LINKER_DEFAULT): Define.
>>>>        (GLIBC_DYNAMIC_LINKER): Redefine to use the hard float path.
>>>>
>>>> diff --git a/gcc/config/arm/linux-eabi.h b/gcc/config/arm/linux-eabi.h
>>>> index 80bd825..2ace6f0 100644
>>>> --- a/gcc/config/arm/linux-eabi.h
>>>> +++ b/gcc/config/arm/linux-eabi.h
>>>> @@ -62,7 +62,17 @@
>>>>  /* Use ld-linux.so.3 so that it will be possible to run "classic"
>>>>    GNU/Linux binaries on an EABI system.  */
>>>>  #undef  GLIBC_DYNAMIC_LINKER
>>>> -#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.3"
>>>> +#define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "/lib/ld-linux.so.3"
>>>> +#define GLIBC_DYNAMIC_LINKER_HARD_FLOAT "/lib/ld-linux-armhf.so.3"
>>>> +#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>> +#else
>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
>>>> +#endif
>>>> +#define GLIBC_DYNAMIC_LINKER \
>>>> +   "%{mfloat-abi=hard:" GLIBC_DYNAMIC_LINKER_HARD_FLOAT "} \
>>>> +    %{mfloat-abi=soft*:" GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "} \
>>>> +    %{!mfloat-abi=*:" GLIBC_DYNAMIC_LINKER_DEFAULT "}"
>>>>
>>>>  /* At this point, bpabi.h will have clobbered LINK_SPEC.  We want to
>>>>    use the GNU/Linux version, not the generic BPABI version.  */
>>>
>>> This patch is broken. Please fix this.
>>>
>>> You can't use a named enumeration in cpp equality.
>>>
>>> The type ARM_FLOAT_ABI_HARD is a named enumeration and evaluates to 0
>>> as an unknown identifier.
>>>
>>> Therefore "#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD"
>>> evaluates to "#if 0 == 0" and is always true.
>>>
>>> Watch out that "#define ARM_FLOAT_ABI_HARD ARM_FLOAT_ABI_HARD" for
>>> such enums is not conforming C99/C11.
>>>
>>> I suggest you define the types as macros and then set the named enum
>>> to those values, then use the macros in the header equality checks.
>>>
>>> e.g.
>>> #define VAL1 0 then enum FOO { RVAL1 = VAL1, ... }
>>>
>>> Look at arm.h for the enum definition.
>>
>> I've looked further into this and I think the original pre-#if version
>> is correct.
>>
>> The float ABI comes from these places:
>>  * The -mfloat-abi= command line argument, else
>>  * The --with-float= configure time argument, else
>>  * TARGET_DEFAULT_FLOAT_ABI from linux-eabi.h
>>
>> In the first case the ABI is explicit.  In the second
>> OPTION_DEFAULT_SPECS turns the configure time argument into an explict
>> -mfloat-abi=.
>>
>> The patch below covers all cases, keeps the logic in the spec file,
>> and adds a comment linking the two #defines.
>>
>> Tested by building with no configure flags, --wtih-float=softfp,
>> --with-float=hard, and then running with all combinations of
>> {,-mfloat-abi=softfp,-mfloat-abi=hard} {,-mglibc,-muclibc,-mbionic}.
>>
>> OK?
>>
>> -- Michael
>>
>> 2012-04-27  Michael Hope  <michael.hope@linaro.org>
>>
>>       * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>       using a spec rule.
>>
>
> Michael,
>
> can you try this patch please.  It should make it possible to then
> create linux-eabihf.h containing just
>
> #undef TARGET_DEFAULT_FLOAT_ABI
> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>
> Which is not quite as simple as leaving out the second re-define, but
> pretty close.

Hi Richard.  Your patch tests just fine.  I like it.  You could change
the spec rule to the newer if-elseif-else form but that's a nit.

-- Michael
Jeff Law April 30, 2012, 10:01 p.m. UTC | #2
On 04/30/2012 03:47 PM, Michael Hope wrote:

>>>
>>> 2012-04-27  Michael Hope<michael.hope@linaro.org>
>>>
>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>>        using a spec rule.
>>>
>>
>> Michael,
>>
>> can you try this patch please.  It should make it possible to then
>> create linux-eabihf.h containing just
>>
>> #undef TARGET_DEFAULT_FLOAT_ABI
>> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
>> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
>> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>
>> Which is not quite as simple as leaving out the second re-define, but
>> pretty close.
>
> Hi Richard.  Your patch tests just fine.  I like it.  You could change
> the spec rule to the newer if-elseif-else form but that's a nit.
So who owns creating the appropriate glibc patch now that we've got a 
good gcc patch?

jeff
Michael Hope May 1, 2012, 2:43 a.m. UTC | #3
On 1 May 2012 10:01, Jeff Law <law@redhat.com> wrote:
> On 04/30/2012 03:47 PM, Michael Hope wrote:
>
>>>>
>>>> 2012-04-27  Michael Hope<michael.hope@linaro.org>
>>>>
>>>>       * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>>>       using a spec rule.
>>>>
>>>
>>> Michael,
>>>
>>> can you try this patch please.  It should make it possible to then
>>> create linux-eabihf.h containing just
>>>
>>> #undef TARGET_DEFAULT_FLOAT_ABI
>>> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
>>> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
>>> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>
>>> Which is not quite as simple as leaving out the second re-define, but
>>> pretty close.
>>
>>
>> Hi Richard.  Your patch tests just fine.  I like it.  You could change
>> the spec rule to the newer if-elseif-else form but that's a nit.
>
> So who owns creating the appropriate glibc patch now that we've got a good
> gcc patch?

Carlos is working on the GLIBC patch:
 http://sourceware.org/ml/libc-ports/2012-04/msg00171.html

-- Michael
Jeff Law May 1, 2012, 3:44 a.m. UTC | #4
On 04/30/2012 08:43 PM, Michael Hope wrote:
> On 1 May 2012 10:01, Jeff Law<law@redhat.com>  wrote:
>> On 04/30/2012 03:47 PM, Michael Hope wrote:
>>
>>>>>
>>>>> 2012-04-27  Michael Hope<michael.hope@linaro.org>
>>>>>
>>>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>>>>        using a spec rule.
>>>>>
>>>>
>>>> Michael,
>>>>
>>>> can you try this patch please.  It should make it possible to then
>>>> create linux-eabihf.h containing just
>>>>
>>>> #undef TARGET_DEFAULT_FLOAT_ABI
>>>> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
>>>> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
>>>> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>>
>>>> Which is not quite as simple as leaving out the second re-define, but
>>>> pretty close.
>>>
>>>
>>> Hi Richard.  Your patch tests just fine.  I like it.  You could change
>>> the spec rule to the newer if-elseif-else form but that's a nit.
>>
>> So who owns creating the appropriate glibc patch now that we've got a good
>> gcc patch?
>
> Carlos is working on the GLIBC patch:
>   http://sourceware.org/ml/libc-ports/2012-04/msg00171.html
Ah, not on the main glibc list...  That's why I missed it.  Thanks.

jeff
Richard Earnshaw May 1, 2012, 8:52 a.m. UTC | #5
On 30/04/12 22:47, Michael Hope wrote:
> On 1 May 2012 03:24, Richard Earnshaw <rearnsha@arm.com> wrote:
>> On 27/04/12 00:27, Michael Hope wrote:
>>> On 27 April 2012 08:20, Carlos O'Donell <carlos@systemhalted.org> wrote:
>>>> On Mon, Apr 23, 2012 at 5:36 PM, Michael Hope <michael.hope@linaro.org> wrote:
>>>>> 2012-04-24  Michael Hope  <michael.hope@linaro.org>
>>>>>            Richard Earnshaw  <rearnsha@arm.com>
>>>>>
>>>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER_SOFT_FLOAT): Define.
>>>>>        (GLIBC_DYNAMIC_LINKER_HARD_FLOAT): Define.
>>>>>        (GLIBC_DYNAMIC_LINKER_DEFAULT): Define.
>>>>>        (GLIBC_DYNAMIC_LINKER): Redefine to use the hard float path.
>>>>>
>>>>> diff --git a/gcc/config/arm/linux-eabi.h b/gcc/config/arm/linux-eabi.h
>>>>> index 80bd825..2ace6f0 100644
>>>>> --- a/gcc/config/arm/linux-eabi.h
>>>>> +++ b/gcc/config/arm/linux-eabi.h
>>>>> @@ -62,7 +62,17 @@
>>>>>  /* Use ld-linux.so.3 so that it will be possible to run "classic"
>>>>>    GNU/Linux binaries on an EABI system.  */
>>>>>  #undef  GLIBC_DYNAMIC_LINKER
>>>>> -#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.3"
>>>>> +#define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "/lib/ld-linux.so.3"
>>>>> +#define GLIBC_DYNAMIC_LINKER_HARD_FLOAT "/lib/ld-linux-armhf.so.3"
>>>>> +#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
>>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>>> +#else
>>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
>>>>> +#endif
>>>>> +#define GLIBC_DYNAMIC_LINKER \
>>>>> +   "%{mfloat-abi=hard:" GLIBC_DYNAMIC_LINKER_HARD_FLOAT "} \
>>>>> +    %{mfloat-abi=soft*:" GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "} \
>>>>> +    %{!mfloat-abi=*:" GLIBC_DYNAMIC_LINKER_DEFAULT "}"
>>>>>
>>>>>  /* At this point, bpabi.h will have clobbered LINK_SPEC.  We want to
>>>>>    use the GNU/Linux version, not the generic BPABI version.  */
>>>>
>>>> This patch is broken. Please fix this.
>>>>
>>>> You can't use a named enumeration in cpp equality.
>>>>
>>>> The type ARM_FLOAT_ABI_HARD is a named enumeration and evaluates to 0
>>>> as an unknown identifier.
>>>>
>>>> Therefore "#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD"
>>>> evaluates to "#if 0 == 0" and is always true.
>>>>
>>>> Watch out that "#define ARM_FLOAT_ABI_HARD ARM_FLOAT_ABI_HARD" for
>>>> such enums is not conforming C99/C11.
>>>>
>>>> I suggest you define the types as macros and then set the named enum
>>>> to those values, then use the macros in the header equality checks.
>>>>
>>>> e.g.
>>>> #define VAL1 0 then enum FOO { RVAL1 = VAL1, ... }
>>>>
>>>> Look at arm.h for the enum definition.
>>>
>>> I've looked further into this and I think the original pre-#if version
>>> is correct.
>>>
>>> The float ABI comes from these places:
>>>  * The -mfloat-abi= command line argument, else
>>>  * The --with-float= configure time argument, else
>>>  * TARGET_DEFAULT_FLOAT_ABI from linux-eabi.h
>>>
>>> In the first case the ABI is explicit.  In the second
>>> OPTION_DEFAULT_SPECS turns the configure time argument into an explict
>>> -mfloat-abi=.
>>>
>>> The patch below covers all cases, keeps the logic in the spec file,
>>> and adds a comment linking the two #defines.
>>>
>>> Tested by building with no configure flags, --wtih-float=softfp,
>>> --with-float=hard, and then running with all combinations of
>>> {,-mfloat-abi=softfp,-mfloat-abi=hard} {,-mglibc,-muclibc,-mbionic}.
>>>
>>> OK?
>>>
>>> -- Michael
>>>
>>> 2012-04-27  Michael Hope  <michael.hope@linaro.org>
>>>
>>>       * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>>       using a spec rule.
>>>
>>
>> Michael,
>>
>> can you try this patch please.  It should make it possible to then
>> create linux-eabihf.h containing just
>>
>> #undef TARGET_DEFAULT_FLOAT_ABI
>> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
>> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
>> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>
>> Which is not quite as simple as leaving out the second re-define, but
>> pretty close.
> 
> Hi Richard.  Your patch tests just fine.  I like it.  You could change
> the spec rule to the newer if-elseif-else form but that's a nit.
> 
> -- Michael
> 

Great, thanks!  I've committed it as is.

R.
Andreas Jaeger May 23, 2012, 7:12 a.m. UTC | #6
On 05/01/2012 10:52 AM, Richard Earnshaw wrote:
> On 30/04/12 22:47, Michael Hope wrote:
>> On 1 May 2012 03:24, Richard Earnshaw<rearnsha@arm.com>  wrote:
>>> On 27/04/12 00:27, Michael Hope wrote:
>>>> On 27 April 2012 08:20, Carlos O'Donell<carlos@systemhalted.org>  wrote:
>>>>> On Mon, Apr 23, 2012 at 5:36 PM, Michael Hope<michael.hope@linaro.org>  wrote:
>>>>>> 2012-04-24  Michael Hope<michael.hope@linaro.org>
>>>>>>             Richard Earnshaw<rearnsha@arm.com>
>>>>>>
>>>>>>         * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER_SOFT_FLOAT): Define.
>>>>>>         (GLIBC_DYNAMIC_LINKER_HARD_FLOAT): Define.
>>>>>>         (GLIBC_DYNAMIC_LINKER_DEFAULT): Define.
>>>>>>         (GLIBC_DYNAMIC_LINKER): Redefine to use the hard float path.
>>>>>>
>>>>>> diff --git a/gcc/config/arm/linux-eabi.h b/gcc/config/arm/linux-eabi.h
>>>>>> index 80bd825..2ace6f0 100644
>>>>>> --- a/gcc/config/arm/linux-eabi.h
>>>>>> +++ b/gcc/config/arm/linux-eabi.h
>>>>>> @@ -62,7 +62,17 @@
>>>>>>   /* Use ld-linux.so.3 so that it will be possible to run "classic"
>>>>>>     GNU/Linux binaries on an EABI system.  */
>>>>>>   #undef  GLIBC_DYNAMIC_LINKER
>>>>>> -#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.3"
>>>>>> +#define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "/lib/ld-linux.so.3"
>>>>>> +#define GLIBC_DYNAMIC_LINKER_HARD_FLOAT "/lib/ld-linux-armhf.so.3"
>>>>>> +#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
>>>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>>>> +#else
>>>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
>>>>>> +#endif
>>>>>> +#define GLIBC_DYNAMIC_LINKER \
>>>>>> +   "%{mfloat-abi=hard:" GLIBC_DYNAMIC_LINKER_HARD_FLOAT "} \
>>>>>> +    %{mfloat-abi=soft*:" GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "} \
>>>>>> +    %{!mfloat-abi=*:" GLIBC_DYNAMIC_LINKER_DEFAULT "}"
>>>>>>
>>>>>>   /* At this point, bpabi.h will have clobbered LINK_SPEC.  We want to
>>>>>>     use the GNU/Linux version, not the generic BPABI version.  */
>>>>>
>>>>> This patch is broken. Please fix this.
>>>>>
>>>>> You can't use a named enumeration in cpp equality.
>>>>>
>>>>> The type ARM_FLOAT_ABI_HARD is a named enumeration and evaluates to 0
>>>>> as an unknown identifier.
>>>>>
>>>>> Therefore "#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD"
>>>>> evaluates to "#if 0 == 0" and is always true.
>>>>>
>>>>> Watch out that "#define ARM_FLOAT_ABI_HARD ARM_FLOAT_ABI_HARD" for
>>>>> such enums is not conforming C99/C11.
>>>>>
>>>>> I suggest you define the types as macros and then set the named enum
>>>>> to those values, then use the macros in the header equality checks.
>>>>>
>>>>> e.g.
>>>>> #define VAL1 0 then enum FOO { RVAL1 = VAL1, ... }
>>>>>
>>>>> Look at arm.h for the enum definition.
>>>>
>>>> I've looked further into this and I think the original pre-#if version
>>>> is correct.
>>>>
>>>> The float ABI comes from these places:
>>>>   * The -mfloat-abi= command line argument, else
>>>>   * The --with-float= configure time argument, else
>>>>   * TARGET_DEFAULT_FLOAT_ABI from linux-eabi.h
>>>>
>>>> In the first case the ABI is explicit.  In the second
>>>> OPTION_DEFAULT_SPECS turns the configure time argument into an explict
>>>> -mfloat-abi=.
>>>>
>>>> The patch below covers all cases, keeps the logic in the spec file,
>>>> and adds a comment linking the two #defines.
>>>>
>>>> Tested by building with no configure flags, --wtih-float=softfp,
>>>> --with-float=hard, and then running with all combinations of
>>>> {,-mfloat-abi=softfp,-mfloat-abi=hard} {,-mglibc,-muclibc,-mbionic}.
>>>>
>>>> OK?
>>>>
>>>> -- Michael
>>>>
>>>> 2012-04-27  Michael Hope<michael.hope@linaro.org>
>>>>
>>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>>>        using a spec rule.
>>>>
>>>
>>> Michael,
>>>
>>> can you try this patch please.  It should make it possible to then
>>> create linux-eabihf.h containing just
>>>
>>> #undef TARGET_DEFAULT_FLOAT_ABI
>>> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
>>> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
>>> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>
>>> Which is not quite as simple as leaving out the second re-define, but
>>> pretty close.
>>
>> Hi Richard.  Your patch tests just fine.  I like it.  You could change
>> the spec rule to the newer if-elseif-else form but that's a nit.
>>
>> -- Michael
>>
>
> Great, thanks!  I've committed it as is.

I suggest to add this also to the gcc 4.7 branch. Richard (Earnshaw), 
could you do take care of this, please?

Thanks,
Andreas
Richard Earnshaw May 23, 2012, 7:56 a.m. UTC | #7
On 23/05/12 08:12, Andreas Jaeger wrote:
> On 05/01/2012 10:52 AM, Richard Earnshaw wrote:
>> On 30/04/12 22:47, Michael Hope wrote:
>>> On 1 May 2012 03:24, Richard Earnshaw<rearnsha@arm.com>  wrote:
>>>> On 27/04/12 00:27, Michael Hope wrote:
>>>>> On 27 April 2012 08:20, Carlos O'Donell<carlos@systemhalted.org>  wrote:
>>>>>> On Mon, Apr 23, 2012 at 5:36 PM, Michael Hope<michael.hope@linaro.org>  wrote:
>>>>>>> 2012-04-24  Michael Hope<michael.hope@linaro.org>
>>>>>>>             Richard Earnshaw<rearnsha@arm.com>
>>>>>>>
>>>>>>>         * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER_SOFT_FLOAT): Define.
>>>>>>>         (GLIBC_DYNAMIC_LINKER_HARD_FLOAT): Define.
>>>>>>>         (GLIBC_DYNAMIC_LINKER_DEFAULT): Define.
>>>>>>>         (GLIBC_DYNAMIC_LINKER): Redefine to use the hard float path.
>>>>>>>
>>>>>>> diff --git a/gcc/config/arm/linux-eabi.h b/gcc/config/arm/linux-eabi.h
>>>>>>> index 80bd825..2ace6f0 100644
>>>>>>> --- a/gcc/config/arm/linux-eabi.h
>>>>>>> +++ b/gcc/config/arm/linux-eabi.h
>>>>>>> @@ -62,7 +62,17 @@
>>>>>>>   /* Use ld-linux.so.3 so that it will be possible to run "classic"
>>>>>>>     GNU/Linux binaries on an EABI system.  */
>>>>>>>   #undef  GLIBC_DYNAMIC_LINKER
>>>>>>> -#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.3"
>>>>>>> +#define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "/lib/ld-linux.so.3"
>>>>>>> +#define GLIBC_DYNAMIC_LINKER_HARD_FLOAT "/lib/ld-linux-armhf.so.3"
>>>>>>> +#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
>>>>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>>>>> +#else
>>>>>>> +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
>>>>>>> +#endif
>>>>>>> +#define GLIBC_DYNAMIC_LINKER \
>>>>>>> +   "%{mfloat-abi=hard:" GLIBC_DYNAMIC_LINKER_HARD_FLOAT "} \
>>>>>>> +    %{mfloat-abi=soft*:" GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "} \
>>>>>>> +    %{!mfloat-abi=*:" GLIBC_DYNAMIC_LINKER_DEFAULT "}"
>>>>>>>
>>>>>>>   /* At this point, bpabi.h will have clobbered LINK_SPEC.  We want to
>>>>>>>     use the GNU/Linux version, not the generic BPABI version.  */
>>>>>>
>>>>>> This patch is broken. Please fix this.
>>>>>>
>>>>>> You can't use a named enumeration in cpp equality.
>>>>>>
>>>>>> The type ARM_FLOAT_ABI_HARD is a named enumeration and evaluates to 0
>>>>>> as an unknown identifier.
>>>>>>
>>>>>> Therefore "#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD"
>>>>>> evaluates to "#if 0 == 0" and is always true.
>>>>>>
>>>>>> Watch out that "#define ARM_FLOAT_ABI_HARD ARM_FLOAT_ABI_HARD" for
>>>>>> such enums is not conforming C99/C11.
>>>>>>
>>>>>> I suggest you define the types as macros and then set the named enum
>>>>>> to those values, then use the macros in the header equality checks.
>>>>>>
>>>>>> e.g.
>>>>>> #define VAL1 0 then enum FOO { RVAL1 = VAL1, ... }
>>>>>>
>>>>>> Look at arm.h for the enum definition.
>>>>>
>>>>> I've looked further into this and I think the original pre-#if version
>>>>> is correct.
>>>>>
>>>>> The float ABI comes from these places:
>>>>>   * The -mfloat-abi= command line argument, else
>>>>>   * The --with-float= configure time argument, else
>>>>>   * TARGET_DEFAULT_FLOAT_ABI from linux-eabi.h
>>>>>
>>>>> In the first case the ABI is explicit.  In the second
>>>>> OPTION_DEFAULT_SPECS turns the configure time argument into an explict
>>>>> -mfloat-abi=.
>>>>>
>>>>> The patch below covers all cases, keeps the logic in the spec file,
>>>>> and adds a comment linking the two #defines.
>>>>>
>>>>> Tested by building with no configure flags, --wtih-float=softfp,
>>>>> --with-float=hard, and then running with all combinations of
>>>>> {,-mfloat-abi=softfp,-mfloat-abi=hard} {,-mglibc,-muclibc,-mbionic}.
>>>>>
>>>>> OK?
>>>>>
>>>>> -- Michael
>>>>>
>>>>> 2012-04-27  Michael Hope<michael.hope@linaro.org>
>>>>>
>>>>>        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Pick the loader
>>>>>        using a spec rule.
>>>>>
>>>>
>>>> Michael,
>>>>
>>>> can you try this patch please.  It should make it possible to then
>>>> create linux-eabihf.h containing just
>>>>
>>>> #undef TARGET_DEFAULT_FLOAT_ABI
>>>> #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_HARD
>>>> #undef GLIBC_DYNAMIC_LINKER_DEFAULT
>>>> #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
>>>>
>>>> Which is not quite as simple as leaving out the second re-define, but
>>>> pretty close.
>>>
>>> Hi Richard.  Your patch tests just fine.  I like it.  You could change
>>> the spec rule to the newer if-elseif-else form but that's a nit.
>>>
>>> -- Michael
>>>
>>
>> Great, thanks!  I've committed it as is.
> 
> I suggest to add this also to the gcc 4.7 branch. Richard (Earnshaw), 
> could you do take care of this, please?

This is a behaviour change.  It would need RM approval for a release branch.

R.
Andreas Jaeger May 23, 2012, 8:01 a.m. UTC | #8
On Wednesday, May 23, 2012 09:56:31 Richard Earnshaw wrote:
> [...]
> This is a behaviour change.  It would need RM approval for a release
> branch.
> 
> R.

There was agreement by all pushing for the change to use it. So, let's ask 
the release managers about their opinion,

Andreas
Richard Biener May 23, 2012, 8:17 a.m. UTC | #9
On Wed, 23 May 2012, Andreas Jaeger wrote:

> On Wednesday, May 23, 2012 09:56:31 Richard Earnshaw wrote:
> > [...]
> > This is a behaviour change.  It would need RM approval for a release
> > branch.
> > 
> > R.
> 
> There was agreement by all pushing for the change to use it. So, let's ask 
> the release managers about their opinion,

I'm ok with the change - but of course only to carry one less patch
in our local tree.  What do others think?  It would definitely (anyway)
need documenting in changes.html (for both 4.7.1 and 4.8).

Richard.
Mike Frysinger May 23, 2012, 2:16 p.m. UTC | #10
On Wednesday 23 May 2012 04:17:51 Richard Guenther wrote:
> On Wed, 23 May 2012, Andreas Jaeger wrote:
> > On Wednesday, May 23, 2012 09:56:31 Richard Earnshaw wrote:
> > > [...]
> > > This is a behaviour change.  It would need RM approval for a release
> > > branch.
> > > 
> > > R.
> > 
> > There was agreement by all pushing for the change to use it. So, let's
> > ask the release managers about their opinion,
> 
> I'm ok with the change - but of course only to carry one less patch
> in our local tree.  What do others think?  It would definitely (anyway)
> need documenting in changes.html (for both 4.7.1 and 4.8).

i've done this for Gentoo and 4.5.0+, so if all the distros are going to be 
doing this in 4.7.x anyways, makes sense to me to do it in the official branch.
-mike
Michael Hope May 23, 2012, 9:11 p.m. UTC | #11
On 24 May 2012 02:16, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 23 May 2012 04:17:51 Richard Guenther wrote:
>> On Wed, 23 May 2012, Andreas Jaeger wrote:
>> > On Wednesday, May 23, 2012 09:56:31 Richard Earnshaw wrote:
>> > > [...]
>> > > This is a behaviour change.  It would need RM approval for a release
>> > > branch.
>> > >
>> > > R.
>> >
>> > There was agreement by all pushing for the change to use it. So, let's
>> > ask the release managers about their opinion,
>>
>> I'm ok with the change - but of course only to carry one less patch
>> in our local tree.  What do others think?  It would definitely (anyway)
>> need documenting in changes.html (for both 4.7.1 and 4.8).
>
> i've done this for Gentoo and 4.5.0+, so if all the distros are going to be
> doing this in 4.7.x anyways, makes sense to me to do it in the official branch.

Agreed.  Google have done it for their 4.6, Fedora have done it for
4.7 (?), and we've done it for Linaro GCC 4.6 and 4.7.

My concern is that a point release of GCC would stop working against
the latest release of GLIBC.

I'm happy to prepare a backport to GCC 4.6, GCC 4.7, and GLIBC 2.15 so
the next set of point releases will all work with each other.  This
would match what the distros are doing.

-- Michael
Mike Frysinger May 24, 2012, 4:09 a.m. UTC | #12
On Wednesday 23 May 2012 17:11:53 Michael Hope wrote:
> On 24 May 2012 02:16, Mike Frysinger wrote:
> > On Wednesday 23 May 2012 04:17:51 Richard Guenther wrote:
> >> On Wed, 23 May 2012, Andreas Jaeger wrote:
> >> > On Wednesday, May 23, 2012 09:56:31 Richard Earnshaw wrote:
> >> > > [...]
> >> > > This is a behaviour change.  It would need RM approval for a release
> >> > > branch.
> >> > > 
> >> > > R.
> >> > 
> >> > There was agreement by all pushing for the change to use it. So, let's
> >> > ask the release managers about their opinion,
> >> 
> >> I'm ok with the change - but of course only to carry one less patch
> >> in our local tree.  What do others think?  It would definitely (anyway)
> >> need documenting in changes.html (for both 4.7.1 and 4.8).
> > 
> > i've done this for Gentoo and 4.5.0+, so if all the distros are going to
> > be doing this in 4.7.x anyways, makes sense to me to do it in the
> > official branch.
> 
> Agreed.  Google have done it for their 4.6, Fedora have done it for
> 4.7 (?), and we've done it for Linaro GCC 4.6 and 4.7.
> 
> My concern is that a point release of GCC would stop working against
> the latest release of GLIBC.
> 
> I'm happy to prepare a backport to GCC 4.6, GCC 4.7, and GLIBC 2.15 so
> the next set of point releases will all work with each other.  This
> would match what the distros are doing.

http://sources.gentoo.org/gentoo/src/patchsets/gcc/4.6.3/gentoo/33_all_armhf.patch
http://sources.gentoo.org/gentoo/src/patchsets/gcc/4.7.0/gentoo/33_all_armhf.patch
http://sources.gentoo.org/gentoo/src/patchsets/glibc/2.15/6226_all_arm-glibc-2.15-hardfp.patch
-mike
Jakub Jelinek May 24, 2012, 5:07 p.m. UTC | #13
On Wed, May 23, 2012 at 10:17:51AM +0200, Richard Guenther wrote:
> On Wed, 23 May 2012, Andreas Jaeger wrote:
> 
> > On Wednesday, May 23, 2012 09:56:31 Richard Earnshaw wrote:
> > > [...]
> > > This is a behaviour change.  It would need RM approval for a release
> > > branch.
> > > 
> > > R.
> > 
> > There was agreement by all pushing for the change to use it. So, let's ask 
> > the release managers about their opinion,
> 
> I'm ok with the change - but of course only to carry one less patch
> in our local tree.  What do others think?  It would definitely (anyway)
> need documenting in changes.html (for both 4.7.1 and 4.8).

I'm ok with that change as well (again, would apply that too), but note that
it isn't effortless on the side of gcc users, with the patch in they either
need recent enough glibc, or at least make a compatiblity symlink.

	Jakub
diff mbox

Patch

--- linux-eabi.h	(revision 186924)
+++ linux-eabi.h	(local)
@@ -32,7 +32,8 @@ 
   while (false)
 
 /* We default to a soft-float ABI so that binaries can run on all
-   target hardware.  */
+   target hardware.  If you override this to use the hard-float ABI then
+   change the setting of GLIBC_DYNAMIC_LINKER_DEFAULT as well.  */
 #undef  TARGET_DEFAULT_FLOAT_ABI
 #define TARGET_DEFAULT_FLOAT_ABI ARM_FLOAT_ABI_SOFT
 
@@ -59,16 +60,19 @@ 
 #undef  SUBTARGET_EXTRA_LINK_SPEC
 #define SUBTARGET_EXTRA_LINK_SPEC " -m " TARGET_LINKER_EMULATION
 
-/* Use ld-linux.so.3 so that it will be possible to run "classic"
-   GNU/Linux binaries on an EABI system.  */
+/* GNU/Linux on ARM currently supports three dynamic linkers:
+   - ld-linux.so.2 - for the legacy ABI
+   - ld-linux.so.3 - for the EABI-derived soft-float ABI
+   - ld-linux-armhf.so.3 - for the EABI-derived hard-float ABI.
+   All the dynamic linkers live in /lib.
+   We default to soft-float, but this can be overridden by changing both
+   GLIBC_DYNAMIC_LINKER_DEFAULT and TARGET_DEFAULT_FLOAT_ABI.  */
+
 #undef  GLIBC_DYNAMIC_LINKER
 #define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "/lib/ld-linux.so.3"
 #define GLIBC_DYNAMIC_LINKER_HARD_FLOAT "/lib/ld-linux-armhf.so.3"
-#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
-#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
-#else
 #define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
-#endif
+
 #define GLIBC_DYNAMIC_LINKER \
    "%{mfloat-abi=hard:" GLIBC_DYNAMIC_LINKER_HARD_FLOAT "} \
     %{mfloat-abi=soft*:" GLIBC_DYNAMIC_LINKER_SOFT_FLOAT "} \