diff mbox

Fix eliminate_redundant_comparison (PR tree-optimization/48734)

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

Commit Message

Jakub Jelinek April 26, 2011, 1:02 p.m. UTC
Hi!

eliminate_redundant_comparison calls maybe_fold_{and,or}_comparisons,
which calls fold.  Expecting that the result is either INTEGER_CST or
a comparison with gimple vals is just bad assumption, fold can create
all kinds of canonicalizations.

While we can perhaps handle a few of them here, I'm slightly worried
to add too much more stuff using build_and_add_sum etc., as that might
increase register pressure.

This patch just gives up if it can't handle it instead of ICEing,
in the future as follow-up we could perhaps handle a few cases.
Given that this ICE hit us only once after so long and never on real-world
code so far, I'd say so it isn't that urgent though.

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

2011-04-26  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/48734
	* tree-ssa-reassoc.c (eliminate_redundant_comparison): Give up
	if return value from maybe_fold_*_comparsions isn't something
	the code is prepared to handle.

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


	Jakub

Comments

Richard Biener April 26, 2011, 1:07 p.m. UTC | #1
On Tue, Apr 26, 2011 at 3:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> eliminate_redundant_comparison calls maybe_fold_{and,or}_comparisons,
> which calls fold.  Expecting that the result is either INTEGER_CST or
> a comparison with gimple vals is just bad assumption, fold can create
> all kinds of canonicalizations.
>
> While we can perhaps handle a few of them here, I'm slightly worried
> to add too much more stuff using build_and_add_sum etc., as that might
> increase register pressure.
>
> This patch just gives up if it can't handle it instead of ICEing,
> in the future as follow-up we could perhaps handle a few cases.
> Given that this ICE hit us only once after so long and never on real-world
> code so far, I'd say so it isn't that urgent though.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?

Ok.

Thanks,
RIchard.

> 2011-04-26  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/48734
>        * tree-ssa-reassoc.c (eliminate_redundant_comparison): Give up
>        if return value from maybe_fold_*_comparsions isn't something
>        the code is prepared to handle.
>
>        * gcc.c-torture/compile/pr48734.c: New test.
>
> --- gcc/tree-ssa-reassoc.c.jj   2011-02-15 15:42:27.000000000 +0100
> +++ gcc/tree-ssa-reassoc.c      2011-04-26 09:32:51.000000000 +0200
> @@ -1279,6 +1279,20 @@ eliminate_redundant_comparison (enum tre
>       if (!useless_type_conversion_p (TREE_TYPE (curr->op), TREE_TYPE (t)))
>        t = fold_convert (TREE_TYPE (curr->op), t);
>
> +      if (TREE_CODE (t) != INTEGER_CST
> +         && !operand_equal_p (t, curr->op, 0))
> +       {
> +         enum tree_code subcode;
> +         tree newop1, newop2;
> +         if (!COMPARISON_CLASS_P (t))
> +           continue;
> +         extract_ops_from_tree (t, &subcode, &newop1, &newop2);
> +         STRIP_USELESS_TYPE_CONVERSION (newop1);
> +         STRIP_USELESS_TYPE_CONVERSION (newop2);
> +         if (!is_gimple_val (newop1) || !is_gimple_val (newop2))
> +           continue;
> +       }
> +
>       if (dump_file && (dump_flags & TDF_DETAILS))
>        {
>          fprintf (dump_file, "Equivalence: ");
> --- gcc/testsuite/gcc.c-torture/compile/pr48734.c.jj    2011-04-26 09:34:38.000000000 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr48734.c       2011-04-26 09:34:17.000000000 +0200
> @@ -0,0 +1,11 @@
> +/* PR tree-optimization/48734 */
> +
> +unsigned int
> +foo (int x, unsigned int y, unsigned int z)
> +{
> +  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
> +  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
> +  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
> +  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
> +  return z;
> +}
>
>        Jakub
>
diff mbox

Patch

--- gcc/tree-ssa-reassoc.c.jj	2011-02-15 15:42:27.000000000 +0100
+++ gcc/tree-ssa-reassoc.c	2011-04-26 09:32:51.000000000 +0200
@@ -1279,6 +1279,20 @@  eliminate_redundant_comparison (enum tre
       if (!useless_type_conversion_p (TREE_TYPE (curr->op), TREE_TYPE (t)))
 	t = fold_convert (TREE_TYPE (curr->op), t);
 
+      if (TREE_CODE (t) != INTEGER_CST
+	  && !operand_equal_p (t, curr->op, 0))
+	{
+	  enum tree_code subcode;
+	  tree newop1, newop2;
+	  if (!COMPARISON_CLASS_P (t))
+	    continue;
+	  extract_ops_from_tree (t, &subcode, &newop1, &newop2);
+	  STRIP_USELESS_TYPE_CONVERSION (newop1);
+	  STRIP_USELESS_TYPE_CONVERSION (newop2);
+	  if (!is_gimple_val (newop1) || !is_gimple_val (newop2))
+	    continue;
+	}
+
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
 	  fprintf (dump_file, "Equivalence: ");
--- gcc/testsuite/gcc.c-torture/compile/pr48734.c.jj	2011-04-26 09:34:38.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr48734.c	2011-04-26 09:34:17.000000000 +0200
@@ -0,0 +1,11 @@ 
+/* PR tree-optimization/48734 */
+
+unsigned int
+foo (int x, unsigned int y, unsigned int z)
+{
+  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
+  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
+  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
+  z &= (x == -__INT_MAX__ - 1 ? x : -x) > y;
+  return z;
+}