diff mbox series

[GCC-10,backport] arm: PR target/95646: Do not clobber callee saved registers with CMSE.

Message ID VI1PR0802MB23689DE3F255D0D499285EF99B599@VI1PR0802MB2368.eurprd08.prod.outlook.com
State New
Headers show
Series [GCC-10,backport] arm: PR target/95646: Do not clobber callee saved registers with CMSE. | expand

Commit Message

Srinath Parvathaneni May 5, 2021, 1:31 p.m. UTC 
Hi,

This is a backport to gcc-10, cleanly applied on the branch.

As reported in bugzilla when the -mcmse option is used while compiling for size
(-Os) with a thumb-1 target the generated code will clear the registers r7-r10.
These however are callee saved and should be preserved accross ABI boundaries.
The reason this happens is because these registers are made "fixed" when
optimising for size with Thumb-1 in a way to make sure they are not used, as
pushing and popping hi-registers requires extra moves to and from LO_REGS.

To fix this, this patch uses 'callee_saved_reg_p', which accounts for this
optimisation, instead of 'call_used_or_fixed_reg_p'. Be aware of
'callee_saved_reg_p''s definition, as it does still take call used registers
into account, which aren't callee_saved in my opinion, so it is a rather
misnoemer, works in our advantage here though as it does exactly what we need.

Regression tested on arm-none-eabi.

Is this Ok for GCC-10 branch?

Regards,
Srinath.

gcc/ChangeLog:
2020-06-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	PR target/95646
	* config/arm/arm.c: (cmse_nonsecure_entry_clear_before_return): Use
	'callee_saved_reg_p' instead of 'calL_used_or_fixed_reg_p'.

gcc/testsuite/ChangeLog:
2020-06-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	PR target/95646
	* gcc.target/arm/pr95646.c: New test.

(cherry picked from commit 5f426554fd804d65509875d706d8b8bc3a48393b)


###############     Attachment also inlined for ease of reply    ###############
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 781bcc8ca42e10524595cb6c90b61450a41f739e..6f4381fd6e959321d8d319fafdce4079c7b54e5f 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -27011,7 +27011,7 @@ cmse_nonsecure_entry_clear_before_return (void)
 	continue;
       if (IN_RANGE (regno, IP_REGNUM, PC_REGNUM))
 	continue;
