Re: [PATCH, ARM]: Fix static interworking call

[Christophe Lyon <christophe.lyon@linaro.org>]

On 18 September 2015 at 17:03, Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
> On 18/09/15 15:38, Christian Bruel wrote:
>>
>>
>> On 09/18/2015 04:16 PM, Richard Earnshaw wrote:
>>> On 17/09/15 09:46, Christian Bruel wrote:
>>>> As obvious, bad operand number.
>>>>
>>>> OK for trunk ?
>>>>
>>>> Christian
>>>>
>>>>
>>>> p1.patch
>>>>
>>>>
>>>> 2015-09-18  Christian Bruel  <christian.bruel@st.com>
>>>>
>>>>     * config/arm/arm.md (*call_value_symbol): Fix operand for
>>>> interworking.
>>>>
>>>> 2015-09-18  Christian Bruel  <christian.bruel@st.com>
>>>>
>>>>     * gcc.target/arm/attr_thumb-static2.c: New test.
>>>>
>>>> --- gnu_trunk.ref/gcc/gcc/config/arm/arm.md    2015-09-14
>>>> 09:52:37.697264500 +0200
>>>> +++ gnu_trunk.p0/gcc/gcc/config/arm/arm.md    2015-09-17
>>>> 10:03:33.849451705 +0200
>>>> @@ -7891,7 +7891,7 @@
>>>>      /* Switch mode now when possible.  */
>>>>      if (SYMBOL_REF_DECL (op) && !TREE_PUBLIC (SYMBOL_REF_DECL (op))
>>>>           && arm_arch5 && arm_change_mode_p (SYMBOL_REF_DECL (op)))
>>>> -      return NEED_PLT_RELOC ? \"blx%?\\t%a0(PLT)\" : \"blx%?\\t(%a0)\";
>>>> +      return NEED_PLT_RELOC ? \"blx%?\\t%a1(PLT)\" : \"blx%?\\t(%a1)\";
>>>>
>>>>       return NEED_PLT_RELOC ? \"bl%?\\t%a1(PLT)\" : \"bl%?\\t%a1\";
>>>>     }"
>>>> diff -ruNp
>>>> gnu_trunk.ref/gcc/gcc/testsuite/gcc.target/arm/attr_thumb-static2.c
>>>> gnu_trunk.p0/gcc/gcc/testsuite/gcc.target/arm/attr_thumb-static2.c
>>>> ---
>>>> gnu_trunk.ref/gcc/gcc/testsuite/gcc.target/arm/attr_thumb-static2.c    1970-01-01
>>>> 01:00:00.000000000 +0100
>>>> +++
>>>> gnu_trunk.p0/gcc/gcc/testsuite/gcc.target/arm/attr_thumb-static2.c
>>>> 2015-09-17 10:08:08.350064131 +0200
>>>> @@ -0,0 +1,40 @@
>>>> +/* Check that interwork between static functions is correctly
>>>> resolved. */
>>>> +
>>>> +/* { dg-skip-if "" { ! { arm_thumb1_ok || arm_thumb2_ok } } } */
>>>> +/* { dg-options "-O0 -march=armv7-a -mfloat-abi=hard" } */
>>>
>>> You can't have thumb1 and hard float,
>>
>> Ah OK I didn't know that. Is it that there was no FPU before V5 ?
>>
>
> Thumb1 had no instruction encodings for accessing the FPU.
>
>>> so the skip unless thumb1 seems a nonsense.
>>
>> And there is no thumb1 and march=armv7-a !. So indeed the skip unless
>> thumb1 is a nonsense.
>> Is the attached patch OK to clean this up ?
>>
>> thanks,
>>
>>
>>>
>>> R.
>>>
>>>> +/* { dg-do compile } */
>>>> +
>>>> +struct _NSPoint
>>>> +{
>>>> +  float x;
>>>> +  float y;
>>>> +};
>>>> +
>>>> +typedef struct _NSPoint NSPoint;
>>>> +
>>>> +static NSPoint
>>>> +__attribute__ ((target("arm")))
>>>> +NSMakePoint (float x, float y)
>>>> +{
>>>> +  NSPoint point;
>>>> +  point.x = x;
>>>> +  point.y = y;
>>>> +  return point;
>>>> +}
>>>> +
>>>> +static NSPoint
>>>> +__attribute__ ((target("thumb")))
>>>> +RelativePoint (NSPoint point, NSPoint refPoint)
>>>> +{
>>>> +  return NSMakePoint (refPoint.x + point.x, refPoint.y + point.y);
>>>> +}
>>>> +
>>>> +NSPoint
>>>> +__attribute__ ((target("arm")))
>>>> +g(NSPoint refPoint)
>>>> +{
>>>> +  float pointA, pointB;
>>>> +  return RelativePoint (NSMakePoint (0, pointA), refPoint);
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler-times "blx" 2 } } */
>>>>
>>>
>>
>> 1.patch
>>
>>
>> 2015-09-17  Christian Bruel  <christian.bruel@st.com>
>>
>>       * gcc.target/arm/attr_thumb-static2.c: Test only for thumb2.
>>
>> Index: attr_thumb-static2.c
>> ===================================================================
>> --- attr_thumb-static2.c      (revision 227904)
>> +++ attr_thumb-static2.c      (working copy)
>> @@ -1,6 +1,6 @@
>>  /* Check that interwork between static functions is correctly resolved. */
>>
>> -/* { dg-skip-if "" { ! { arm_thumb1_ok || arm_thumb2_ok } } } */
>> +/* { dg-require-effective-target arm_thumb2_ok } */
>>  /* { dg-options "-O0 -march=armv7-a -mfloat-abi=hard" } */
>>  /* { dg-do compile } */
>>
>>
>
> Do you really need -mfloat-abi=hard for this test?  If so, I think you
> also need "dg-require-effective-target arm_hard_vfp_ok".  See
> gcc.target/arm/pr65729.c
>
> R.
>
Christian,

It seems you committed the 1st version of your patch.
However, it fails if one forces a armv5t target, because, as Richard
said -mfloat-abi=hard is not supported on Thumb1.

It tried to remove -mfloat-abi=hard, and I noticed that only 1 'blx'
is generated when using soft or softfp.

I expect this to be fixed by the linker anyway, but it may mean there
is still something wrong.

Christophe.