On 08/07/2020 09:04, Andre Simoes Dias Vieira wrote:
On 07/07/2020 13:43, Christophe Lyon wrote:
Hi,
On Mon, 6 Jul 2020 at 16:31, Andre Vieira (lists)
<andre.simoesdiasvie...@arm.com> wrote:
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!
Indeed with your patch I don't see any failure with pr95646.c
Note that it is still unsupported with arm-eabi when running the tests
with -mcpu=cortex-mXX
because the compiler complains that -mcpu=cortex-mXX conflicts with
-march=armv8-m.base,
thus the effective-target test fails.
BTW, is that warning useful/practical? Wouldn't it be more convenient
if the last -mcpu/-march
on the command line was the only one taken into account? (I had a
similar issue when
running tests (libstdc++) getting -march=armv8-m.main+fp from their
multilib environment
and forcing -mcpu=cortex-m33 because it also means '+dsp' and produces
a warning;
I had to use -mcpu=cortex-m33 -march=armv8-m.main+fp+dsp to
workaround this)
Yeah I've been annoyed by that before, also in the context of testing
multilibs.
Even though I can see how it can be a useful warning though, if you
are using these in build-systems and you accidentally introduce a new
(incompatible) -mcpu/-march alongside the old one. Though it seems
arbitrary, as we do not warn against multiple -mcpu's or -march's...
I would be in favor of introducing a flag to turn off the warning,
such that we don't have to worry about it in the testsuite.
Unfortunately, it seems -march always wins, regardless of order ....
Meaning that if we do add and use such a warning-off option, we would
also have to always use '-march' as additional-options when creating
testcases, to ensure it overrides any RUNTESTFLAG incompatible -mcpu
or different -march options.
Either way, its a bit of a mess that could probably do with some
cleaning up. One thing at a time though.
Ping.
Still need an OK to commit this patch.
It passes with the config I test without overriding runtestflags flags
though, so it's covered.
Thanks
Christophe
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.
