arm: Fix vfp_operand_register for VFP HI regs

arm: Fix vfp_operand_register for VFP HI regs

[Christophe Lyon]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

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.

Regression-tested and bootstrapped OK.

2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>

        gcc/
        * config/arm/predicates.md (vfp_register_operand): Use VFP_HI_REGS
        instead of VFP_REGS.

OK?

Thanks,

Christophe

[-- Attachment #2: vfp-predicate.chlog.txt --]
[-- Type: text/plain, Size: 320 bytes --]

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.

2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>

	gcc/
	* config/arm/predicates.md (vfp_register_operand): Use VFP_HI_REGS
	instead of VFP_REGS.

[-- Attachment #3: vfp-predicate.patch.txt --]
[-- Type: text/plain, Size: 535 bytes --]

diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index 009862e..dbd4fb4 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -161,7 +161,7 @@ (define_predicate "vfp_register_operand"
 	      || REGNO_REG_CLASS (REGNO (op)) == VFP_D0_D7_REGS
 	      || REGNO_REG_CLASS (REGNO (op)) == VFP_LO_REGS
 	      || (TARGET_VFPD32
-		  && REGNO_REG_CLASS (REGNO (op)) == VFP_REGS)));
+		  && REGNO_REG_CLASS (REGNO (op)) == VFP_HI_REGS)));
 })
 
 (define_predicate "vfp_hard_register_operand"

RE: arm: Fix vfp_operand_register for VFP HI regs

[Kyrylo Tkachov]

Hi Christophe,

> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces@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?

Thanks,
Kyrill

> 
> Regression-tested and bootstrapped OK.
> 
> 2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>
> 
>         gcc/
>         * config/arm/predicates.md (vfp_register_operand): Use VFP_HI_REGS
>         instead of VFP_REGS.
> 
> OK?
> 
> Thanks,
> 
> Christophe

Re: arm: Fix vfp_operand_register for VFP HI regs

[Christophe Lyon]

On Wed, 29 Apr 2020 at 18:40, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote:
>
> Hi Christophe,
>
> > -----Original Message-----
> > From: Gcc-patches <gcc-patches-bounces@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.


> Thanks,
> Kyrill
>
> >
> > Regression-tested and bootstrapped OK.
> >
> > 2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>
> >
> >         gcc/
> >         * config/arm/predicates.md (vfp_register_operand): Use VFP_HI_REGS
> >         instead of VFP_REGS.
> >
> > OK?
> >
> > Thanks,
> >
> > Christophe

RE: arm: Fix vfp_operand_register for VFP HI regs

[Kyrylo Tkachov]

> -----Original Message-----
> From: Christophe Lyon <christophe.lyon@linaro.org>
> Sent: 30 April 2020 09:51
> To: Kyrylo Tkachov <Kyrylo.Tkachov@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.Tkachov@arm.com>
> wrote:
> >
> > Hi Christophe,
> >
> > > -----Original Message-----
> > > From: Gcc-patches <gcc-patches-bounces@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

> 
> 
> > Thanks,
> > Kyrill
> >
> > >
> > > Regression-tested and bootstrapped OK.
> > >
> > > 2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>
> > >
> > >         gcc/
> > >         * config/arm/predicates.md (vfp_register_operand): Use
> VFP_HI_REGS
> > >         instead of VFP_REGS.
> > >
> > > OK?
> > >
> > > Thanks,
> > >
> > > Christophe

Re: arm: Fix vfp_operand_register for VFP HI regs

[Christophe Lyon]

On Fri, 1 May 2020 at 12:57, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Sent: 30 April 2020 09:51
> > To: Kyrylo Tkachov <Kyrylo.Tkachov@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.Tkachov@arm.com>
> > wrote:
> > >
> > > Hi Christophe,
> > >
> > > > -----Original Message-----
> > > > From: Gcc-patches <gcc-patches-bounces@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
>

OK, I hoped my simple-warning patch could be included in gcc-10 but I
think it's now too late.
(https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544872.html)

I'm not quite sure about the next steps, but let's continue the
discussion in bugzilla.

Thanks

> >
> >
> > > Thanks,
> > > Kyrill
> > >
> > > >
> > > > Regression-tested and bootstrapped OK.
> > > >
> > > > 2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>
> > > >
> > > >         gcc/
> > > >         * config/arm/predicates.md (vfp_register_operand): Use
> > VFP_HI_REGS
> > > >         instead of VFP_REGS.
> > > >
> > > > OK?
> > > >
> > > > Thanks,
> > > >
> > > > Christophe

Re: arm: Fix vfp_operand_register for VFP HI regs

[Christophe Lyon]

[-- Attachment #1: Type: text/plain, Size: 2870 bytes --]

On Fri, 1 May 2020 at 12:57, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Sent: 30 April 2020 09:51
> > To: Kyrylo Tkachov <Kyrylo.Tkachov@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.Tkachov@arm.com>
> > wrote:
> > >
> > > Hi Christophe,
> > >
> > > > -----Original Message-----
> > > > From: Gcc-patches <gcc-patches-bounces@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.lyon@linaro.org>
> > > >
> > > >         gcc/
> > > >         * config/arm/predicates.md (vfp_register_operand): Use
> > VFP_HI_REGS
> > > >         instead of VFP_REGS.
> > > >
> > > > OK?
> > > >
> > > > Thanks,
> > > >
> > > > Christophe

[-- Attachment #2: 0001-arm-Save-FP-regs-in-interrupt-handlers-PR-target-947.patch --]
[-- Type: text/x-patch, Size: 4920 bytes --]

From 834868c176adbec0f84b71289c2fbae7500b3fce Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.lyon@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.lyon@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

Re: arm: Fix vfp_operand_register for VFP HI regs

[Christophe Lyon]

Hi,


On Thu, 14 May 2020 at 17:08, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
>
> On Fri, 1 May 2020 at 12:57, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Christophe Lyon <christophe.lyon@linaro.org>
> > > Sent: 30 April 2020 09:51
> > > To: Kyrylo Tkachov <Kyrylo.Tkachov@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.Tkachov@arm.com>
> > > wrote:
> > > >
> > > > Hi Christophe,
> > > >
> > > > > -----Original Message-----
> > > > > From: Gcc-patches <gcc-patches-bounces@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.
>

I've committed [1] and [2] several days ago, so it's now easier to apply
the patch from
https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545754.html
which will trigger an ICE in the new testcases it adds,
which is fixed by the patch that started this thread:
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544877.html

Is that lalter patch OK?

Thanks,

Christophe

> Thanks
>
> > >
> > >
> > > > Thanks,
> > > > Kyrill
> > > >
> > > > >
> > > > > Regression-tested and bootstrapped OK.
> > > > >
> > > > > 2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>
> > > > >
> > > > >         gcc/
> > > > >         * config/arm/predicates.md (vfp_register_operand): Use
> > > VFP_HI_REGS
> > > > >         instead of VFP_REGS.
> > > > >
> > > > > OK?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Christophe

RE: arm: Fix vfp_operand_register for VFP HI regs

[Kyrylo Tkachov]

Hi Christophe,

> -----Original Message-----
> From: Christophe Lyon <christophe.lyon@linaro.org>
> Sent: 03 June 2020 14:30
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: arm: Fix vfp_operand_register for VFP HI regs
> 
> Hi,
> 
> 
> On Thu, 14 May 2020 at 17:08, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
> >
> > On Fri, 1 May 2020 at 12:57, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Christophe Lyon <christophe.lyon@linaro.org>
> > > > Sent: 30 April 2020 09:51
> > > > To: Kyrylo Tkachov <Kyrylo.Tkachov@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.Tkachov@arm.com>
> > > > wrote:
> > > > >
> > > > > Hi Christophe,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Gcc-patches <gcc-patches-bounces@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.
> >
> 
> I've committed [1] and [2] several days ago, so it's now easier to apply
> the patch from
> https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545754.html
> which will trigger an ICE in the new testcases it adds,
> which is fixed by the patch that started this thread:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544877.html

This is okay.
Thanks,
Kyrill

> 
> Is that lalter patch OK?
> 
> Thanks,
> 
> Christophe
> 
> > Thanks
> >
> > > >
> > > >
> > > > > Thanks,
> > > > > Kyrill
> > > > >
> > > > > >
> > > > > > Regression-tested and bootstrapped OK.
> > > > > >
> > > > > > 2020-04-29  Christophe Lyon  <christophe.lyon@linaro.org>
> > > > > >
> > > > > >         gcc/
> > > > > >         * config/arm/predicates.md (vfp_register_operand): Use
> > > > VFP_HI_REGS
> > > > > >         instead of VFP_REGS.
> > > > > >
> > > > > > OK?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Christophe