On Sat, Feb 13, 2016 at 07:50:25AM +0000, James Greenhalgh wrote: > On Fri, Feb 12, 2016 at 05:34:21PM +0100, Jakub Jelinek wrote: > > On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: > > > >>- mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > > > >>+ mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1; > > > >> if (xmode1 != VOIDmode && xmode1 != mode1) > > > >> { > > > > > > Here, things aren't so clear, and the fact that the mode1 calculation now > > > differs from the mode0 one may be overlooked by someone in the future. > > > > > > Rather than use codes like "mode variable is VOIDmode", I'd prefer to use > > > booleans with descriptive names, like "op1_may_need_conversion". > > > > So do you prefer e.g. following? Bootstrapped/regtested on x86_64-linux and > > i686-linux. > > > > 2016-02-12 Jakub Jelinek <ja...@redhat.com> > > > > PR rtl-optimization/69764 > > PR rtl-optimization/69771 > > * optabs.c (expand_binop_directly): For shift_optab_p, force > > convert_modes with VOIDmode if xop1 has VOIDmode. > > > > * c-c++-common/pr69764.c: New test. > > * gcc.dg/torture/pr69771.c: New testcase. > > > > These two new tests are failing for me on AArch64 as so:
As I said earlier, I wanted to fix it in expand_binop_directly because the higher levels still GEN_INT the various shift counters and then call expand_binop_directly. But, as can be seen on aarch64/arm/m68k, there are cases that need op1 to be valid for mode already in expand_binop, so in addition to the already committed fix I think we need to handle it at the expand_binop level too. As we don't have a single spot with convert_modes like expand_binop_directly, I think the best is to do there a change for the uncommon and invalid cases only, like (seems to fix the ICE both on aarch64 and m68k): 2016-02-15 Jakub Jelinek <ja...@redhat.com> PR rtl-optimization/69764 PR rtl-optimization/69771 * optabs.c (expand_binop): Ensure for shift optabs invalid CONST_INT op1 is valid for mode. --- gcc/optabs.c.jj 2016-02-12 17:49:25.000000000 +0100 +++ gcc/optabs.c 2016-02-15 16:15:53.983673792 +0100 @@ -1125,6 +1125,12 @@ expand_binop (machine_mode mode, optab b op1 = negate_rtx (mode, op1); binoptab = add_optab; } + /* For shifts, constant invalid op1 might be expanded from different + mode than MODE. */ + else if (CONST_INT_P (op1) + && shift_optab_p (binoptab) + && UINTVAL (op1) >= GET_MODE_BITSIZE (GET_MODE_INNER (mode))) + op1 = gen_int_mode (INTVAL (op1), mode); /* Record where to delete back to if we backtrack. */ last = get_last_insn (); Jakub