[PATCH][RFC] fix reload causing ICE in subreg_get_info on m68k (PR58369)

[Mikael Pettersson]

Jeff Law writes:
 > On 09/28/13 09:30, Mikael Pettersson wrote:
 > > This patch fixes PR58369, an ICE in subreg_get_info when compiling
 > > boost for m68k-linux.
 > >
 > > choose_reload_regs attempts to reload a DFmode (8-byte) reg, finds
 > > an XFmode (12-byte) reg in "last_reg", and calls subreg_regno_offset
 > > with these two modes and a subreg offset of zero.  However, this is
 > > not a correct lowpart subreg offset for big-endian and these two modes,
 > > so the lowpart subreg check in subreg_get_info fails, and the code
 > > continues to
 > >
 > >      gcc_assert ((GET_MODE_SIZE (xmode) % GET_MODE_SIZE (ymode)) == 0);
 > >
 > > which fails because (12 % 8) != 0.
 > >
 > > choose_reload_regs passes the constant zero, in all cases where the reg
 > > isn't already a subreg, as the subreg offset to subreg_regno_offset, even
 > > though lowpart subregs on big-endian targets require an explicit offset
 > > computation.  I think that is a bug.
 > >
 > > I believe other big-endian targets don't see this ICE because
 > > a) they define CANNOT_CHANGE_MODE_CLASS to reject differently-sized
 > >     modes in floating-point registers (which prevents this path in
 > >     choose_reload_regs), or
 > > b) their differently-sized modes are such that the size of a larger
 > >     mode is a whole multiple of the size of the smaller mode (which
 > >     allows the gcc_assert above to pass).
 > >
 > > This patch changes choose_reload_regs to call subreg_lowpart_offset
 > > to pass an endian-correct offset to subreg_regno_offset, except where
 > > the offset comes from a pre-existing subreg.
 > >
 > > [Defining CANNOT_CHANGE_MODE_CLASS appropriately for m68k also fixes
 > > the ICE, but I don't think the m68k backend really wants that, and I
 > > think it just papers over a generic bug.]
 > >
 > > Tested with trunk and 4.8 on {m68k,sparc64,powerpc64}-linux (big-endian),
 > > and on x86_64-linux/armv5tel-linux-gnueabi (little-endian).  No regressions.
 > >
 > > Comments?
 > > Is this Ok for trunk?
 > >
 > > gcc/
 > >
 > > 2013-09-28  Mikael Pettersson  <mikpelinux@gmail.com>
 > >
 > > 	PR rtl-optimization/58369
 > > 	* reload1.c (choose_reload_regs): Use subreg_lowpart_offset
 > > 	to pass endian-correct lowpart offset to subreg_regno_offset.
 > Thanks Mikael.  My only concern is the lack of adjustment when the value 
 > found was already a SUBREG.
 > 
 > ie, let's assume rld[r].in_reg was something like
 > (subreg:XF (reg:DF) 0)
 > 
 > and our target is (reg:DF)
 > 
 > In this case it seems to me we still want to compute the subreg offset, 
 > right?
 > 
 > jeff

Thanks Jeff.  Sorry about the delay, but it took me a while to work
out the details of the various cases (I was afraid of having to do
something like simplify_subreg, but the cases in reload are simpler),
and then to re-test on my various targets.

Let the reloadee be rld[r].in_reg, outermode be its mode, and innermode
be the mode of the hard reg in last_reg.

The trivial cases are when the reloadee is a plain REG or a SUBREG of a
hard reg.  For these, reload virtually forms a normal lowpart subreg of
last_reg, and subreg_lowpart_offset (outermode, innermode) computes the
endian-correct offset for subreg_regno_offset.  This is exactly what my
previous patch did.

In remaining cases the reloadee is a SUBREG of a pseudo.  Let outer_offset
be its BYTE, and middlemode be the mode of its REG.

Another simple case is when the reloadee is paradoxical.  Then outer_offset
is zero (by convention), and reload should just form a normal lowpart
subreg as in the trivial cases.  Even though the reloadee is paradoxical,
this subreg will fit thanks to the mode size check on lines 6546-6547.

