[ARM] Fix broken sibcall with longcall, APCS frame and VFP

[ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Eric Botcazou]

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

Hi,

this is a regression present on the mainline and 6 branch and introduced by:

2014-04-25  Jiong Wang  <jiong.wang@arm.com>

	* config/arm/predicates.md (call_insn_operand): Add long_call check.
	* config/arm/arm.md (sibcall, sibcall_value): Force the address to
	reg for long_call.
	* config/arm/arm.c (arm_function_ok_for_sibcall): Remove long_call
	restriction.

For a longcall, any sibcall becomes an indirect sibcall and therefore requires 
a register to hold the target address.  Now if all the argument registers are  
taken, this register will be IP but, for APCS frames and VFP, IP can be used 
in the sibcall epilogue to restore the VFP registers, so the target address is 
overwritten and the call goes astray.  Testcase attached, compile it with e.g. 
-mapcs-frame -mfloat-abi=soft -O -foptimize-sibling-calls -ffunction-sections
and you'll see for arm-eabi:

	sub	ip, fp, #36
	vldm	ip!, {d8}
	sub	sp, fp, #28
	ldmfd	sp, {r4, r5, r6, r7, fp, sp, lr}
	bx	ip

The attached patch reinstates the restriction for APCS frames and VFP.  This 
might be deemed a big hammer, but isn't a regression from earlier releases of 
the compiler and APCS frames are deprecated in any case (we still support them 
for VxWorks 6 but VxWorks 7 switched to AAPCS).

Tested on ARM/VxWorks 6 and ARM/EABI, OK for mainline and 6 branch?


2016-08-31  Eric Botcazou  <ebotcazou@adacore.com>

        * config/arm/arm.c (arm_function_ok_for_sibcall): Add back restriction
        for long calls with APCS frame and VFP.


2016-08-31  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.target/arm/vfp-longcall-apcs.c: New test.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 874 bytes --]

Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 239888)
+++ config/arm/arm.c	(working copy)
@@ -6773,6 +6773,14 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
   if (TARGET_VXWORKS_RTP && flag_pic && decl && !targetm.binds_local_p (decl))
     return false;
 
+  /* ??? Cannot tail-call to long calls with APCS frame and VFP, because IP
+     may be used both as target of the call and base register for restoring
+     the VFP registers  */
+  if (TARGET_APCS_FRAME && TARGET_ARM
+      && TARGET_HARD_FLOAT && TARGET_VFP
+      && decl && arm_is_long_call_p (decl))
+    return false;
+
   /* If we are interworking and the function is not declared static
      then we can't tail-call it unless we know that it exists in this
      compilation unit (since it might be a Thumb routine).  */

[-- Attachment #3: vfp-longcall-apcs.c --]
[-- Type: text/x-csrc, Size: 654 bytes --]

/* { dg-do run } */
/* { dg-require-effective-target arm_vfp_ok } */
/* { dg-options "-mapcs-frame -mfpu=vfp -mfloat-abi=softfp -O -foptimize-sibling-calls -ffunction-sections" } */

extern void abort (void);

static __attribute__((noclone, noinline, long_call))
int foo (int a, int b, int c, int d, double i)
{
  return a;
}

static __attribute__((noclone, noinline))
double baz (double i)
{
  return i;
}

static __attribute__((noclone, noinline))
int bar (int a, int b, int c, int d, double i, double j)
{
  double l = baz (i) * j;
  return foo (a, b, c, d, l);
}

int
main (void)
{
  if (bar (0, 0, 0, 0, 0.0, 0.0) != 0)
    abort ();

  return 0;
}

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Richard Earnshaw (lists)]

On 31/08/16 16:35, Eric Botcazou wrote:
> Hi,
> 
> this is a regression present on the mainline and 6 branch and introduced by:
> 
> 2014-04-25  Jiong Wang  <jiong.wang@arm.com>
> 
> 	* config/arm/predicates.md (call_insn_operand): Add long_call check.
> 	* config/arm/arm.md (sibcall, sibcall_value): Force the address to
> 	reg for long_call.
> 	* config/arm/arm.c (arm_function_ok_for_sibcall): Remove long_call
> 	restriction.
> 
> For a longcall, any sibcall becomes an indirect sibcall and therefore requires 
> a register to hold the target address.  Now if all the argument registers are  
> taken, this register will be IP but, for APCS frames and VFP, IP can be used 
> in the sibcall epilogue to restore the VFP registers, so the target address is 
> overwritten and the call goes astray.  Testcase attached, compile it with e.g. 
> -mapcs-frame -mfloat-abi=soft -O -foptimize-sibling-calls -ffunction-sections
> and you'll see for arm-eabi:
> 
> 	sub	ip, fp, #36
> 	vldm	ip!, {d8}
> 	sub	sp, fp, #28
> 	ldmfd	sp, {r4, r5, r6, r7, fp, sp, lr}
> 	bx	ip
> 
> The attached patch reinstates the restriction for APCS frames and VFP.  This 
> might be deemed a big hammer, but isn't a regression from earlier releases of 
> the compiler and APCS frames are deprecated in any case (we still support them 
> for VxWorks 6 but VxWorks 7 switched to AAPCS).
> 
> Tested on ARM/VxWorks 6 and ARM/EABI, OK for mainline and 6 branch?
> 
> 

Since you're going to need a back-port there should be a PR filed for this.


> 2016-08-31  Eric Botcazou  <ebotcazou@adacore.com>
> 
>         * config/arm/arm.c (arm_function_ok_for_sibcall): Add back restriction
>         for long calls with APCS frame and VFP.
> 
> 
> 2016-08-31  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* gcc.target/arm/vfp-longcall-apcs.c: New test.
> 

