[PATCH] Allow FP to be used as a call-saved registe

[PATCH] Allow FP to be used as a call-saved registe

[Tamar Christina]

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

Hi All,

This patch allows the FP register to be used as a call-saved
register when -fomit-frame-pointer is used.

The change is done in such a way that the defaults do not change.
To use the FP register both -fomit-frame-pointer and
-fcall-saved-<hard_fp_reg> need to be used.

Regression ran on aarch64-none-linux-gnu and no regressions.
Bootstrapped and ran regressions on `x86_64` and no regressions.

A new test fp_free_1 was added to test functionality.

Ok for trunk?

Thanks,
Tamar

PS. I don't have commit rights so if OK can someone apply the patch for me.

gcc/
2016-09-01  Tamar Christina  <tamar.christina@arm.com>

	* gcc/reginfo.c (fix_register): Allow FP to be set if
	-fomit-frame-pointer.

gcc/testsuite/
2016-08-17  Tamar Christina  <tamar.christina@arm.com>

	* gcc.target/aarch64/fp_free_1.c: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc.patch --]
[-- Type: text/x-patch; name=gcc.patch, Size: 1905 bytes --]

diff --git a/gcc/reginfo.c b/gcc/reginfo.c
index 0cda6aa620098c752522add589b42631b382d9fc..ea96e236a5f76fb7ddc2c9406b7b377aa0eb3087 100644
--- a/gcc/reginfo.c
+++ b/gcc/reginfo.c
@@ -692,12 +692,13 @@ fix_register (const char *name, int fixed, int call_used)
       for (i = reg; i < reg + nregs; i++)
 	{
 	  if ((i == STACK_POINTER_REGNUM
+	       || (
 #ifdef HARD_FRAME_POINTER_REGNUM
-	       || i == HARD_FRAME_POINTER_REGNUM
+		   i == HARD_FRAME_POINTER_REGNUM
 #else
-	       || i == FRAME_POINTER_REGNUM
+		   i == FRAME_POINTER_REGNUM
 #endif
-	       )
+		    && !flag_omit_frame_pointer))
 	      && (fixed == 0 || call_used == 0))
 	    {
 	      switch (fixed)
diff --git a/gcc/testsuite/gcc.target/aarch64/fp_free_1.c b/gcc/testsuite/gcc.target/aarch64/fp_free_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..79e23b13d3fb0f801e201c69286d27bac97aa013
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/fp_free_1.c
@@ -0,0 +1,32 @@
+/* { dg-do run { target aarch64-*-* } } */
+/* { dg-options "-O2 -fno-inline -fomit-frame-pointer -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7 -ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x16 -ffixed-x17 -ffixed-x18 -ffixed-x19 -ffixed-x20 -ffixed-x21 -ffixed-x22 -ffixed-x23 -ffixed-x24 -ffixed-x25 -ffixed-x26 -ffixed-x27 -ffixed-x28 -ffixed-x30 -mgeneral-regs-only -fno-ipa-cp -fno-schedule-fusion -fno-peephole2 -fcall-saved-x29  -fdump-rtl-ira" } */
+
+extern void abort ();
+
+int
+dec (int a, int b)
+{
+  return a + b;
+}
+
+int
+cal (int a, int b)
+{
+  int sum1 = a * b;
+  int sum2 = a / b;
+  int sum = dec (sum1, sum2);
+  return a + b + sum + sum1 + sum2;
+}
+
+int
+main (int argc, char **argv)
+{
+  int ret = cal (2, 1);
+
+  if (ret != 11)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-rtl-dump "assign reg 29" "ira" } } */

Re: [PATCH] Allow FP to be used as a call-saved registe

[James Greenhalgh]

