diff mbox

[IRA] Analysis of register usage of functions for usage by IRA.

Message ID 52D4115E.2000904@mentor.com
State New
Headers show

Commit Message

Tom de Vries Jan. 13, 2014, 4:16 p.m. UTC
On 10-01-14 12:39, Richard Earnshaw wrote:
>>> >>Consequently, you'll need to add a patch for AArch64 which has two
>>> >>registers clobbered by PLT-based calls.
>>> >>
>> >
>> >Thanks for pointing that out. That's r16 and r17, right? I can propose the hook
>> >for AArch64, once we all agree on how the hook should look.
>> >
> Yes; and thanks!

Hi Richard,

I'm posting this patch that implements the TARGET_FN_OTHER_HARD_REG_USAGE hook 
for aarch64. It uses the conservative hook format for now.

I've build gcc and cc1 with the patch, and observed the impact on this code snippet:
...
static int
bar (int x)
{
   return x + 3;
}

int
foo (int y)
{
   return y + bar (y);
}
...

AFAICT, that looks as expected:
...
$ gcc fuse-caller-save.c -mno-lra -fno-use-caller-save -O2 -S -o- > 1
$ gcc fuse-caller-save.c -mno-lra -fuse-caller-save -O2 -S -o- > 2
$ diff -u 1 2
...

Btw, the results are the same for -mno-lra and -mlra. I'm just using the 
-mno-lra version here because the -mlra version of -fuse-caller-save is still in 
review ( http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00586.html ).

Thanks,
- Tom

Comments

Richard Earnshaw Jan. 14, 2014, 10 a.m. UTC | #1
On 13/01/14 16:16, Tom de Vries wrote:
> On 10-01-14 12:39, Richard Earnshaw wrote:
>>>>>> Consequently, you'll need to add a patch for AArch64 which has two
>>>>>> registers clobbered by PLT-based calls.
>>>>>>
>>>>
>>>> Thanks for pointing that out. That's r16 and r17, right? I can propose the hook
>>>> for AArch64, once we all agree on how the hook should look.
>>>>
>> Yes; and thanks!
> 
> Hi Richard,
> 
> I'm posting this patch that implements the TARGET_FN_OTHER_HARD_REG_USAGE hook 
> for aarch64. It uses the conservative hook format for now.
> 
> I've build gcc and cc1 with the patch, and observed the impact on this code snippet:
> ...
> static int
> bar (int x)
> {
>    return x + 3;
> }
> 
> int
> foo (int y)
> {
>    return y + bar (y);
> }
> ...
> 
> AFAICT, that looks as expected:
> ...
> $ gcc fuse-caller-save.c -mno-lra -fno-use-caller-save -O2 -S -o- > 1
> $ gcc fuse-caller-save.c -mno-lra -fuse-caller-save -O2 -S -o- > 2
> $ diff -u 1 2
> --- 1	2014-01-13 16:51:24.000000000 +0100
> +++ 2	2014-01-13 16:51:19.000000000 +0100
> @@ -11,14 +11,12 @@
>   	.global	foo
>   	.type	foo, %function
>   foo:
> -	stp	x29, x30, [sp, -32]!
> +	stp	x29, x30, [sp, -16]!
> +	mov	w1, w0
>   	add	x29, sp, 0
> -	str	x19, [sp,16]
> -	mov	w19, w0
>   	bl	bar
> -	add	w0, w0, w19
> -	ldr	x19, [sp,16]
> -	ldp	x29, x30, [sp], 32
> +	ldp	x29, x30, [sp], 16
> +	add	w0, w0, w1
>   	ret
>   	.size	foo, .-foo
>   	.section	.text.startup,"ax",%progbits
> ...
> 
> Btw, the results are the same for -mno-lra and -mlra. I'm just using the 
> -mno-lra version here because the -mlra version of -fuse-caller-save is still in 
> review ( http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00586.html ).
> 
> Thanks,
> - Tom
> 
> 
> fuse-caller-save-aarch64-hook.patch
> 
> 
> 2014-01-11  Tom de Vries  <tom@codesourcery.com>
> 
> 	* config/aarch64/aarch64.c (TARGET_FN_OTHER_HARD_REG_USAGE): Redefine as
> 	aarch64_fn_other_hard_reg_usage.
> 	(aarch64_fn_other_hard_reg_usage): New function.
> ---
>  gcc/config/aarch64/aarch64.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 3b1f6b5..295fd5d 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3287,6 +3287,16 @@ aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2)
>    return true;
>  }
>  
> +/* Implement TARGET_FN_OTHER_HARD_REG_USAGE.  */
> +
> +static bool
> +aarch64_fn_other_hard_reg_usage (struct hard_reg_set_container *regs)
> +{
> +  SET_HARD_REG_BIT (regs->set, R16_REGNUM);
> +  SET_HARD_REG_BIT (regs->set, R17_REGNUM);
> +  return true;
> +}


I think that in this context using IP0_REGNUM and IP1_REGNUM would be
slightly clearer; since it is because these registers are the
inter-procedure-call scratch registers that they aren't safe to use in
this context.

Otherwise, this is OK.

R.
diff mbox

Patch

2014-01-11  Tom de Vries  <tom@codesourcery.com>

	* config/aarch64/aarch64.c (TARGET_FN_OTHER_HARD_REG_USAGE): Redefine as
	aarch64_fn_other_hard_reg_usage.
	(aarch64_fn_other_hard_reg_usage): New function.
---
 gcc/config/aarch64/aarch64.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 3b1f6b5..295fd5d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3287,6 +3287,16 @@  aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2)
   return true;
 }
 
+/* Implement TARGET_FN_OTHER_HARD_REG_USAGE.  */
+
+static bool
+aarch64_fn_other_hard_reg_usage (struct hard_reg_set_container *regs)
+{
+  SET_HARD_REG_BIT (regs->set, R16_REGNUM);
+  SET_HARD_REG_BIT (regs->set, R17_REGNUM);
+  return true;
+}
+
 enum machine_mode
 aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
 {
@@ -8472,6 +8482,11 @@  aarch64_vectorize_vec_perm_const_ok (enum machine_mode vmode,
 #undef TARGET_FIXED_CONDITION_CODE_REGS
 #define TARGET_FIXED_CONDITION_CODE_REGS aarch64_fixed_condition_code_regs
 
+#undef TARGET_FN_OTHER_HARD_REG_USAGE
+#define TARGET_FN_OTHER_HARD_REG_USAGE \
+  aarch64_fn_other_hard_reg_usage
+
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"
-- 
1.8.3.2