Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

[Jakub Jelinek <jakub at redhat dot com>]

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  <jakub@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  <jakub@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