Have you checked that this works with multi-lib dejagnu runs and on
hard-float systems?  I doubt this will work in armhf-linux, for example.

R.

> 
> p.diff
> 
> 
> Index: config/arm/arm.c
> ===================================================================
> --- config/arm/arm.c	(revision 239888)
> +++ config/arm/arm.c	(working copy)
> @@ -6773,6 +6773,14 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>    if (TARGET_VXWORKS_RTP && flag_pic && decl && !targetm.binds_local_p (decl))
>      return false;
>  
> +  /* ??? Cannot tail-call to long calls with APCS frame and VFP, because IP
> +     may be used both as target of the call and base register for restoring
> +     the VFP registers  */
> +  if (TARGET_APCS_FRAME && TARGET_ARM
> +      && TARGET_HARD_FLOAT && TARGET_VFP
> +      && decl && arm_is_long_call_p (decl))
> +    return false;
> +
>    /* If we are interworking and the function is not declared static
>       then we can't tail-call it unless we know that it exists in this
>       compilation unit (since it might be a Thumb routine).  */
> 
> 
> vfp-longcall-apcs.c
> 
> 
> /* { dg-do run } */
> /* { dg-require-effective-target arm_vfp_ok } */
> /* { dg-options "-mapcs-frame -mfpu=vfp -mfloat-abi=softfp -O -foptimize-sibling-calls -ffunction-sections" } */
> 
> extern void abort (void);
> 
> static __attribute__((noclone, noinline, long_call))
> int foo (int a, int b, int c, int d, double i)
> {
>   return a;
> }
> 
> static __attribute__((noclone, noinline))
> double baz (double i)
> {
>   return i;
> }
> 
> static __attribute__((noclone, noinline))
> int bar (int a, int b, int c, int d, double i, double j)
> {
>   double l = baz (i) * j;
>   return foo (a, b, c, d, l);
> }
> 
> int
> main (void)
> {
>   if (bar (0, 0, 0, 0, 0.0, 0.0) != 0)
>     abort ();
> 
>   return 0;
> }
> 

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Eric Botcazou]

> Since you're going to need a back-port there should be a PR filed for this.

PR target/77439

> Have you checked that this works with multi-lib dejagnu runs and on
> hard-float systems?  I doubt this will work in armhf-linux, for example.

No, I guess some dg-skip-if would be in order, but I'm not able to devise one.

-- 
Eric Botcazou

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Richard Earnshaw (lists)]

On 01/09/16 08:47, Eric Botcazou wrote:
>> Since you're going to need a back-port there should be a PR filed for this.
> 
> PR target/77439
> 
>> Have you checked that this works with multi-lib dejagnu runs and on
>> hard-float systems?  I doubt this will work in armhf-linux, for example.
> 
> No, I guess some dg-skip-if would be in order, but I'm not able to devise one.
> 

Since the test is an execution test, why not just drop the problematic
parts of dg-options "-mfpu=vfp -mfloat-abi=softfp" and rely on multilib
permutations as appropriate.  That gives more all-round coverage as well.

R.

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Richard Earnshaw (lists)]

On 01/09/16 10:03, Richard Earnshaw (lists) wrote:
> On 01/09/16 08:47, Eric Botcazou wrote:
>>> Since you're going to need a back-port there should be a PR filed for this.
>>
>> PR target/77439
>>
>>> Have you checked that this works with multi-lib dejagnu runs and on
>>> hard-float systems?  I doubt this will work in armhf-linux, for example.
>>
>> No, I guess some dg-skip-if would be in order, but I'm not able to devise one.
>>
> 
> Since the test is an execution test, why not just drop the problematic
> parts of dg-options "-mfpu=vfp -mfloat-abi=softfp" and rely on multilib
> permutations as appropriate.  That gives more all-round coverage as well.
> 
> R.
> 

That would also allow you to drop the effective-target test as well.

R.

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Richard Earnshaw (lists)]

On 01/09/16 10:03, Richard Earnshaw (lists) wrote:
> On 01/09/16 10:03, Richard Earnshaw (lists) wrote:
>> On 01/09/16 08:47, Eric Botcazou wrote:
>>>> Since you're going to need a back-port there should be a PR filed for this.
>>>
>>> PR target/77439
>>>
>>>> Have you checked that this works with multi-lib dejagnu runs and on
>>>> hard-float systems?  I doubt this will work in armhf-linux, for example.
>>>
>>> No, I guess some dg-skip-if would be in order, but I'm not able to devise one.
>>>
>>
>> Since the test is an execution test, why not just drop the problematic
>> parts of dg-options "-mfpu=vfp -mfloat-abi=softfp" and rely on multilib
>> permutations as appropriate.  That gives more all-round coverage as well.
>>
>> R.
>>
> 
> That would also allow you to drop the effective-target test as well.
> 
> R.
> 

This seems to have fallen through a crack.  Did you get a chance to try
either of these suggestions?

R.

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Eric Botcazou]

> This seems to have fallen through a crack.  Did you get a chance to try
> either of these suggestions?

So just:

/* { dg-do run } */
/* { dg-options "-mapcs-frame -O -foptimize-sibling-calls -ffunction-sections" 
} */

in the header of the tescase?

-- 
Eric Botcazou

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Richard Earnshaw (lists)]

On 20/01/17 18:13, Eric Botcazou wrote:
>> This seems to have fallen through a crack.  Did you get a chance to try
>> either of these suggestions?
> 
> So just:
> 
> /* { dg-do run } */
> /* { dg-options "-mapcs-frame -O -foptimize-sibling-calls -ffunction-sections" 
> } */
> 
> in the header of the tescase?
> 

Yes, let's try it like that.  If it causes problems on multilib test
runs we can always revisit and apply some restrictions.

R.