-      if (call_used_or_fixed_reg_p (regno)
+      if (!callee_saved_reg_p (regno)
 	  && (!IN_RANGE (regno, FIRST_VFP_REGNUM, LAST_VFP_REGNUM)
 	      || TARGET_HARD_FLOAT))
 	bitmap_set_bit (to_clear_bitmap, regno);
diff --git a/gcc/testsuite/gcc.target/arm/pr95646.c b/gcc/testsuite/gcc.target/arm/pr95646.c
new file mode 100644
index 0000000000000000000000000000000000000000..12d06a0c8c1ed7de1f8d4d15130432259e613a32
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr95646.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv8-m.base" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m23" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mfpu=*" } { } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */
+/* { dg-options "-mcpu=cortex-m23 -mcmse" } */
+/* { dg-additional-options "-Os" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+int __attribute__ ((cmse_nonsecure_entry))
+foo (void)
+{
+  return 1;
+}
+/* { { dg-final { scan-assembler-not "mov\tr9, r0" } } */
+
+/*
+** __acle_se_bar:
+**	mov	(r[0-3]), r9
+**	push	{\1}
+** ...
+**	pop	{(r[0-3])}
+**	mov	r9, \2
+** ...
+**	bxns	lr
+*/
+int __attribute__ ((cmse_nonsecure_entry))
+bar (void)
+{
+  asm ("": : : "r9");
+  return 1;
+}

Comments

Kyrylo Tkachov May 6, 2021, 8:13 a.m. UTC | #1 
> -----Original Message-----
> From: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> Sent: 05 May 2021 14:32
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>
> Subject: [GCC-10 backport][PATCH] arm: PR target/95646: Do not clobber
> callee saved registers with CMSE.
> 
> Hi,
> 
> This is a backport to gcc-10, cleanly applied on the branch.
> 
> As reported in bugzilla when the -mcmse option is used while compiling for
> size
> (-Os) with a thumb-1 target the generated code will clear the registers r7-r10.
> These however are callee saved and should be preserved accross ABI
> boundaries.
> The reason this happens is because these registers are made "fixed" when
> optimising for size with Thumb-1 in a way to make sure they are not used, as
> pushing and popping hi-registers requires extra moves to and from LO_REGS.
> 
> To fix this, this patch uses 'callee_saved_reg_p', which accounts for this
> optimisation, instead of 'call_used_or_fixed_reg_p'. Be aware of
> 'callee_saved_reg_p''s definition, as it does still take call used registers
> into account, which aren't callee_saved in my opinion, so it is a rather
> misnoemer, works in our advantage here though as it does exactly what we
> need.
> 
> Regression tested on arm-none-eabi.
> 
> Is this Ok for GCC-10 branch?

Ok.
Thanks,
Kyrill

> 
> Regards,
> Srinath.
> 
> gcc/ChangeLog:
> 2020-06-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> 
> 	PR target/95646
> 	* config/arm/arm.c: (cmse_nonsecure_entry_clear_before_return):
> Use
> 	'callee_saved_reg_p' instead of 'calL_used_or_fixed_reg_p'.
> 
> gcc/testsuite/ChangeLog:
> 2020-06-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> 
> 	PR target/95646
> 	* gcc.target/arm/pr95646.c: New test.
> 
> (cherry picked from commit 5f426554fd804d65509875d706d8b8bc3a48393b)
> 
> 
> ###############     Attachment also inlined for ease of reply
> ###############
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index
> 781bcc8ca42e10524595cb6c90b61450a41f739e..6f4381fd6e959321d8d319f
> afdce4079c7b54e5f 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -27011,7 +27011,7 @@ cmse_nonsecure_entry_clear_before_return
> (void)
>  	continue;
>        if (IN_RANGE (regno, IP_REGNUM, PC_REGNUM))
>  	continue;
> -      if (call_used_or_fixed_reg_p (regno)
> +      if (!callee_saved_reg_p (regno)
>  	  && (!IN_RANGE (regno, FIRST_VFP_REGNUM, LAST_VFP_REGNUM)
>  	      || TARGET_HARD_FLOAT))
>  	bitmap_set_bit (to_clear_bitmap, regno);
> diff --git a/gcc/testsuite/gcc.target/arm/pr95646.c
> b/gcc/testsuite/gcc.target/arm/pr95646.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..12d06a0c8c1ed7de1f8d4d
> 15130432259e613a32
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr95646.c
> @@ -0,0 +1,32 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-
> march=armv8-m.base" } } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mcpu=*" } { "-
> mcpu=cortex-m23" } } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mfpu=*" } { } }
> */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mfloat-abi=*" }
> { "-mfloat-abi=soft" } } */
> +/* { dg-options "-mcpu=cortex-m23 -mcmse" } */
> +/* { dg-additional-options "-Os" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +int __attribute__ ((cmse_nonsecure_entry))
> +foo (void)
> +{
> +  return 1;
> +}
> +/* { { dg-final { scan-assembler-not "mov\tr9, r0" } } */
> +
> +/*
> +** __acle_se_bar:
> +**	mov	(r[0-3]), r9
> +**	push	{\1}
> +** ...
> +**	pop	{(r[0-3])}
> +**	mov	r9, \2
> +** ...
> +**	bxns	lr
> +*/
> +int __attribute__ ((cmse_nonsecure_entry))
> +bar (void)
> +{
> +  asm ("": : : "r9");
> +  return 1;
> +}
diff mbox series

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 781bcc8ca42e10524595cb6c90b61450a41f739e..6f4381fd6e959321d8d319fafdce4079c7b54e5f 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -27011,7 +27011,7 @@  cmse_nonsecure_entry_clear_before_return (void)
 	continue;
       if (IN_RANGE (regno, IP_REGNUM, PC_REGNUM))
 	continue;
-      if (call_used_or_fixed_reg_p (regno)
+      if (!callee_saved_reg_p (regno)
 	  && (!IN_RANGE (regno, FIRST_VFP_REGNUM, LAST_VFP_REGNUM)
 	      || TARGET_HARD_FLOAT))
 	bitmap_set_bit (to_clear_bitmap, regno);
diff --git a/gcc/testsuite/gcc.target/arm/pr95646.c b/gcc/testsuite/gcc.target/arm/pr95646.c
new file mode 100644
index 0000000000000000000000000000000000000000..12d06a0c8c1ed7de1f8d4d15130432259e613a32
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr95646.c
@@ -0,0 +1,32 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv8-m.base" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m23" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mfpu=*" } { } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */
+/* { dg-options "-mcpu=cortex-m23 -mcmse" } */
+/* { dg-additional-options "-Os" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+int __attribute__ ((cmse_nonsecure_entry))
+foo (void)
+{
+  return 1;
+}
+/* { { dg-final { scan-assembler-not "mov\tr9, r0" } } */
+
+/*
+** __acle_se_bar:
+**	mov	(r[0-3]), r9
+**	push	{\1}
+** ...
+**	pop	{(r[0-3])}
+**	mov	r9, \2
+** ...
+**	bxns	lr
+*/
+int __attribute__ ((cmse_nonsecure_entry))
+bar (void)
+{
+  asm ("": : : "r9");
+  return 1;
+}