diff mbox

Fix up register_edge_assert_for_2 (PR tree-optimization/52533)

Message ID 20120309145834.GV16117@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 9, 2012, 2:58 p.m. UTC
Hi!

My recent commit to tree-vrp.c on the trunk caused the following
testcase to fail, the problem is that we would insert <= 255
assertion for unsigned char expression (which doesn't say anything)
and VRP insist that such ASSERT_EXPRs aren't added.

Fixed thusly, in addition the patch cleans the code slightly,
e.g. by using double_int instead of a pair of uHWIs.

Bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2012-03-09  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/52533
	* tree-vrp.c (register_edge_assert_for_2): Use double_int
	type for mask, only handle shifts by non-zero in-range
	shift count, for LE_EXPR and GT_EXPR if new_val is
	maximum, don't add the assertion.

	* gcc.c-torture/compile/pr52533.c: New test.


	Jakub

Comments

Richard Biener March 12, 2012, 9:40 a.m. UTC | #1
On Fri, 9 Mar 2012, Jakub Jelinek wrote:

> Hi!
> 
> My recent commit to tree-vrp.c on the trunk caused the following
> testcase to fail, the problem is that we would insert <= 255
> assertion for unsigned char expression (which doesn't say anything)
> and VRP insist that such ASSERT_EXPRs aren't added.
> 
> Fixed thusly, in addition the patch cleans the code slightly,
> e.g. by using double_int instead of a pair of uHWIs.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?

Ok.

Thanks,
Richard.

> 2012-03-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/52533
> 	* tree-vrp.c (register_edge_assert_for_2): Use double_int
> 	type for mask, only handle shifts by non-zero in-range
> 	shift count, for LE_EXPR and GT_EXPR if new_val is
> 	maximum, don't add the assertion.
> 
> 	* gcc.c-torture/compile/pr52533.c: New test.
> 
> --- gcc/tree-vrp.c.jj	2012-03-09 10:26:26.000000000 +0100
> +++ gcc/tree-vrp.c	2012-03-09 12:30:19.348501480 +0100
> @@ -4470,7 +4470,8 @@ register_edge_assert_for_2 (tree name, e
>        gimple def_stmt = SSA_NAME_DEF_STMT (name);
>        tree name2 = NULL_TREE, cst2 = NULL_TREE;
>        tree val2 = NULL_TREE;
> -      unsigned HOST_WIDE_INT mask[2] = { 0, 0 };
> +      double_int mask = double_int_zero;
> +      unsigned int prec = TYPE_PRECISION (TREE_TYPE (val));
>  
>        /* Extract CST2 from the right shift.  */
>        if (is_gimple_assign (def_stmt)
> @@ -4480,23 +4481,13 @@ register_edge_assert_for_2 (tree name, e
>  	  cst2 = gimple_assign_rhs2 (def_stmt);
>  	  if (TREE_CODE (name2) == SSA_NAME
>  	      && host_integerp (cst2, 1)
> -	      && (unsigned HOST_WIDE_INT) tree_low_cst (cst2, 1)
> -		 < 2 * HOST_BITS_PER_WIDE_INT
>  	      && INTEGRAL_TYPE_P (TREE_TYPE (name2))
> +	      && IN_RANGE (tree_low_cst (cst2, 1), 1, prec - 1)
> +	      && prec <= 2 * HOST_BITS_PER_WIDE_INT
>  	      && live_on_edge (e, name2)
>  	      && !has_single_use (name2))
>  	    {
> -	      if ((unsigned HOST_WIDE_INT) tree_low_cst (cst2, 1)
> -		  < HOST_BITS_PER_WIDE_INT)
> -		mask[0] = ((unsigned HOST_WIDE_INT) 1
> -			   << tree_low_cst (cst2, 1)) - 1;
> -	      else
> -		{
> -		  mask[1] = ((unsigned HOST_WIDE_INT) 1
> -			     << (tree_low_cst (cst2, 1)
> -				 - HOST_BITS_PER_WIDE_INT)) - 1;
> -		  mask[0] = -1;
> -		}
> +	      mask = double_int_mask (tree_low_cst (cst2, 1));
>  	      val2 = fold_binary (LSHIFT_EXPR, TREE_TYPE (val), val, cst2);
>  	    }
>  	}
> @@ -4515,37 +4506,40 @@ register_edge_assert_for_2 (tree name, e
>  	    {
>  	      if (!TYPE_UNSIGNED (TREE_TYPE (val)))
>  		{
> -		  unsigned int prec = TYPE_PRECISION (TREE_TYPE (val));
>  		  tree type = build_nonstandard_integer_type (prec, 1);
>  		  tmp = build1 (NOP_EXPR, type, name2);
>  		  val2 = fold_convert (type, val2);
>  		}
>  	      tmp = fold_build2 (MINUS_EXPR, TREE_TYPE (tmp), tmp, val2);
> -	      new_val = build_int_cst_wide (TREE_TYPE (tmp), mask[0], mask[1]);
> +	      new_val = double_int_to_tree (TREE_TYPE (tmp), mask);
>  	      new_comp_code = comp_code == EQ_EXPR ? LE_EXPR : GT_EXPR;
>  	    }
>  	  else if (comp_code == LT_EXPR || comp_code == GE_EXPR)
>  	    new_val = val2;
>  	  else
>  	    {
> -	      new_val = build_int_cst_wide (TREE_TYPE (val2),
> -					    mask[0], mask[1]);
> -	      new_val = fold_binary (BIT_IOR_EXPR, TREE_TYPE (val2),
> -				     val2, new_val);
> +	      mask = double_int_ior (tree_to_double_int (val2), mask);
> +	      if (double_int_minus_one_p (double_int_sext (mask, prec)))
> +		new_val = NULL_TREE;
> +	      else
> +		new_val = double_int_to_tree (TREE_TYPE (val2), mask);
>  	    }
>  
> -	  if (dump_file)
> +	  if (new_val)
>  	    {
> -	      fprintf (dump_file, "Adding assert for ");
> -	      print_generic_expr (dump_file, name2, 0);
> -	      fprintf (dump_file, " from ");
> -	      print_generic_expr (dump_file, tmp, 0);
> -	      fprintf (dump_file, "\n");
> -	    }
> +	      if (dump_file)
> +		{
> +		  fprintf (dump_file, "Adding assert for ");
> +		  print_generic_expr (dump_file, name2, 0);
> +		  fprintf (dump_file, " from ");
> +		  print_generic_expr (dump_file, tmp, 0);
> +		  fprintf (dump_file, "\n");
> +		}
>  
> -	  register_new_assert_for (name2, tmp, new_comp_code, new_val,
> -				   NULL, e, bsi);
> -	  retval = true;
> +	      register_new_assert_for (name2, tmp, new_comp_code, new_val,
> +				       NULL, e, bsi);
> +	      retval = true;
> +	    }
>  	}
>      }
>  
> --- gcc/testsuite/gcc.c-torture/compile/pr52533.c.jj	2012-03-09 12:20:39.189877465 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr52533.c	2012-03-09 09:22:35.000000000 +0100
> @@ -0,0 +1,19 @@
> +/* PR tree-optimization/52533 */
> +
> +int
> +foo (unsigned char x)
> +{
> +  if (x <= 9)
> +    return '0' + x;
> +  else if (x <= 15)
> +    return 'a' + (x - 10);
> +  else
> +    return 0;
> +}
> +
> +void
> +bar (unsigned char x, unsigned char *y)
> +{
> +  y[0] = foo ((unsigned char) (x >> 4));
> +  y[1] = foo ((unsigned char) (x & 0x0f));
> +}
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/tree-vrp.c.jj	2012-03-09 10:26:26.000000000 +0100
+++ gcc/tree-vrp.c	2012-03-09 12:30:19.348501480 +0100
@@ -4470,7 +4470,8 @@  register_edge_assert_for_2 (tree name, e
       gimple def_stmt = SSA_NAME_DEF_STMT (name);
       tree name2 = NULL_TREE, cst2 = NULL_TREE;
       tree val2 = NULL_TREE;
-      unsigned HOST_WIDE_INT mask[2] = { 0, 0 };
+      double_int mask = double_int_zero;
+      unsigned int prec = TYPE_PRECISION (TREE_TYPE (val));
 
       /* Extract CST2 from the right shift.  */
       if (is_gimple_assign (def_stmt)
@@ -4480,23 +4481,13 @@  register_edge_assert_for_2 (tree name, e
 	  cst2 = gimple_assign_rhs2 (def_stmt);
 	  if (TREE_CODE (name2) == SSA_NAME
 	      && host_integerp (cst2, 1)
-	      && (unsigned HOST_WIDE_INT) tree_low_cst (cst2, 1)
-		 < 2 * HOST_BITS_PER_WIDE_INT
 	      && INTEGRAL_TYPE_P (TREE_TYPE (name2))
+	      && IN_RANGE (tree_low_cst (cst2, 1), 1, prec - 1)
+	      && prec <= 2 * HOST_BITS_PER_WIDE_INT
 	      && live_on_edge (e, name2)
 	      && !has_single_use (name2))
 	    {
-	      if ((unsigned HOST_WIDE_INT) tree_low_cst (cst2, 1)
-		  < HOST_BITS_PER_WIDE_INT)
-		mask[0] = ((unsigned HOST_WIDE_INT) 1
-			   << tree_low_cst (cst2, 1)) - 1;
-	      else
-		{
-		  mask[1] = ((unsigned HOST_WIDE_INT) 1
-			     << (tree_low_cst (cst2, 1)
-				 - HOST_BITS_PER_WIDE_INT)) - 1;
-		  mask[0] = -1;
-		}
+	      mask = double_int_mask (tree_low_cst (cst2, 1));
 	      val2 = fold_binary (LSHIFT_EXPR, TREE_TYPE (val), val, cst2);
 	    }
 	}
@@ -4515,37 +4506,40 @@  register_edge_assert_for_2 (tree name, e
 	    {
 	      if (!TYPE_UNSIGNED (TREE_TYPE (val)))
 		{
-		  unsigned int prec = TYPE_PRECISION (TREE_TYPE (val));
 		  tree type = build_nonstandard_integer_type (prec, 1);
 		  tmp = build1 (NOP_EXPR, type, name2);
 		  val2 = fold_convert (type, val2);
 		}
 	      tmp = fold_build2 (MINUS_EXPR, TREE_TYPE (tmp), tmp, val2);
-	      new_val = build_int_cst_wide (TREE_TYPE (tmp), mask[0], mask[1]);
+	      new_val = double_int_to_tree (TREE_TYPE (tmp), mask);
 	      new_comp_code = comp_code == EQ_EXPR ? LE_EXPR : GT_EXPR;
 	    }
 	  else if (comp_code == LT_EXPR || comp_code == GE_EXPR)
 	    new_val = val2;
 	  else
 	    {
-	      new_val = build_int_cst_wide (TREE_TYPE (val2),
-					    mask[0], mask[1]);
-	      new_val = fold_binary (BIT_IOR_EXPR, TREE_TYPE (val2),
-				     val2, new_val);
+	      mask = double_int_ior (tree_to_double_int (val2), mask);
+	      if (double_int_minus_one_p (double_int_sext (mask, prec)))
+		new_val = NULL_TREE;
+	      else
+		new_val = double_int_to_tree (TREE_TYPE (val2), mask);
 	    }
 
-	  if (dump_file)
+	  if (new_val)
 	    {
-	      fprintf (dump_file, "Adding assert for ");
-	      print_generic_expr (dump_file, name2, 0);
-	      fprintf (dump_file, " from ");
-	      print_generic_expr (dump_file, tmp, 0);
-	      fprintf (dump_file, "\n");
-	    }
+	      if (dump_file)
+		{
+		  fprintf (dump_file, "Adding assert for ");
+		  print_generic_expr (dump_file, name2, 0);
+		  fprintf (dump_file, " from ");
+		  print_generic_expr (dump_file, tmp, 0);
+		  fprintf (dump_file, "\n");
+		}
 
-	  register_new_assert_for (name2, tmp, new_comp_code, new_val,
-				   NULL, e, bsi);
-	  retval = true;
+	      register_new_assert_for (name2, tmp, new_comp_code, new_val,
+				       NULL, e, bsi);
+	      retval = true;
+	    }
 	}
     }
 
--- gcc/testsuite/gcc.c-torture/compile/pr52533.c.jj	2012-03-09 12:20:39.189877465 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr52533.c	2012-03-09 09:22:35.000000000 +0100
@@ -0,0 +1,19 @@ 
+/* PR tree-optimization/52533 */
+
+int
+foo (unsigned char x)
+{
+  if (x <= 9)
+    return '0' + x;
+  else if (x <= 15)
+    return 'a' + (x - 10);
+  else
+    return 0;
+}
+
+void
+bar (unsigned char x, unsigned char *y)
+{
+  y[0] = foo ((unsigned char) (x >> 4));
+  y[1] = foo ((unsigned char) (x & 0x0f));
+}