diff mbox

Fix reassoc range opt related ICE (PR tree-optimization/81003)

Message ID 20170608184906.GA2154@tucnak
State New
Headers show

Commit Message

Jakub Jelinek June 8, 2017, 6:49 p.m. UTC
Hi!

force_gimple_operand_gsi called by update_range_test can using match.pd
simplifications sometimes return INTEGER_CST (especially when cunroll
unrolled code isn't really optimized by forwprop/ccp and similar passes
before reassoc2), but that is something not acceptable to the rest of
the optimize_range* code, because it needs to know not just the value,
but also some gimple_stmt_iterator to insert related code etc.

This patch makes sure we have a SSA_NAME even in that case.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-06-08  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/81003
	* tree-ssa-reassoc.c (force_into_ssa_name): New function.
	(update_range_test): Use it instead of force_gimple_operand_gsi.

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


	Jakub

Comments

Richard Biener June 9, 2017, 9:29 a.m. UTC | #1
On Thu, 8 Jun 2017, Jakub Jelinek wrote:

> Hi!
> 
> force_gimple_operand_gsi called by update_range_test can using match.pd
> simplifications sometimes return INTEGER_CST (especially when cunroll
> unrolled code isn't really optimized by forwprop/ccp and similar passes
> before reassoc2), but that is something not acceptable to the rest of
> the optimize_range* code, because it needs to know not just the value,
> but also some gimple_stmt_iterator to insert related code etc.
> 
> This patch makes sure we have a SSA_NAME even in that case.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

In the end this results in a missed optimization downstream but I
guess we can't resolve this w/o major refactoring?

Thanks,
Richard.

> 2017-06-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/81003
> 	* tree-ssa-reassoc.c (force_into_ssa_name): New function.
> 	(update_range_test): Use it instead of force_gimple_operand_gsi.
> 
> 	* gcc.c-torture/compile/pr81003.c: New test.
> 
> --- gcc/tree-ssa-reassoc.c.jj	2017-06-05 11:58:14.000000000 +0200
> +++ gcc/tree-ssa-reassoc.c	2017-06-08 12:20:47.619790903 +0200
> @@ -2282,6 +2282,26 @@ range_entry_cmp (const void *a, const vo
>      }
>  }
>  
> +/* Helper function for update_range_test.  Force EXPR into an SSA_NAME,
> +   insert needed statements BEFORE or after GSI.  */
> +
> +static tree
> +force_into_ssa_name (gimple_stmt_iterator *gsi, tree expr, bool before)
> +{
> +  enum gsi_iterator_update m = before ? GSI_SAME_STMT : GSI_CONTINUE_LINKING;
> +  tree ret = force_gimple_operand_gsi (gsi, expr, true, NULL_TREE, before, m);
> +  if (TREE_CODE (ret) != SSA_NAME)
> +    {
> +      gimple *g = gimple_build_assign (make_ssa_name (TREE_TYPE (ret)), ret);
> +      if (before)
> +	gsi_insert_before (gsi, g, GSI_SAME_STMT);
> +      else
> +	gsi_insert_after (gsi, g, GSI_CONTINUE_LINKING);
> +      ret = gimple_assign_lhs (g);
> +    }
> +  return ret;
> +}
> +
>  /* Helper routine of optimize_range_test.
>     [EXP, IN_P, LOW, HIGH, STRICT_OVERFLOW_P] is a merged range for
>     RANGE and OTHERRANGE through OTHERRANGE + COUNT - 1 ranges,
> @@ -2393,15 +2413,13 @@ update_range_test (struct range_entry *r
>    else if (op != range->exp)
>      {
>        gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> -      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
> -				      GSI_SAME_STMT);
> +      tem = force_into_ssa_name (&gsi, tem, true);
>        gsi_prev (&gsi);
>      }
>    else if (gimple_code (stmt) != GIMPLE_PHI)
>      {
>        gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
> -      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, false,
> -				      GSI_CONTINUE_LINKING);
> +      tem = force_into_ssa_name (&gsi, tem, false);
>      }
>    else
>      {
> @@ -2419,8 +2437,7 @@ update_range_test (struct range_entry *r
>  	    }
>  	}
>        gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> -      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
> -				      GSI_SAME_STMT);
> +      tem = force_into_ssa_name (&gsi, tem, true);
>        if (gsi_end_p (gsi))
>  	gsi = gsi_last_bb (gimple_bb (stmt));
>        else
> --- gcc/testsuite/gcc.c-torture/compile/pr81003.c.jj	2017-06-08 12:16:20.284127013 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr81003.c	2017-06-08 12:16:07.000000000 +0200
> @@ -0,0 +1,10 @@
> +/* PR tree-optimization/81003 */
> +
> +unsigned int a, b;
> +
> +void
> +foo (void)
> +{
> +  for (b = 0; b < 13; b += 2)
> +    a &= !!b;
> +}
> 
> 	Jakub
> 
>
Jakub Jelinek June 9, 2017, 9:34 a.m. UTC | #2
On Fri, Jun 09, 2017 at 11:29:48AM +0200, Richard Biener wrote:
> > force_gimple_operand_gsi called by update_range_test can using match.pd
> > simplifications sometimes return INTEGER_CST (especially when cunroll
> > unrolled code isn't really optimized by forwprop/ccp and similar passes
> > before reassoc2), but that is something not acceptable to the rest of
> > the optimize_range* code, because it needs to know not just the value,
> > but also some gimple_stmt_iterator to insert related code etc.
> > 
> > This patch makes sure we have a SSA_NAME even in that case.
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Ok.
> 
> In the end this results in a missed optimization downstream but I
> guess we can't resolve this w/o major refactoring?