If the reloadee is a normal lowpart SUBREG, then again reload should just
form a normal lowpart subreg as in the trivial cases.  (But see below.)

The tricky case is when the reloadee is a normal but not lowpart SUBREG.
We get the correct offset for reload's virtual subreg by adding outer_offset
to the lowpart offset of middlemode and innermode.  This works for both
big-endian and little-endian.  It also handles normal lowpart SUBREGs,
so we don't need to check for lowpart vs non-lowpart normal SUBREGs.

Tested with trunk and 4.8 on {m68k,sparc64,powerpc64,x86_64}-linux-gnu
and armv5tel-linux-gnueabi.  No regressions.

Ok for trunk?

gcc/

2013-10-18  Mikael Pettersson  <mikpelinux@gmail.com>

	PR rtl-optimization/58369
	* reload1.c (compute_reload_subreg_offset): Define.
	(choose_reload_regs): Use it to pass endian-correct
	offset to subreg_regno_offset.

--- gcc-4.9-20131006/gcc/reload1.c.~1~	2013-09-28 10:42:34.000000000 +0200
+++ gcc-4.9-20131006/gcc/reload1.c	2013-10-14 01:04:29.815104039 +0200
@@ -6363,6 +6363,34 @@ replaced_subreg (rtx x)
 }
 #endif
 
+/* Compute the offset to pass to subreg_regno_offset, for a pseudo of
+   mode OUTERMODE that is available in a hard reg of mode INNERMODE.
+   SUBREG is non-NULL if the pseudo is a subreg whose reg is a pseudo,
+   otherwise it is NULL.  */
+
+static int
+compute_reload_subreg_offset (enum machine_mode outermode, rtx subreg, enum machine_mode innermode)
+{
+  int outer_offset;
+  enum machine_mode middlemode;
+
+  if (! subreg)
+    return subreg_lowpart_offset (outermode, innermode);
+
+  outer_offset = SUBREG_BYTE (subreg);
+  middlemode = GET_MODE (SUBREG_REG (subreg));
+
+  /* If SUBREG is paradoxical then return the normal lowpart offset
+     for OUTERMODE and INNERMODE.  Our caller has already checked
+     that OUTERMODE fits in INNERMODE.  */
+  if (outer_offset == 0 && GET_MODE_SIZE (outermode) > GET_MODE_SIZE (middlemode))
+    return subreg_lowpart_offset (outermode, innermode);
+
+  /* SUBREG is normal, but may not be lowpart; return OUTER_OFFSET
+     plus the normal lowpart offset for MIDDLEMODE and INNERMODE.  */
+  return outer_offset + subreg_lowpart_offset (middlemode, innermode);
+}
+
 /* Assign hard reg targets for the pseudo-registers we must reload
    into hard regs for this insn.
    Also output the instructions to copy them in and out of the hard regs.
@@ -6500,6 +6528,7 @@ choose_reload_regs (struct insn_chain *c
 	      int byte = 0;
 	      int regno = -1;
 	      enum machine_mode mode = VOIDmode;
+	      rtx subreg = NULL_RTX;
 
 	      if (rld[r].in == 0)
 		;
@@ -6520,7 +6549,10 @@ choose_reload_regs (struct insn_chain *c
 		  if (regno < FIRST_PSEUDO_REGISTER)
 		    regno = subreg_regno (rld[r].in_reg);
 		  else
-		    byte = SUBREG_BYTE (rld[r].in_reg);
+		    {
+		      subreg = rld[r].in_reg;
+		      byte = SUBREG_BYTE (subreg);
+		    }
 		  mode = GET_MODE (rld[r].in_reg);
 		}
 #ifdef AUTO_INC_DEC
@@ -6558,6 +6590,7 @@ choose_reload_regs (struct insn_chain *c
 		  rtx last_reg = reg_last_reload_reg[regno];
 
 		  i = REGNO (last_reg);
+		  byte = compute_reload_subreg_offset (mode, subreg, GET_MODE (last_reg));
 		  i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode);
 		  last_class = REGNO_REG_CLASS (i);