On 30/06/2020 14:50, Andre Vieira (lists) wrote:

On 29/06/2020 11:15, Christophe Lyon wrote:
On Mon, 29 Jun 2020 at 10:56, Andre Vieira (lists)
<andre.simoesdiasvie...@arm.com> wrote:

On 23/06/2020 21:52, Christophe Lyon wrote:
On Tue, 23 Jun 2020 at 15:28, Andre Vieira (lists)
<andre.simoesdiasvie...@arm.com> wrote:
On 23/06/2020 13:10, Kyrylo Tkachov wrote:
-----Original Message-----
From: Andre Vieira (lists) <andre.simoesdiasvie...@arm.com>
Sent: 22 June 2020 09:52
To: gcc-patches@gcc.gnu.org
Cc: Kyrylo Tkachov <kyrylo.tkac...@arm.com>
Subject: [PATCH][GCC][Arm] PR target/95646: Do not clobber callee saved
registers with CMSE

Hi,

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 trunk? (Will eventually backport to previous versions if
stable.)
Ok.
Thanks,
Kyrill
As I was getting ready to push this I noticed I didn't add any skip-ifs
to prevent this failing with specific target options. So here's a new
version with those.

Still OK?

Hi,

This is not sufficient to skip arm-linux-gnueabi* configs built with
non-default cpu/fpu.

For instance, with arm-linux-gnueabihf --with-cpu=cortex-a9
--with-fpu=neon-fp16 --with-float=hard
I see:
FAIL: gcc.target/arm/pr95646.c (test for excess errors)
Excess errors:
cc1: error: ARMv8-M Security Extensions incompatible with selected FPU
cc1: error: target CPU does not support ARM mode

and the testcase is compiled with -mcpu=cortex-m23 -mcmse -Os
Resending as I don't think my earlier one made it to the lists (sorry if
you are receiving this double!)

I'm not following this, before I go off and try to reproduce it, what do you mean by 'the testcase is compiled with -mcpu=cortex-m23 -mcmse -Os'? These are the options you are seeing in the log file? Surely they should
override the default options? Only thing I can think of is this might
need an extra -mfloat-abi=soft to make sure it overrides the default
float-abi.  Could you give that a try?
No it doesn't make a difference alone.

I also had to add:
-mfpu=auto (that clears the above warning)
-mthumb otherwise we now get cc1: error: target CPU does not support ARM mode

Looks like some effective-target machinery is needed
So I had a look at this,  I was pretty sure that -mfloat-abi=soft overwrote -mfpu=<>, which in large it does, as in no FP instructions will be generated but the error you see only checks for the right number of FP registers. Which doesn't check whether 'TARGET_HARD_FLOAT' is set or not. I'll fix this too and use the check-effective-target for armv8-m.base for this test as it is indeed a better approach than my bag of skip-ifs. I'm testing it locally to make sure my changes don't break anything.

Cheers,
Andre
Hi,

Sorry for the delay. So I changed the test to use the effective-target machinery as you suggested and I also made sure that you don't get the "ARMv8-M Security Extensions incompatible with selected FPU" when -mfloat-abi=soft.
Further changed 'asm' to '__asm__' to avoid failures with '-std=' options.

Regression tested on arm-none-eabi.
@Christophe: could you test this for your configuration, shouldn't fail anymore!

Is this OK for trunk?

Cheers,
Andre

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

        * config/arm/arm.c (arm_options_perform_arch_sanity_checks): Do not
        check +D32 for CMSE if -mfloat-abi=soft

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

        * gcc.target/arm/pr95646.c: Fix testism.

Christophe


Cheers,
Andre
Christophe

Cheers,
Andre
Cheers,
Andre

gcc/ChangeLog:
2020-06-22  Andre Vieira <andre.simoesdiasvie...@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-22  Andre Vieira <andre.simoesdiasvie...@arm.com>

            PR target/95646
            * gcc.target/arm/pr95646.c: New test.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 
dac9a6fb5c41ce42cd7a278b417eab25239a043c..38500220bfb2a1ddbbff15eb552451701f7256d5
 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3834,7 +3834,7 @@ arm_options_perform_arch_sanity_checks (void)
 
   /* We don't clear D16-D31 VFP registers for cmse_nonsecure_call functions
      and ARMv8-M Baseline and Mainline do not allow such configuration.  */
-  if (use_cmse && LAST_VFP_REGNUM > LAST_LO_VFP_REGNUM)
+  if (use_cmse && TARGET_HARD_FLOAT && LAST_VFP_REGNUM > LAST_LO_VFP_REGNUM)
     error ("ARMv8-M Security Extensions incompatible with selected FPU");
 
 
diff --git a/gcc/testsuite/gcc.target/arm/pr95646.c 
b/gcc/testsuite/gcc.target/arm/pr95646.c
index 
12d06a0c8c1ed7de1f8d4d15130432259e613a32..cde1b2d9d36a4e39cd916fdcc9eef424a22bd589
 100644
--- a/gcc/testsuite/gcc.target/arm/pr95646.c
+++ b/gcc/testsuite/gcc.target/arm/pr95646.c
@@ -1,10 +1,7 @@
 /* { 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-require-effective-target arm_arch_v8m_base_ok } */
+/* { dg-add-options arm_arch_v8m_base } */
+/* { dg-additional-options "-mcmse -Os" } */
 /* { dg-final { check-function-bodies "**" "" } } */
 
 int __attribute__ ((cmse_nonsecure_entry))
@@ -27,6 +24,6 @@ foo (void)
 int __attribute__ ((cmse_nonsecure_entry))
 bar (void)
 {
-  asm ("": : : "r9");
+  __asm__ ("" : : : "r9");
   return 1;
 }

Reply via email to