The constants should be propagated into uses soon afterwards, there
is both forwprop and ccp shortly after the first reassoc pass and
e.g. dom/vrp2 after second reassoc pass that should fix it up.

	Jakub
diff mbox

Patch

--- gcc/tree-ssa-reassoc.c.jj	2017-06-05 11:58:14.000000000 +0200
+++ gcc/tree-ssa-reassoc.c	2017-06-08 12:20:47.619790903 +0200
@@ -2282,6 +2282,26 @@  range_entry_cmp (const void *a, const vo
     }
 }
 
+/* Helper function for update_range_test.  Force EXPR into an SSA_NAME,
+   insert needed statements BEFORE or after GSI.  */
+
+static tree
+force_into_ssa_name (gimple_stmt_iterator *gsi, tree expr, bool before)
+{
+  enum gsi_iterator_update m = before ? GSI_SAME_STMT : GSI_CONTINUE_LINKING;
+  tree ret = force_gimple_operand_gsi (gsi, expr, true, NULL_TREE, before, m);
+  if (TREE_CODE (ret) != SSA_NAME)
+    {
+      gimple *g = gimple_build_assign (make_ssa_name (TREE_TYPE (ret)), ret);
+      if (before)
+	gsi_insert_before (gsi, g, GSI_SAME_STMT);
+      else
+	gsi_insert_after (gsi, g, GSI_CONTINUE_LINKING);
+      ret = gimple_assign_lhs (g);
+    }
+  return ret;
+}
+
 /* Helper routine of optimize_range_test.
    [EXP, IN_P, LOW, HIGH, STRICT_OVERFLOW_P] is a merged range for
    RANGE and OTHERRANGE through OTHERRANGE + COUNT - 1 ranges,
@@ -2393,15 +2413,13 @@  update_range_test (struct range_entry *r
   else if (op != range->exp)
     {
       gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
-      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
-				      GSI_SAME_STMT);
+      tem = force_into_ssa_name (&gsi, tem, true);
       gsi_prev (&gsi);
     }
   else if (gimple_code (stmt) != GIMPLE_PHI)
     {
       gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
-      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, false,
-				      GSI_CONTINUE_LINKING);
+      tem = force_into_ssa_name (&gsi, tem, false);
     }
   else
     {
@@ -2419,8 +2437,7 @@  update_range_test (struct range_entry *r
 	    }
 	}
       gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
-      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
-				      GSI_SAME_STMT);
+      tem = force_into_ssa_name (&gsi, tem, true);
       if (gsi_end_p (gsi))
 	gsi = gsi_last_bb (gimple_bb (stmt));
       else
--- gcc/testsuite/gcc.c-torture/compile/pr81003.c.jj	2017-06-08 12:16:20.284127013 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr81003.c	2017-06-08 12:16:07.000000000 +0200
@@ -0,0 +1,10 @@ 
+/* PR tree-optimization/81003 */
+
+unsigned int a, b;
+
+void
+foo (void)
+{
+  for (b = 0; b < 13; b += 2)
+    a &= !!b;
+}