[PING] AArch64: add R30_REGNUM into shrink-wrapping separate

Message ID b34a5ba2-240d-c5f3-5723-bfbfc6c9f859@linux.alibaba.com
State New
Headers
Series [PING] AArch64: add R30_REGNUM into shrink-wrapping separate |

Commit Message

Dan Li March 14, 2022, 8:40 a.m. UTC
  Gentile ping for this :), thanks.

Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590906.html

> R30_REGNUM could also be used as a component in shrink-wrapping
> separate, this patch enables it in aarch64.
> 
> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64.cc (aarch64_get_separate_components):
> 	Remove bitmap clear of R30_REGNUM.
> 	(aarch64_components_for_bb): Support R30_REGNUM as a component.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/aarch64/shrink_wrap_separate_1.c: New test.

---
  gcc/config/aarch64/aarch64.cc                   |  4 ++--
  .../gcc.target/aarch64/shrink_wrap_separate_1.c | 17 +++++++++++++++++
  2 files changed, 19 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c
  

Comments

Richard Sandiford April 12, 2022, 1:05 p.m. UTC | #1
Dan Li <ashimida@linux.alibaba.com> writes:
> Gentile ping for this :), thanks.
>
> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590906.html

Sorry, I should have realised this at the time, but I don't think
we can do this after all.  The ABI requires us to set up the frame
chain before assigning to the frame pointer.

Sorry again for the bogus suggestion.

Thanks,
Richard

>
>> R30_REGNUM could also be used as a component in shrink-wrapping
>> separate, this patch enables it in aarch64.
>> 
>> gcc/ChangeLog:
>> 
>> 	* config/aarch64/aarch64.cc (aarch64_get_separate_components):
>> 	Remove bitmap clear of R30_REGNUM.
>> 	(aarch64_components_for_bb): Support R30_REGNUM as a component.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.target/aarch64/shrink_wrap_separate_1.c: New test.
>
> ---
>   gcc/config/aarch64/aarch64.cc                   |  4 ++--
>   .../gcc.target/aarch64/shrink_wrap_separate_1.c | 17 +++++++++++++++++
>   2 files changed, 19 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 8bcee8be9eb..6e1589b0312 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -8463,7 +8463,6 @@ aarch64_get_separate_components (void)
>     if (reg1 != INVALID_REGNUM)
>       bitmap_clear_bit (components, reg1);
>   
> -  bitmap_clear_bit (components, LR_REGNUM);
>     bitmap_clear_bit (components, SP_REGNUM);
>   
>     return components;
> @@ -8500,7 +8499,8 @@ aarch64_components_for_bb (basic_block bb)
>     /* GPRs are used in a bb if they are in the IN, GEN, or KILL sets.  */
>     for (unsigned regno = 0; regno <= LAST_SAVED_REGNUM; regno++)
>       if (!fixed_regs[regno]
> -	&& !crtl->abi->clobbers_full_reg_p (regno)
> +	&& (regno == R30_REGNUM
> +	    || !crtl->abi->clobbers_full_reg_p (regno))
>   	&& (TEST_HARD_REG_BIT (extra_caller_saves, regno)
>   	    || bitmap_bit_p (in, regno)
>   	    || bitmap_bit_p (gen, regno)
> diff --git a/gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c b/gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c
> new file mode 100644
> index 00000000000..34002705ace
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fomit-frame-pointer -fdump-rtl-pro_and_epilogue" } */
> +
> +void f();
> +
> +int g(int x)
> +{
> +  if (x == 0)
> +    {
> +      __asm__ ("":::"x19", "x20");
> +      return 1;
> +    }
> +  f();
> +  return 2;
> +}
> +
> +/* { dg-final { scan-rtl-dump {The components we wrap separately are \[sep 30\]} "pro_and_epilogue"  } } */
  
Dan Li April 15, 2022, 9:17 a.m. UTC | #2
On 4/12/22 06:05, Richard Sandiford wrote:
> Dan Li <ashimida@linux.alibaba.com> writes:
>> Gentile ping for this :), thanks.
>>
>> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590906.html
> 
> Sorry, I should have realised this at the time, but I don't think
> we can do this after all.  The ABI requires us to set up the frame
> chain before assigning to the frame pointer.
> 
> Sorry again for the bogus suggestion.
> 
> Thanks,
> Richard
> 

OK, thanks Richard :)

But I think I still don't quite understand it, so could the x30 be
shrinked alone when we don't need the frame chain?

Thanks,
Dan.
  

Patch

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 8bcee8be9eb..6e1589b0312 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -8463,7 +8463,6 @@  aarch64_get_separate_components (void)
    if (reg1 != INVALID_REGNUM)
      bitmap_clear_bit (components, reg1);
  
-  bitmap_clear_bit (components, LR_REGNUM);
    bitmap_clear_bit (components, SP_REGNUM);
  
    return components;
@@ -8500,7 +8499,8 @@  aarch64_components_for_bb (basic_block bb)
    /* GPRs are used in a bb if they are in the IN, GEN, or KILL sets.  */
    for (unsigned regno = 0; regno <= LAST_SAVED_REGNUM; regno++)
      if (!fixed_regs[regno]
-	&& !crtl->abi->clobbers_full_reg_p (regno)
+	&& (regno == R30_REGNUM
+	    || !crtl->abi->clobbers_full_reg_p (regno))
  	&& (TEST_HARD_REG_BIT (extra_caller_saves, regno)
  	    || bitmap_bit_p (in, regno)
  	    || bitmap_bit_p (gen, regno)
diff --git a/gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c b/gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c
new file mode 100644
index 00000000000..34002705ace
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/shrink_wrap_separate_1.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fomit-frame-pointer -fdump-rtl-pro_and_epilogue" } */
+
+void f();
+
+int g(int x)
+{
+  if (x == 0)
+    {
+      __asm__ ("":::"x19", "x20");
+      return 1;
+    }
+  f();
+  return 2;
+}
+
+/* { dg-final { scan-rtl-dump {The components we wrap separately are \[sep 30\]} "pro_and_epilogue"  } } */