This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH, rs6000][GCC6] Fix PR78543, ICE in push_reload on powerpc64le-linux-gnu
- From: Peter Bergner <bergner at vnet dot ibm dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Segher Boessenkool <segher at kernel dot crashing dot org>, David Edelsohn <dje dot gcc at gmail dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>, Michael Meissner <meissner at linux dot vnet dot ibm dot com>, Matthias Klose <doko at ubuntu dot com>
- Date: Mon, 6 Mar 2017 12:16:58 -0600
- Subject: [PATCH, rs6000][GCC6] Fix PR78543, ICE in push_reload on powerpc64le-linux-gnu
- Authentication-results: sourceware.org; auth=none
PR78543 has two related test cases that have similar insns that need
reloading (pseudo 185 in this case) due to spills:
(insn 142 144 146 20 (parallel [
(set (reg:HI 4 4 [orig:260 p ] [260])
(bswap:HI (subreg/s/v:HI (reg:DI 185 [ load_dst_59 ]) 0)))
(clobber (scratch:SI))
]) pr78543.i:23 144 {bswaphi2_internal}
(expr_list:REG_DEAD (reg:DI 185 [ load_dst_59 ])
(nil)))
In simplify_subreg, we execute the following code:
/* If we have a SUBREG of a register that we are replacing and we are
replacing it with a MEM, make a new MEM and try replacing the
SUBREG with it. Don't do this if the MEM has a mode-dependent address
or if we would be widening it. */
if (MEM_P (op)
&& ! mode_dependent_address_p (XEXP (op, 0), MEM_ADDR_SPACE (op))
/* Allow splitting of volatile memory references in case we don't
have instruction to move the whole thing. */
&& (! MEM_VOLATILE_P (op)
|| ! have_insn_for (SET, innermode))
&& GET_MODE_SIZE (outermode) <= GET_MODE_SIZE (GET_MODE (op)))
return adjust_address_nv (op, outermode, byte);
The address we pass to mode_dependent_address_p() is:
(plus:DI (reg/f:DI 1 1)
(const_int 65648 [0x10070]))
...which rs6000_mode_dependent_address() says *is* mode dependent, so we
do not end up calling adjust_address_nv() to adjust the stack address.
This ends up leaving us with the followng insn to handle:
(subreg/s/v:HI (mem/c:DI (plus:DI (plus:DI (reg/f:DI 1 1)
(const_int 65536 [0x10000]))
(const_int 112 [0x70])) [5 %sfp+65648 S8 A64]) 0)
Which triggers the gcc_assert at reload.c:1348:
/* Optional output reloads are always OK even if we have no register class,
since the function of these reloads is only to have spill_reg_store etc.
set, so that the storing insn can be deleted later. */
gcc_assert (rclass != NO_REGS
|| (optional != 0 && type == RELOAD_FOR_OUTPUT));
where: rclass = NO_REGS, optional = 0, type = RELOAD_FOR_INPUT
If we look at rs6000_mode_dependent_address(), it accepts some addresses
as not being mode dependent:
case PLUS:
/* Any offset from virtual_stack_vars_rtx and arg_pointer_rtx
is considered a legitimate address before reload, so there
are no offset restrictions in that case. Note that this
condition is safe in strict mode because any address involving
virtual_stack_vars_rtx or arg_pointer_rtx would already have
been rejected as illegitimate. */
if (XEXP (addr, 0) != virtual_stack_vars_rtx
&& XEXP (addr, 0) != arg_pointer_rtx
&& GET_CODE (XEXP (addr, 1)) == CONST_INT)
{
unsigned HOST_WIDE_INT val = INTVAL (XEXP (addr, 1));
return val + 0x8000 >= 0x10000 - (TARGET_POWERPC64 ? 8 : 12);
}
break;
It seems wrong that we accept addresses based off virtual_stack_vars_rtx
and arg_pointer_rtx, but not stack_pointer_rtx (ie, r1) like we have in
these test cases. Adding a change to accept stack_pointer_rtx, allows
us to call adjust_address_nv() which ends up transforming the:
(mem:DI (plus (reg:1) (const_int 65648)))
into:
(mem:HI (plus (reg:1) (const_int 65648)))
allowing us to eliminate the subreg insn below:
(subreg/s/v:HI (mem/c:DI (plus:DI (plus:DI (reg/f:DI 1 1)
(const_int 65536 [0x10000]))
(const_int 112 [0x70])) [5 %sfp+65648 S8 A64]) 0)
which solves our ICE.
The following patch passes bootstrap and regtesting on powerpc64le-linux.
Ok for the GCC 6 branch?
We don't hit this on trunk, because we're using LRA there, so I'm not
sure whether we want to add this there this late in the release cycle.
Peter
gcc/
PR target/78543
* config/rs6000/rs6000.c (rs6000_mode_dependent_address): Handle
stack_pointer_rtx.
gcc/testsuite/
PR target/78543
* gcc.target/powerpc/pr78543.c: New test.
* gcc.target/powerpc/pr78543-2.c: Likewise.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 245904)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -8988,6 +8988,7 @@
been rejected as illegitimate. */
if (XEXP (addr, 0) != virtual_stack_vars_rtx
&& XEXP (addr, 0) != arg_pointer_rtx
+ && XEXP (addr, 0) != stack_pointer_rtx
&& GET_CODE (XEXP (addr, 1)) == CONST_INT)
{
unsigned HOST_WIDE_INT val = INTVAL (XEXP (addr, 1));
Index: gcc/testsuite/gcc.target/powerpc/pr78543-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr78543-2.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr78543-2.c (working copy)
@@ -0,0 +1,60 @@
+/* PR target/78543 */
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-O1 -mcpu=power8" } */
+
+typedef long a;
+enum c { e, f, g, h, i, ab } j();
+int l, n, o, p;
+a q, r;
+void *memcpy();
+void b();
+static int k(int *s) {
+ int m;
+ if (j(&m))
+ *s = m;
+ return !0;
+}
+void d(char s) {
+ int af[4];
+ int ag;
+ enum c ah;
+ char ai[24 << 11];
+ unsigned aj;
+ if (!k(&aj))
+ goto ak;
+ for (;;) {
+ if (!k(&ag))
+ goto ak;
+ switch (ah) {
+ case e:
+ b("");
+ b("bad length %d for GUID in fileinfo v%u for \"%s\"");
+ case i:
+ b("bad length %d for TTH in fileinfo v%u for \"%s\"", aj);
+ case ab:
+ if (ag % 24)
+ b("for \"%s\"", s);
+ case f:
+ if (20 == ag)
+ case h:
+ if (20 == ag)
+ o = 0;
+ break;
+ case g:
+ memcpy(af, ai, sizeof af);
+ b();
+ if (p) {
+ a al, am;
+ r = al << 2 | am;
+ n = af[2];
+ al = n;
+ l = __builtin_bswap32(af[3]);
+ am = q = n | l;
+ }
+ default:
+ b("%s0 unhandled field ID %u 0", __func__);
+ }
+ }
+ak:;
+}
Index: gcc/testsuite/gcc.target/powerpc/pr78543.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr78543.c (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr78543.c (working copy)
@@ -0,0 +1,40 @@
+/* PR target/78543 */
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-O3 -mcpu=power8" } */
+
+int a, b, c, d, e, f = 0, g, i;
+int h[4];
+extern void fn2();
+extern int fn3();
+extern void fn4();
+extern void fn5();
+void
+fn1 (void)
+{
+ unsigned short j;
+ int k = c;
+ char l[65535];
+ if (g && h[e])
+ fn2();
+ for (; h[e]; e--)
+ if (a)
+ return;
+ for (short n = h[0]; n; n--) {
+ a = fn3(d, l, sizeof l) < 0;
+ if (a)
+ return;
+ char m;
+ short o = 0, p = ({
+ j = i;
+ j >> 8 | (j & 255) << 8;
+ });
+ long q = b;
+ if (g && h[0]) {
+ fn2(k, "%7d", "%7", &o, "%7 ", f);
+ while (b < q)
+ fn4();
+ fn5(m, p, "");
+ }
+ }
+}