[PATCH, ARM]: Fix static interworking call

[PATCH, ARM]: Fix static interworking call

[Christian Bruel]

[-- Attachment #1: Type: text/plain, Size: 60 bytes --]

As obvious, bad operand number.

OK for trunk ?

Christian


[-- Attachment #2: p1.patch --]
[-- Type: text/x-patch, Size: 2051 bytes --]

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" } */
+/* { 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

[Kyrill Tkachov]

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.

Ok, assuming testing is clean.
Thanks,
Kyrill

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

[Richard Earnshaw]

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, 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

[Christian Bruel]

[-- Attachment #1: Type: text/plain, Size: 2749 bytes --]



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 ?

 > 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 } } */
>>
>

[-- Attachment #2: 1.patch --]
[-- Type: text/x-patch, Size: 597 bytes --]

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 } */
 

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

[Richard Earnshaw]

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.

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

[Christophe Lyon]

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.

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

[Christian Bruel]

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.

> 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,

thanks

Christian

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

[Christophe Lyon]

On 21 September 2015 at 12:55, Christian Bruel <christian.bruel@st.com> 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

[Christian Bruel]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]



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 ?

[-- Attachment #2: attr_thumb-static2.c --]
[-- Type: text/x-csrc, Size: 789 bytes --]

/* Check that interwork between static functions is correctly resolved. */

/* { dg-require-effective-target arm_thumb2_ok } */
/* { dg-options "-O0 -march=armv7-a" } */
/* { 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

[Christophe Lyon]

On 21 September 2015 at 16:15, Christian Bruel <christian.bruel@st.com> 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).