| Message ID | 21062.62988.797370.296379@gargle.gargle.HOWL |
|---|---|
| State | New |
| Headers | show |
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
--- gcc-4.9-20130922/gcc/reload1.c.~1~ 2013-09-09 15:07:10.000000000 +0200 +++ gcc-4.9-20130922/gcc/reload1.c 2013-09-28 16:24:21.068294912 +0200 @@ -6497,6 +6497,7 @@ choose_reload_regs (struct insn_chain *c if (inheritance) { int byte = 0; + bool byte_is_fixed = false; int regno = -1; enum machine_mode mode = VOIDmode; @@ -6519,7 +6520,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); + { + byte = SUBREG_BYTE (rld[r].in_reg); + byte_is_fixed = true; + } mode = GET_MODE (rld[r].in_reg); } #ifdef AUTO_INC_DEC @@ -6557,6 +6561,8 @@ choose_reload_regs (struct insn_chain *c rtx last_reg = reg_last_reload_reg[regno]; i = REGNO (last_reg); + if (! byte_is_fixed) + byte = subreg_lowpart_offset (mode, GET_MODE (last_reg)); i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode); last_class = REGNO_REG_CLASS (i);
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.