diff mbox

[ARM/AArch64] PR 68088: Fix RTL checking ICE due to subregs inside accumulator forwarding check

Message ID 563229DD.8080900@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov Oct. 29, 2015, 2:14 p.m. UTC
On 29/10/15 14:00, Marcus Shawcroft wrote:
> On 29 October 2015 at 13:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

>

>>>> Ok for trunk?

>>> rtl.h exposes reg_or_subregno() already doesn't that do what we need here?

>>

>> reg_or_subregno assumes that what it's passed is REG or a SUBREG.

>> It will ICE on any other rtx. Here I want to strip the subreg if it is

>> a subreg, but leave it as it is otherwise.

> OK, I follow.

>

>>> The test case is not aarch64 specific therefore I think convention is

>>> that it should go into a generic directory.

>>

>> Ok, I'll put it in gcc.dg/

>

> OK with the test case moved. Thanks /Marcus

>

Thanks, but I'd like to do a slight respin.
The testcase is moved to gcc.dg but I also avoid creating the new
helper function and just do the SUBREG extraction once at the very end.
This makes the patch smaller.

Since you're ok with the approach and this revision is logically equivalent,
I just need an ok from an arm perspective.

Thanks,
Kyrill

2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/68088
     * config/arm/aarch-common.c (aarch_accumulator_forwarding): Strip
     subregs from accumulator and make sure it's a register.

2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/68088
     * gcc.dg/pr68088_1.c: New test.

Comments

Kyrylo Tkachov Nov. 6, 2015, 11:39 a.m. UTC | #1
Ping.
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03170.html

Thanks,
Kyrill

On 29/10/15 14:14, Kyrill Tkachov wrote:
>

> On 29/10/15 14:00, Marcus Shawcroft wrote:

>> On 29 October 2015 at 13:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

>>

>>>>> Ok for trunk?

>>>> rtl.h exposes reg_or_subregno() already doesn't that do what we need here?

>>>

>>> reg_or_subregno assumes that what it's passed is REG or a SUBREG.

>>> It will ICE on any other rtx. Here I want to strip the subreg if it is

>>> a subreg, but leave it as it is otherwise.

>> OK, I follow.

>>

>>>> The test case is not aarch64 specific therefore I think convention is

>>>> that it should go into a generic directory.

>>>

>>> Ok, I'll put it in gcc.dg/

>>

>> OK with the test case moved. Thanks /Marcus

>>

> Thanks, but I'd like to do a slight respin.

> The testcase is moved to gcc.dg but I also avoid creating the new

> helper function and just do the SUBREG extraction once at the very end.

> This makes the patch smaller.

>

> Since you're ok with the approach and this revision is logically equivalent,

> I just need an ok from an arm perspective.

>

> Thanks,

> Kyrill

>

> 2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     PR target/68088

>     * config/arm/aarch-common.c (aarch_accumulator_forwarding): Strip

>     subregs from accumulator and make sure it's a register.

>

> 2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     PR target/68088

>     * gcc.dg/pr68088_1.c: New test.

>
diff mbox

Patch

commit aa4df340968b330544edbbb8ea706f2d56011381
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Oct 27 11:42:19 2015 +0000

    [ARM/AArch64] PR 68088: Fix RTL checking ICE due to subregs inside accumulator forwarding check

diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
index a940a02..e6668d5 100644
--- a/gcc/config/arm/aarch-common.c
+++ b/gcc/config/arm/aarch-common.c
@@ -460,6 +460,12 @@  aarch_accumulator_forwarding (rtx_insn *producer, rtx_insn *consumer)
 	return 0;
     }
 
+  if (GET_CODE (accumulator) == SUBREG)
+    accumulator = SUBREG_REG (accumulator);
+
+  if (!REG_P (accumulator))
+    return 0;
+
   return (REGNO (dest) == REGNO (accumulator));
 }
 
diff --git a/gcc/testsuite/gcc.dg/pr68088_1.c b/gcc/testsuite/gcc.dg/pr68088_1.c
new file mode 100644
index 0000000..49c6aa1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr68088_1.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void bar (unsigned long);
+
+void
+foo (unsigned long aul, unsigned m, unsigned i)
+{
+  while (1)
+    {
+      aul += i;
+      i = aul % m;
+      bar (aul);
+    }
+}