[PATCH] Fix ICE when expanding incorrect shift counts (PR rtl-optimization/69764, take 2)

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Feb 12, 2016 at 02:08:53AM +0100, Bernd Schmidt wrote:
> Isn't the problem just that shifts are unusual as binops, in that the two
> operands can have different modes? We have the appropriate mode for the
> shift count in xmode1, so I think we should just always rebuild a CONST_INT
> in that mode. As in,
> 
>   else if (CONST_INT_P (xop1))
>     /* Shifts can have different modes for the shift count, and the caller
>        might not have taken this into account when generating an integer.
> */
>     xop1 = gen_int_mode (INTVAL (xop1), xmode1);
> 
> Ok with that change if it passes.

That works, but I need to also ensure that convert_modes is not called on
it, as that would again assume the constant is in mode, while it is now in
xmode1.  So, is this ok for trunk (if it passes bootstrap/regtest)?

2016-02-12  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/69764
	* optabs.c (expand_binop_directly): For shift_optab_p, if xop1
	is CONST_INT, recreate it in xmode1 and ensure convert_modes
	will not be called on it.

	* c-c++-common/pr69764.c: New test.

--- gcc/optabs.c.jj	2016-02-12 00:50:30.410240366 +0100
+++ gcc/optabs.c	2016-02-12 10:33:32.743492532 +0100
@@ -988,7 +988,7 @@ expand_binop_directly (machine_mode mode
 						      from_mode, 1);
   machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
   machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
-  machine_mode mode0, mode1, tmp_mode;
+  machine_mode mode0, mode1 = VOIDmode, tmp_mode;
   struct expand_operand ops[3];
   bool commutative_p;
   rtx_insn *pat;
@@ -1006,6 +1006,13 @@ expand_binop_directly (machine_mode mode
   xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp);
   if (!shift_optab_p (binoptab))
     xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
+  else if (CONST_INT_P (xop1))
+    {
+      /* Shifts can have different modes for the shift count, and the caller
+	 might not have taken this into account when generating an integer.  */
+      xop1 = gen_int_mode (INTVAL (xop1), xmode1);
+      mode1 = xmode1;
+    }
 
   /* In case the insn wants input operands in modes different from
      those of the actual operands, convert the operands.  It would
@@ -1020,7 +1027,8 @@ expand_binop_directly (machine_mode mode
       mode0 = xmode0;
     }
 
-  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
+  if (mode1 == VOIDmode)
+    mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
   if (xmode1 != VOIDmode && xmode1 != mode1)
     {
       xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
--- gcc/testsuite/c-c++-common/pr69764.c.jj	2016-02-12 10:27:34.522587995 +0100
+++ gcc/testsuite/c-c++-common/pr69764.c	2016-02-12 10:27:34.522587995 +0100
@@ -0,0 +1,38 @@
+/* PR rtl-optimization/69764 */
+/* { dg-do compile { target int32plus } } */
+
+unsigned char
+fn1 (unsigned char a)
+{
+  return a >> ~6;	/* { dg-warning "right shift count is negative" } */
+}
+
+unsigned short
+fn2 (unsigned short a)
+{
+  return a >> ~6;	/* { dg-warning "right shift count is negative" } */
+}
+
+unsigned int
+fn3 (unsigned int a)
+{
+  return a >> ~6;	/* { dg-warning "right shift count is negative" } */
+}
+
+unsigned char
+fn4 (unsigned char a)
+{
+  return a >> 0xff03;	/* { dg-warning "right shift count >= width of type" } */
+}
+
+unsigned short
+fn5 (unsigned short a)
+{
+  return a >> 0xff03;	/* { dg-warning "right shift count >= width of type" } */
+}
+
+unsigned int
+fn6 (unsigned int a)
+{
+  return a >> 0xff03;	/* { dg-warning "right shift count >= width of type" } */
+}


	Jakub