On Mon, Sep 05, 2016 at 03:59:18PM +0100, Tamar Christina wrote:
> Hi All,
> 
> This patch allows the FP register to be used as a call-saved
> register when -fomit-frame-pointer is used.
> 
> The change is done in such a way that the defaults do not change.
> To use the FP register both -fomit-frame-pointer and
> -fcall-saved-<hard_fp_reg> need to be used.
> 
> Regression ran on aarch64-none-linux-gnu and no regressions.
> Bootstrapped and ran regressions on `x86_64` and no regressions.
> 
> A new test fp_free_1 was added to test functionality.
> 
> Ok for trunk?
> 
> Thanks,
> Tamar
> 
> PS. I don't have commit rights so if OK can someone apply the patch for me.
> 
> gcc/
> 2016-09-01  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* gcc/reginfo.c (fix_register): Allow FP to be set if
> 	-fomit-frame-pointer.
> 
> gcc/testsuite/
> 2016-08-17  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* gcc.target/aarch64/fp_free_1.c: New.

The AArch64 testcase is fine. But this change is to generic code, so you'll
want to CC some maintaners for that area (check the MAINTAINERS file) rather
than Richard, Marcus and myself. I've added Vlad to CC for you.

Thanks,
James

Re: [PATCH] Allow FP to be used as a call-saved registe

[Jeff Law]

On 09/05/2016 08:59 AM, Tamar Christina wrote:
> Hi All,
>
> This patch allows the FP register to be used as a call-saved
> register when -fomit-frame-pointer is used.
>
> The change is done in such a way that the defaults do not change.
> To use the FP register both -fomit-frame-pointer and
> -fcall-saved-<hard_fp_reg> need to be used.
>
> Regression ran on aarch64-none-linux-gnu and no regressions.
> Bootstrapped and ran regressions on `x86_64` and no regressions.
>
> A new test fp_free_1 was added to test functionality.
>
> Ok for trunk?
>
> Thanks,
> Tamar
>
> PS. I don't have commit rights so if OK can someone apply the patch for me.
>
> gcc/
> 2016-09-01  Tamar Christina  <tamar.christina@arm.com>
>
> 	* gcc/reginfo.c (fix_register): Allow FP to be set if
> 	-fomit-frame-pointer.
I'm a little surprised you need this.  Most ports allow use of FP as a 
call-saved register with -fomit-frame-pointer.

Also note the documentation explicitly forbids using -fcall-saved for 
the stack or frame pointer.


Jeff

Re: [PATCH] Allow FP to be used as a call-saved registe

[Tamar Christina]

Hi Jeff,


On 12/09/16 18:16, Jeff Law wrote:
> On 09/05/2016 08:59 AM, Tamar Christina wrote:
>> Hi All,
>>
>> This patch allows the FP register to be used as a call-saved
>> register when -fomit-frame-pointer is used.
>>
>> The change is done in such a way that the defaults do not change.
>> To use the FP register both -fomit-frame-pointer and
>> -fcall-saved-<hard_fp_reg> need to be used.
>>
>> Regression ran on aarch64-none-linux-gnu and no regressions.
>> Bootstrapped and ran regressions on `x86_64` and no regressions.
>>
>> A new test fp_free_1 was added to test functionality.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Tamar
>>
>> PS. I don't have commit rights so if OK can someone apply the patch 
>> for me.
>>
>> gcc/
>> 2016-09-01  Tamar Christina  <tamar.christina@arm.com>
>>
>>     * gcc/reginfo.c (fix_register): Allow FP to be set if
>>     -fomit-frame-pointer.
> I'm a little surprised you need this.  Most ports allow use of FP as a 
> call-saved register with -fomit-frame-pointer.
I think this is because on most architectures the FP is not in the fixed 
registers list. But the AArch64 ABI (I believe) currently
mandates that it is. With the option of:

- It may permit the frame pointer register to be used as a 
general-purpose callee-saved register, but provide a platform-specific 
mechanism for external agents to reliably detect this condition

- It may elect not to maintain a frame chain and to use the frame 
pointer register as a general-purpose callee-saved register.

(from section 5.2.3 of the ABI).

In which case `-fomit-frame-pointer` alone won't be enough to use it as 
a callee-saved register. I could try looking into alternatives if this 
is a problem for other ports.

> Also note the documentation explicitly forbids using -fcall-saved for 
> the stack or frame pointer.
>
Ah, yes, hadn't noticed that before. Isn't it a bit too strict a 
restriction? In general if you have -fomit-frame-pointer then shouldn't 
the it be safe for the FP to be used
with -fcall-saved? Since it's probably a no-op on most ports that 
support -fomit-frame-pointer anyway?
> Jeff
>

