On Fri, 1 May 2020 at 12:57, Kyrylo Tkachov <kyrylo.tkac...@arm.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Christophe Lyon <christophe.l...@linaro.org>
> > Sent: 30 April 2020 09:51
> > To: Kyrylo Tkachov <kyrylo.tkac...@arm.com>
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: arm: Fix vfp_operand_register for VFP HI regs
> >
> > On Wed, 29 Apr 2020 at 18:40, Kyrylo Tkachov <kyrylo.tkac...@arm.com>
> > wrote:
> > >
> > > Hi Christophe,
> > >
> > > > -----Original Message-----
> > > > From: Gcc-patches <gcc-patches-boun...@gcc.gnu.org> On Behalf Of
> > > > Christophe Lyon via Gcc-patches
> > > > Sent: 29 April 2020 16:53
> > > > To: gcc Patches <gcc-patches@gcc.gnu.org>
> > > > Subject: arm: Fix vfp_operand_register for VFP HI regs
> > > >
> > > > Hi,
> > > >
> > > > While looking at PR target/94743 I noticed an ICE when I tried to save
> > > > all the FP registers: this was because all HI registers wouldn't match
> > > > vfp_register_operand.
> > >
> > > Hmm, I see that arm_regno_class indeed never returns VFP_REGS and
> > would return VFP_HI_REGS here.
> > > So the patch looks correct to me.
> > > Do you have a testcase for the ICE to add to the testsuite?
> > >
> >
> > No C source code: I found that while extending the list of registers
> > pushed in the prologue of an IRQ handler, more-or-less modifying
> > arm_save_coproc_regs so that more registers are handled by
> > vfp_emit_fstmd.
> > The problem occurs when trying to push d16-d31.
>
> I'd be comfortable taking this now for trunk (GCC 11) so it has time to bake.
> Once you're ready to post the IRQ handler work we can see about backporting
> this fix it to the branch, if we deem it necessary.
> Thanks,
> Kyrill
>
Hi,
I've just sent several patches for PR 94743:
https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545747.html
is v2 of my previous patch: it only emits a warning, and might be
sufficient to close the PR, if we decide that
we don't want to save the FP registers without explicit user request...
Then,
[1] https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545748.html
[2] https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545749.html
are refactorization patches that would make implementing the patch
attached here easier.
If you apply the attached on top of [1] and [2], you'll notice an ICE
fixed by the original patch of this thread.
So hopefully applying [1], [2] and the attached should help convince you that
the vfp_operand_register is OK.
Thanks
> >
> >
> > > Thanks,
> > > Kyrill
> > >
> > > >
> > > > Regression-tested and bootstrapped OK.
> > > >
> > > > 2020-04-29 Christophe Lyon <christophe.l...@linaro.org>
> > > >
> > > > gcc/
> > > > * config/arm/predicates.md (vfp_register_operand): Use
> > VFP_HI_REGS
> > > > instead of VFP_REGS.
> > > >
> > > > OK?
> > > >
> > > > Thanks,
> > > >
> > > > Christophe
From 834868c176adbec0f84b71289c2fbae7500b3fce Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.l...@linaro.org>
Date: Thu, 14 May 2020 11:14:59 +0000
Subject: [PATCH] arm: Save FP regs in interrupt handlers [PR target/94743]
2020-05-xx Christophe Lyon <christophe.l...@linaro.org>
gcc/
* config/arm/arm.c (reg_needs_saving_p): Remove limit to core
registers.
gcc/testsuite/
* gcc.target/arm/pr94743-4.c: New test.
* gcc.target/arm/pr94743-5.c: New test.
---
gcc/config/arm/arm.c | 4 ++--
gcc/testsuite/gcc.target/arm/pr94743-4.c | 31 ++++++++++++++++++++++++++++
gcc/testsuite/gcc.target/arm/pr94743-5.c | 35 ++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arm/pr94743-4.c
create mode 100644 gcc/testsuite/gcc.target/arm/pr94743-5.c
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 0107f50..38413fb 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -4200,8 +4200,8 @@ static bool reg_needs_saving_p (unsigned reg)
if (IS_INTERRUPT (func_type))
if (df_regs_ever_live_p (reg)
- /* Save call-clobbered core registers. */
- || (! crtl->is_leaf && call_used_or_fixed_reg_p (reg) && reg < FIRST_VFP_REGNUM))
+ /* Save call-clobbered registers. */
+ || (! crtl->is_leaf && call_used_or_fixed_reg_p (reg)))
return true;
else
return false;
diff --git a/gcc/testsuite/gcc.target/arm/pr94743-4.c b/gcc/testsuite/gcc.target/arm/pr94743-4.c
new file mode 100644
index 0000000..378981a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr94743-4.c
@@ -0,0 +1,31 @@
+/* PR target/94743 */
+/* { dg-do compile } */
+/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } {"-mfloat-abi=hard" } } */
+/* { dg-options "-mfloat-abi=hard" } */
+
+/* Check that we emit a warning when compiling an IRQ handler without
+ -mgeneral-regs-only. */
+/* Also check that we save and restore the VFP registers clobbered
+ locally. */
+typedef struct {
+ double fpdata[32];
+} dummy_t;
+
+dummy_t global_d;
+dummy_t global_d1;
+
+/* This function may clobber VFP registers, but it is a leaf function:
+ we can analyze which registers to save. */
+__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test(void)
+{ /* { dg-warning { FP registers might be clobbered despite 'interrupt' attribute: compile with -mgeneral-regs-only} "" { target *-*-* } . } */
+ global_d.fpdata[3] += global_d.fpdata[3] * global_d1.fpdata[3];
+}
+/* { dg-final { scan-assembler-times "vpush.64\t{d\\d+, d\\d+, d\\d+}" 1 } } */
+/* { dg-final { scan-assembler-times "vldm\tsp!, {d\\d+-d\\d+}" 1 } } */
+
+/* This function does not need to clobber VFP registers. */
+/* Do we want to emit a (useless?) warning? */
+__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test2(void)
+{ /* { dg-warning { FP registers might be clobbered despite 'interrupt' attribute: compile with -mgeneral-regs-only} "" { target *-*-* } . } */
+ global_d.fpdata[3] = 1.0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/pr94743-5.c b/gcc/testsuite/gcc.target/arm/pr94743-5.c
new file mode 100644
index 0000000..55fb1dd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr94743-5.c
@@ -0,0 +1,35 @@
+/* PR target/94743 */
+/* { dg-do compile } */
+/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } {"-mfloat-abi=hard" } } */
+/* Require Neon so that we push d16-d31. */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-add-options arm_neon } */
+/* { dg-additional-options "-mfloat-abi=hard" } */
+
+/* Check that we emit a warning when compiling an IRQ handler without
+ -mgeneral-regs-only. */
+/* Also check that we save and restore the call-clobbered VFP registers. */
+typedef struct {
+ double fpdata[32];
+} dummy_t;
+
+dummy_t global_d;
+dummy_t global_d1;
+
+/* This function may clobber VFP registers. */
+__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test(void)
+{ /* { dg-warning { FP registers might be clobbered despite 'interrupt' attribute: compile with -mgeneral-regs-only} "" { target *-*-* } . } */
+ /* Force implicit call to memcpy. */
+ global_d = global_d1;
+ global_d.fpdata[3] += global_d.fpdata[3] * global_d1.fpdata[3];
+}
+/* { dg-final { scan-assembler-times "vpush.64\t{d0, d1, d2, d3, d4, d5, d6, d7}" 1 } } */
+/* { dg-final { scan-assembler-times "vpush.64\t{d16, d17, d18, d19, d20, d21, d22, d23, d24, d25, d26, d27, d28, d29, d30, d31}" 1 } } */
+/* { dg-final { scan-assembler-times "vldm\tsp!, {d16-d31}" 1 } } */
+/* { dg-final { scan-assembler-times "vldm\tsp!, {d0-d7}" 1 } } */
+
+/* This function does not need to clobber VFP registers. */
+__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test2(void)
+{ /* { dg-warning { FP registers might be clobbered despite 'interrupt' attribute: compile with -mgeneral-regs-only} "" { target *-*-* } . } */
+ global_d.fpdata[3] = 1.0;
+}
--
2.7.4
