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 <mikpeli...@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 <mikpeli...@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);