Regards,
Tamar

Re: [PATCH] Allow FP to be used as a call-saved registe

[Jeff Law]

On 09/13/2016 05:10 AM, Tamar Christina wrote:
> Hi Jeff,
>
>
> On 12/09/16 18:16, Jeff Law wrote:
>> On 09/05/2016 08:59 AM, Tamar Christina wrote:
>>> Hi All,
>>>
>>> This patch allows the FP register to be used as a call-saved
>>> register when -fomit-frame-pointer is used.
>>>
>>> The change is done in such a way that the defaults do not change.
>>> To use the FP register both -fomit-frame-pointer and
>>> -fcall-saved-<hard_fp_reg> need to be used.
>>>
>>> Regression ran on aarch64-none-linux-gnu and no regressions.
>>> Bootstrapped and ran regressions on `x86_64` and no regressions.
>>>
>>> A new test fp_free_1 was added to test functionality.
>>>
>>> Ok for trunk?
>>>
>>> Thanks,
>>> Tamar
>>>
>>> PS. I don't have commit rights so if OK can someone apply the patch
>>> for me.
>>>
>>> gcc/
>>> 2016-09-01  Tamar Christina  <tamar.christina@arm.com>
>>>
>>>     * gcc/reginfo.c (fix_register): Allow FP to be set if
>>>     -fomit-frame-pointer.
>> I'm a little surprised you need this.  Most ports allow use of FP as a
>> call-saved register with -fomit-frame-pointer.
> I think this is because on most architectures the FP is not in the fixed
> registers list. But the AArch64 ABI (I believe) currently
> mandates that it is. With the option of:
>
> - It may permit the frame pointer register to be used as a
> general-purpose callee-saved register, but provide a platform-specific
> mechanism for external agents to reliably detect this condition
>
> - It may elect not to maintain a frame chain and to use the frame
> pointer register as a general-purpose callee-saved register.
So those don't seem to me to imply that the frame pointer needs to be a 
fixed register.   So the first thing I'd do is fix the aarch64 port to 
not do that and see what fallout there is and how to fix it.

Most ports simply don't mark the frame pointer as fixed.

I am a bit curious about how you're going to solve the "external agents 
to reliably detect this condition" :-)

>
>> Also note the documentation explicitly forbids using -fcall-saved for
>> the stack or frame pointer.
>>
> Ah, yes, hadn't noticed that before. Isn't it a bit too strict a
> restriction? In general if you have -fomit-frame-pointer then shouldn't
> the it be safe for the FP to be used
> with -fcall-saved? Since it's probably a no-op on most ports that
> support -fomit-frame-pointer anyway?
It might be.  In general I don't think the -fcall-whatever options are 
used that much anymore and I don't think anyone has seriously looked at 
their documentation in a long time.

Regardless, I still think the first step is to "unfix" the frame pointer 
hard register on the aarch64 port.

jeff

Re: [PATCH] Allow FP to be used as a call-saved registe

[Richard Earnshaw (lists)]

On 15/09/16 17:36, Jeff Law wrote:
> On 09/13/2016 05:10 AM, Tamar Christina wrote:
>> Hi Jeff,
>>
>>
>> On 12/09/16 18:16, Jeff Law wrote:
>>> On 09/05/2016 08:59 AM, Tamar Christina wrote:
>>>> Hi All,
>>>>
>>>> This patch allows the FP register to be used as a call-saved
>>>> register when -fomit-frame-pointer is used.
>>>>
>>>> The change is done in such a way that the defaults do not change.
>>>> To use the FP register both -fomit-frame-pointer and
>>>> -fcall-saved-<hard_fp_reg> need to be used.
>>>>
>>>> Regression ran on aarch64-none-linux-gnu and no regressions.
>>>> Bootstrapped and ran regressions on `x86_64` and no regressions.
>>>>
>>>> A new test fp_free_1 was added to test functionality.
>>>>
>>>> Ok for trunk?
>>>>
>>>> Thanks,
>>>> Tamar
>>>>
>>>> PS. I don't have commit rights so if OK can someone apply the patch
>>>> for me.
>>>>
>>>> gcc/
>>>> 2016-09-01  Tamar Christina  <tamar.christina@arm.com>
>>>>
>>>>     * gcc/reginfo.c (fix_register): Allow FP to be set if
>>>>     -fomit-frame-pointer.
>>> I'm a little surprised you need this.  Most ports allow use of FP as a
>>> call-saved register with -fomit-frame-pointer.
>> I think this is because on most architectures the FP is not in the fixed
>> registers list. But the AArch64 ABI (I believe) currently
>> mandates that it is. With the option of:
>>
>> - It may permit the frame pointer register to be used as a
>> general-purpose callee-saved register, but provide a platform-specific
>> mechanism for external agents to reliably detect this condition
>>
>> - It may elect not to maintain a frame chain and to use the frame
>> pointer register as a general-purpose callee-saved register.
> So those don't seem to me to imply that the frame pointer needs to be a
> fixed register.   So the first thing I'd do is fix the aarch64 port to
> not do that and see what fallout there is and how to fix it.
> 
> Most ports simply don't mark the frame pointer as fixed.
> 

