Re: [PATCH][GCC 7] Remove broken path in extract_bit_field_1
On Mon, Apr 4, 2016 at 3:33 PM, Jakub Jelinekwrote: > On Mon, Apr 04, 2016 at 03:27:23PM +0200, Richard Biener wrote: >> It ICEs in >> >> /space/rguenther/src/svn/trunk/gcc/testsuite/gcc.target/i386/pr37870.c:19:1: >> internal compiler error: in subreg_get_info, at rtlanal.c:3695 >> 0xddee5a subreg_get_info(unsigned int, machine_mode, unsigned int, >> machine_mode, subreg_info*) >> /space/rguenther/src/svn/trunk/gcc/rtlanal.c:3695 >> 0xddf12b simplify_subreg_regno(unsigned int, machine_mode, unsigned int, >> machine_mode) >> /space/rguenther/src/svn/trunk/gcc/rtlanal.c:3808 >> 0xd8bc7a simplifiable_subregs(subreg_shape const&) >> /space/rguenther/src/svn/trunk/gcc/reginfo.c:1234 >> 0xd8be1d record_subregs_of_mode >> /space/rguenther/src/svn/trunk/gcc/reginfo.c:1294 >> 0xd8c246 init_subregs_of_mode() >> /space/rguenther/src/svn/trunk/gcc/reginfo.c:1348 >> 0xc1a55c init_costs >> /space/rguenther/src/svn/trunk/gcc/ira-costs.c:2187 >> >> which is >> >> /* This should always pass, otherwise we don't know how to verify >> the constraint. These conditions may be relaxed but >> subreg_regno_offset would need to be redesigned. */ >> gcc_assert ((GET_MODE_SIZE (xmode) % GET_MODE_SIZE (ymode)) == 0); > > So perhaps either validate_subreg should check the same thing, or > the extraction could check this. > Anyway, I don't have anything against removing that hunk from > extract_bit_field_1 for GCC7. Now committed. Richard. > Jakub
Re: [PATCH][GCC 7] Remove broken path in extract_bit_field_1
On Mon, Apr 04, 2016 at 03:27:23PM +0200, Richard Biener wrote: > It ICEs in > > /space/rguenther/src/svn/trunk/gcc/testsuite/gcc.target/i386/pr37870.c:19:1: > internal compiler error: in subreg_get_info, at rtlanal.c:3695 > 0xddee5a subreg_get_info(unsigned int, machine_mode, unsigned int, > machine_mode, subreg_info*) > /space/rguenther/src/svn/trunk/gcc/rtlanal.c:3695 > 0xddf12b simplify_subreg_regno(unsigned int, machine_mode, unsigned int, > machine_mode) > /space/rguenther/src/svn/trunk/gcc/rtlanal.c:3808 > 0xd8bc7a simplifiable_subregs(subreg_shape const&) > /space/rguenther/src/svn/trunk/gcc/reginfo.c:1234 > 0xd8be1d record_subregs_of_mode > /space/rguenther/src/svn/trunk/gcc/reginfo.c:1294 > 0xd8c246 init_subregs_of_mode() > /space/rguenther/src/svn/trunk/gcc/reginfo.c:1348 > 0xc1a55c init_costs > /space/rguenther/src/svn/trunk/gcc/ira-costs.c:2187 > > which is > > /* This should always pass, otherwise we don't know how to verify > the constraint. These conditions may be relaxed but > subreg_regno_offset would need to be redesigned. */ > gcc_assert ((GET_MODE_SIZE (xmode) % GET_MODE_SIZE (ymode)) == 0); So perhaps either validate_subreg should check the same thing, or the extraction could check this. Anyway, I don't have anything against removing that hunk from extract_bit_field_1 for GCC7. Jakub
Re: [PATCH][GCC 7] Remove broken path in extract_bit_field_1
On Mon, 4 Apr 2016, Jakub Jelinek wrote: > On Mon, Apr 04, 2016 at 03:10:34PM +0200, Richard Biener wrote: > > On Mon, 4 Apr 2016, Jakub Jelinek wrote: > > > > > On Mon, Apr 04, 2016 at 02:56:51PM +0200, Richard Biener wrote: > > > > The testcase gcc.target/i386/pr37870.c will already ICE with that > > > > patch, so no additional testcase. > > > > > > In theory you could validate_subreg first and use that code if validation > > > went ok, otherwise go through memory. > > > But I admit I don't have anything in particular in mind where it would > > > trigger this code and the subreg would successfully validate. > > > > Not sure if it would help as that has > > > > /* ??? Similarly, e.g. with (subreg:DF (reg:TI)). Though > > store_bit_field > > is the culprit here, and not the backends. */ > > else if (osize >= UNITS_PER_WORD && isize >= osize) > > ; > > > > and thus we'd return true anyway for (subreg:XF (reg:TI) 0) > > If XFmode subreg of TImode reg passes validation, where does it ICE then? It ICEs in /space/rguenther/src/svn/trunk/gcc/testsuite/gcc.target/i386/pr37870.c:19:1: internal compiler error: in subreg_get_info, at rtlanal.c:3695 0xddee5a subreg_get_info(unsigned int, machine_mode, unsigned int, machine_mode, subreg_info*) /space/rguenther/src/svn/trunk/gcc/rtlanal.c:3695 0xddf12b simplify_subreg_regno(unsigned int, machine_mode, unsigned int, machine_mode) /space/rguenther/src/svn/trunk/gcc/rtlanal.c:3808 0xd8bc7a simplifiable_subregs(subreg_shape const&) /space/rguenther/src/svn/trunk/gcc/reginfo.c:1234 0xd8be1d record_subregs_of_mode /space/rguenther/src/svn/trunk/gcc/reginfo.c:1294 0xd8c246 init_subregs_of_mode() /space/rguenther/src/svn/trunk/gcc/reginfo.c:1348 0xc1a55c init_costs /space/rguenther/src/svn/trunk/gcc/ira-costs.c:2187 which is /* This should always pass, otherwise we don't know how to verify the constraint. These conditions may be relaxed but subreg_regno_offset would need to be redesigned. */ gcc_assert ((GET_MODE_SIZE (xmode) % GET_MODE_SIZE (ymode)) == 0); Richard.
Re: [PATCH][GCC 7] Remove broken path in extract_bit_field_1
On Mon, Apr 04, 2016 at 03:10:34PM +0200, Richard Biener wrote: > On Mon, 4 Apr 2016, Jakub Jelinek wrote: > > > On Mon, Apr 04, 2016 at 02:56:51PM +0200, Richard Biener wrote: > > > The testcase gcc.target/i386/pr37870.c will already ICE with that > > > patch, so no additional testcase. > > > > In theory you could validate_subreg first and use that code if validation > > went ok, otherwise go through memory. > > But I admit I don't have anything in particular in mind where it would > > trigger this code and the subreg would successfully validate. > > Not sure if it would help as that has > > /* ??? Similarly, e.g. with (subreg:DF (reg:TI)). Though > store_bit_field > is the culprit here, and not the backends. */ > else if (osize >= UNITS_PER_WORD && isize >= osize) > ; > > and thus we'd return true anyway for (subreg:XF (reg:TI) 0) If XFmode subreg of TImode reg passes validation, where does it ICE then? Jakub
Re: [PATCH][GCC 7] Remove broken path in extract_bit_field_1
On Mon, 4 Apr 2016, Jakub Jelinek wrote: > On Mon, Apr 04, 2016 at 02:56:51PM +0200, Richard Biener wrote: > > The testcase gcc.target/i386/pr37870.c will already ICE with that > > patch, so no additional testcase. > > In theory you could validate_subreg first and use that code if validation > went ok, otherwise go through memory. > But I admit I don't have anything in particular in mind where it would > trigger this code and the subreg would successfully validate. Not sure if it would help as that has /* ??? Similarly, e.g. with (subreg:DF (reg:TI)). Though store_bit_field is the culprit here, and not the backends. */ else if (osize >= UNITS_PER_WORD && isize >= osize) ; and thus we'd return true anyway for (subreg:XF (reg:TI) 0) Richard. > > 2016-04-04 Richard Biener> > > > PR middle-end/37870 > > * expmed.c (extract_bit_field_1): Remove broken case > > using a wider MODE_INT mode. > > > > Index: gcc/expmed.c > > === > > --- gcc/expmed.c(revision 234708) > > +++ gcc/expmed.c(working copy) > > @@ -1647,17 +1647,6 @@ extract_bit_field_1 (rtx str_rtx, unsign > > if (GET_CODE (op0) == SUBREG) > > op0 = force_reg (imode, op0); > > } > > - else if (REG_P (op0)) > > - { > > - rtx reg, subreg; > > - imode = smallest_mode_for_size (GET_MODE_BITSIZE (GET_MODE (op0)), > > - MODE_INT); > > - reg = gen_reg_rtx (imode); > > - subreg = gen_lowpart_SUBREG (GET_MODE (op0), reg); > > - emit_move_insn (subreg, op0); > > - op0 = reg; > > - bitnum += SUBREG_BYTE (subreg) * BITS_PER_UNIT; > > - } > > else > > { > > HOST_WIDE_INT size = GET_MODE_SIZE (GET_MODE (op0)); > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH][GCC 7] Remove broken path in extract_bit_field_1
On Mon, Apr 04, 2016 at 02:56:51PM +0200, Richard Biener wrote: > The testcase gcc.target/i386/pr37870.c will already ICE with that > patch, so no additional testcase. In theory you could validate_subreg first and use that code if validation went ok, otherwise go through memory. But I admit I don't have anything in particular in mind where it would trigger this code and the subreg would successfully validate. > 2016-04-04 Richard Biener> > PR middle-end/37870 > * expmed.c (extract_bit_field_1): Remove broken case > using a wider MODE_INT mode. > > Index: gcc/expmed.c > === > --- gcc/expmed.c (revision 234708) > +++ gcc/expmed.c (working copy) > @@ -1647,17 +1647,6 @@ extract_bit_field_1 (rtx str_rtx, unsign > if (GET_CODE (op0) == SUBREG) > op0 = force_reg (imode, op0); > } > - else if (REG_P (op0)) > - { > - rtx reg, subreg; > - imode = smallest_mode_for_size (GET_MODE_BITSIZE (GET_MODE (op0)), > - MODE_INT); > - reg = gen_reg_rtx (imode); > - subreg = gen_lowpart_SUBREG (GET_MODE (op0), reg); > - emit_move_insn (subreg, op0); > - op0 = reg; > - bitnum += SUBREG_BYTE (subreg) * BITS_PER_UNIT; > - } > else > { > HOST_WIDE_INT size = GET_MODE_SIZE (GET_MODE (op0)); Jakub