diff mbox

Fix reassoc bit test optimization (PR tree-optimization/65418)

Message ID 20150314091034.GK1746@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 14, 2015, 9:10 a.m. UTC
Hi!

The first testcase shows a bug in my bit test reassoc optimization,
extract_bit_test_mask is (intentionally) stripping nops, but was setting
*totallowp and operating with tbias in the type of unstripped expression,
which then results in different signedness of types used and confusing the
optimization.  In particular, -218 and -216 are already folded into (x is
signed int)
(((unsigned) x + 218U) & -2U) == 0
and thus without the patch we set lowi in the parent to -218U.
Then -146 and -132 are just
x == -146
and
x == -132
thus we were comparing -218U to -146 or -132.  But we really want
to use -218 instead, as that is the type of x.

Fixed thusly, bootstrapped/regtested on
{x86_64,i686,aarch64,powerpc64{,le},s390{,x}}-linux, ok for trunk?

2015-03-14  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/65418
	* tree-ssa-reassoc.c (extract_bit_test_mask): If there
	are casts in the first PLUS_EXPR operand, ensure tbias and
	*totallowp are in the inner type.

	* gcc.c-torture/execute/pr65418-1.c: New test.
	* gcc.c-torture/execute/pr65418-2.c: New test.


	Jakub

Comments

Richard Biener March 14, 2015, 9:48 a.m. UTC | #1
On March 14, 2015 10:10:34 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The first testcase shows a bug in my bit test reassoc optimization,
>extract_bit_test_mask is (intentionally) stripping nops, but was
>setting
>*totallowp and operating with tbias in the type of unstripped
>expression,
>which then results in different signedness of types used and confusing
>the
>optimization.  In particular, -218 and -216 are already folded into (x
>is
>signed int)
>(((unsigned) x + 218U) & -2U) == 0
>and thus without the patch we set lowi in the parent to -218U.
>Then -146 and -132 are just
>x == -146
>and
>x == -132
>thus we were comparing -218U to -146 or -132.  But we really want
>to use -218 instead, as that is the type of x.
>
>Fixed thusly, bootstrapped/regtested on
>{x86_64,i686,aarch64,powerpc64{,le},s390{,x}}-linux, ok for trunk?

OK.

Thanks,
Richard.

>2015-03-14  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/65418
>	* tree-ssa-reassoc.c (extract_bit_test_mask): If there
>	are casts in the first PLUS_EXPR operand, ensure tbias and
>	*totallowp are in the inner type.
>
>	* gcc.c-torture/execute/pr65418-1.c: New test.
>	* gcc.c-torture/execute/pr65418-2.c: New test.
>
>--- gcc/tree-ssa-reassoc.c.jj	2015-02-26 22:02:39.000000000 +0100
>+++ gcc/tree-ssa-reassoc.c	2015-03-13 16:22:50.506295252 +0100
>@@ -2439,26 +2439,25 @@ extract_bit_test_mask (tree exp, int pre
> 	      && TREE_CODE (exp) == PLUS_EXPR
> 	      && TREE_CODE (TREE_OPERAND (exp, 1)) == INTEGER_CST)
> 	    {
>+	      tree ret = TREE_OPERAND (exp, 0);
>+	      STRIP_NOPS (ret);
> 	      widest_int bias
> 	        = wi::neg (wi::sext (wi::to_widest (TREE_OPERAND (exp, 1)),
> 				     TYPE_PRECISION (TREE_TYPE (low))));
>-	      tree tbias = wide_int_to_tree (TREE_TYPE (low), bias);
>+	      tree tbias = wide_int_to_tree (TREE_TYPE (ret), bias);
> 	      if (totallowp)
> 		{
> 		  *totallowp = tbias;
>-		  exp = TREE_OPERAND (exp, 0);
>-		  STRIP_NOPS (exp);
>-		  return exp;
>+		  return ret;
> 		}
> 	      else if (!tree_int_cst_lt (totallow, tbias))
> 		return NULL_TREE;
>+	      bias = wi::to_widest (tbias);
> 	      bias -= wi::to_widest (totallow);
> 	      if (wi::ges_p (bias, 0) && wi::lts_p (bias, prec - max))
> 		{
> 		  *mask = wi::lshift (*mask, bias);
>-		  exp = TREE_OPERAND (exp, 0);
>-		  STRIP_NOPS (exp);
>-		  return exp;
>+		  return ret;
> 		}
> 	    }
> 	}
>--- gcc/testsuite/gcc.c-torture/execute/pr65418-1.c.jj	2015-03-13
>16:49:07.973604649 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/pr65418-1.c	2015-03-13
>16:48:28.000000000 +0100
>@@ -0,0 +1,19 @@
>+/* PR tree-optimization/65418 */
>+
>+__attribute__((noinline, noclone)) int
>+foo (int x)
>+{
>+  if (x == -216 || x == -132 || x == -218 || x == -146)
>+     return 1;
>+  return 0;
>+}
>+
>+int
>+main ()
>+{
>+  volatile int i;
>+  for (i = -230; i < -120; i++)
>+    if (foo (i) != (i == -216 || i == -132 || i == -218 || i == -146))
>+      __builtin_abort ();
>+  return 0;
>+}
>--- gcc/testsuite/gcc.c-torture/execute/pr65418-2.c.jj	2015-03-13
>16:49:10.992556110 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/pr65418-2.c	2015-03-13
>16:48:44.000000000 +0100
>@@ -0,0 +1,19 @@
>+/* PR tree-optimization/65418 */
>+
>+__attribute__((noinline, noclone)) int
>+foo (int x)
>+{
>+  if (x == -216 || x == -211 || x == -218 || x == -205 || x == -223)
>+     return 1;
>+  return 0;
>+}
>+
>+int
>+main ()
>+{
>+  volatile int i;
>+  for (i = -230; i < -200; i++)
>+    if (foo (i) != (i == -216 || i == -211 || i == -218 || i == -205
>|| i == -223))
>+      __builtin_abort ();
>+  return 0;
>+}
>
>	Jakub
diff mbox