The AArch64 ABI specification strongly recommends that, when a frame
record is not created, the frame pointer register is left unused so that
the frame chain, while not complete, is still valid (a chain of valid
records but ending in a NULL pointer).  That strongly suggests that FP
should remain a fixed register.

I guess we could push all of this into a new back-end option to permit
the 'I really want to use FP for general purposes', but it seems to be
just duplicating the existing use of -fcall-saved; so would be yet
another flag in the compiler that needs documenting.

It seems much more sensible to me to just make a slight relaxation of
the fixed-register code and then re-use the existing options.

> I am a bit curious about how you're going to solve the "external agents
> to reliably detect this condition" :-)
> 

:-)  It's a get-out clause to pacify folk who want a completely
different ABI variant.  Reliable detection would probably mean 'a
platform convention' that all code had to conform to.

R.

>>
>>> Also note the documentation explicitly forbids using -fcall-saved for
>>> the stack or frame pointer.
>>>
>> Ah, yes, hadn't noticed that before. Isn't it a bit too strict a
>> restriction? In general if you have -fomit-frame-pointer then shouldn't
>> the it be safe for the FP to be used
>> with -fcall-saved? Since it's probably a no-op on most ports that
>> support -fomit-frame-pointer anyway?
> It might be.  In general I don't think the -fcall-whatever options are
> used that much anymore and I don't think anyone has seriously looked at
> their documentation in a long time.
> 
> Regardless, I still think the first step is to "unfix" the frame pointer
> hard register on the aarch64 port.
> 
> jeff
> 

Re: [PATCH] Allow FP to be used as a call-saved registe

[Jeff Law]

On 09/19/2016 03:45 AM, Richard Earnshaw (lists) wrote:
>> So those don't seem to me to imply that the frame pointer needs to be a
>> fixed register.   So the first thing I'd do is fix the aarch64 port to
>> not do that and see what fallout there is and how to fix it.
>>
>> Most ports simply don't mark the frame pointer as fixed.
>>
>
> The AArch64 ABI specification strongly recommends that, when a frame
> record is not created, the frame pointer register is left unused so that
> the frame chain, while not complete, is still valid (a chain of valid
> records but ending in a NULL pointer).  That strongly suggests that FP
> should remain a fixed register.
So essentially you're asking that even with -fomit-frame-pointer that FP 
still not be used and that the user should specify an additional 
-fcall-saved- parameter?

I can see your point -- lots of things use -fomit-frame-pointer and you 
may not necessarily want to make FP available to the register allocator.

But by requiring -fcall-saved-whatever, aren't you forcing folks to 
encode aarch64 specific flags into their build systems?

Neither seems like a good position to be in.
>
> I guess we could push all of this into a new back-end option to permit
> the 'I really want to use FP for general purposes', but it seems to be
> just duplicating the existing use of -fcall-saved; so would be yet
> another flag in the compiler that needs documenting.
Yea, and more aarch64 specific bits in the build system.

>
> It seems much more sensible to me to just make a slight relaxation of
> the fixed-register code and then re-use the existing options.
Maybe.   I understand the situation much better now, but I don't see a 
particularly good solution.
jeff