Re: [PATCH, ARM]: Fix static interworking call
On 21 September 2015 at 16:15, Christian Bruel wrote:
>
>
> On 09/18/2015 05:03 PM, Richard Earnshaw wrote:
>
>>> 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
>>
>
> The test was not crashing for -mfloat-abi=soft.
> But the number of blx is independent. So yes we can write the conditions in
> such a way the test runs without hard fp.
>
> is this one better ?
You need to move the dg-do directive before the other ones (otherwise,
it overrides the effect of require-target).
Re: [PATCH, ARM]: Fix static interworking call
On 21 September 2015 at 12:55, Christian Bruel wrote: > Hi Christophe, > >> It seems you committed the 1st version of your patch. > > > Not sure what version you are talking about. I committed what was posted > (https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01260.html) at revision > #227880. I thought you had modified it, after Richard's comments. > >> However, it fails if one forces a armv5t target, because, as Richard >> said -mfloat-abi=hard is not supported on Thumb1. > > > sure, but the testcase was for thumb2. It's just that the thumb1 check in > the dg-skip-if is indeed useless. I'll fix this. > >> >> 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. > > > This is expected. For thumb1, such static calls interwork are still done by > the linker. > For consistency IMHO we could also resolve static interwork thumb1 calls in > the compiler, but that would be another issue. > This failure was only for thumb2 (this having -march=armv7-a in the > dg-options). (See https://sourceware.org/bugzilla/show_bug.cgi?id=17505) > > Hope that clarifies, > Yes, thanks. > thanks > > Christian
Re: [PATCH, ARM]: Fix static interworking call
On 18 September 2015 at 17:03, Richard Earnshaw
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
* config/arm/arm.md (*call_value_symbol): Fix operand for
interworking.
2015-09-18 Christian Bruel
* gcc.target/arm/attr_thumb-static2.c: New test.
--- gnu_trunk.ref/gcc/gcc/config/arm/arm.md2015-09-14
09:52:37.697264500 +0200
+++ gnu_trunk.p0/gcc/gcc/config/arm/arm.md2015-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.0 +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
>>
>> * 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.
Re: [PATCH, ARM]: Fix static interworking call
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
>>>
>>> * config/arm/arm.md (*call_value_symbol): Fix operand for
>>> interworking.
>>>
>>> 2015-09-18 Christian Bruel
>>>
>>> * gcc.target/arm/attr_thumb-static2.c: New test.
>>>
>>> --- gnu_trunk.ref/gcc/gcc/config/arm/arm.md2015-09-14
>>> 09:52:37.697264500 +0200
>>> +++ gnu_trunk.p0/gcc/gcc/config/arm/arm.md2015-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.0 +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
>
> * 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.
Re: [PATCH, ARM]: Fix static interworking call
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
>
> * config/arm/arm.md (*call_value_symbol): Fix operand for interworking.
>
> 2015-09-18 Christian Bruel
>
> * 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.md2015-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.0 +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, so the skip unless thumb1 seems a
nonsense.
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 } } */
>
Re: [PATCH, ARM]: Fix static interworking call
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 * config/arm/arm.md (*call_value_symbol): Fix operand for interworking. 2015-09-18 Christian Bruel * gcc.target/arm/attr_thumb-static2.c: New test. Ok, assuming testing is clean. Thanks, Kyrill