Patch

--- gcc/tree-ssa-reassoc.c.jj	2015-02-26 22:02:39.000000000 +0100
+++ gcc/tree-ssa-reassoc.c	2015-03-13 16:22:50.506295252 +0100
@@ -2439,26 +2439,25 @@  extract_bit_test_mask (tree exp, int pre
 	      && TREE_CODE (exp) == PLUS_EXPR
 	      && TREE_CODE (TREE_OPERAND (exp, 1)) == INTEGER_CST)
 	    {
+	      tree ret = TREE_OPERAND (exp, 0);
+	      STRIP_NOPS (ret);
 	      widest_int bias
 	        = wi::neg (wi::sext (wi::to_widest (TREE_OPERAND (exp, 1)),
 				     TYPE_PRECISION (TREE_TYPE (low))));
-	      tree tbias = wide_int_to_tree (TREE_TYPE (low), bias);
+	      tree tbias = wide_int_to_tree (TREE_TYPE (ret), bias);
 	      if (totallowp)
 		{
 		  *totallowp = tbias;
-		  exp = TREE_OPERAND (exp, 0);
-		  STRIP_NOPS (exp);
-		  return exp;
+		  return ret;
 		}
 	      else if (!tree_int_cst_lt (totallow, tbias))
 		return NULL_TREE;
+	      bias = wi::to_widest (tbias);
 	      bias -= wi::to_widest (totallow);
 	      if (wi::ges_p (bias, 0) && wi::lts_p (bias, prec - max))
 		{
 		  *mask = wi::lshift (*mask, bias);
-		  exp = TREE_OPERAND (exp, 0);
-		  STRIP_NOPS (exp);
-		  return exp;
+		  return ret;
 		}
 	    }
 	}
--- gcc/testsuite/gcc.c-torture/execute/pr65418-1.c.jj	2015-03-13 16:49:07.973604649 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr65418-1.c	2015-03-13 16:48:28.000000000 +0100
@@ -0,0 +1,19 @@ 
+/* PR tree-optimization/65418 */
+
+__attribute__((noinline, noclone)) int
+foo (int x)
+{
+  if (x == -216 || x == -132 || x == -218 || x == -146)
+     return 1;
+  return 0;
+}
+
+int
+main ()
+{
+  volatile int i;
+  for (i = -230; i < -120; i++)
+    if (foo (i) != (i == -216 || i == -132 || i == -218 || i == -146))
+      __builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr65418-2.c.jj	2015-03-13 16:49:10.992556110 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr65418-2.c	2015-03-13 16:48:44.000000000 +0100
@@ -0,0 +1,19 @@ 
+/* PR tree-optimization/65418 */
+
+__attribute__((noinline, noclone)) int
+foo (int x)
+{
+  if (x == -216 || x == -211 || x == -218 || x == -205 || x == -223)
+     return 1;
+  return 0;
+}
+
+int
+main ()
+{
+  volatile int i;
+  for (i = -230; i < -200; i++)
+    if (foo (i) != (i == -216 || i == -211 || i == -218 || i == -205 || i == -223))
+      __builtin_abort ();
+  return 0;
+}