Patchwork Fix condition propagation in forwprop, fixup locations in phiopt

login
register
mail settings
Submitter Richard Guenther
Date July 20, 2011, 12:16 p.m.
Message ID <alpine.LNX.2.00.1107201413100.810@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/105667/
State New
Headers show

Comments

Richard Guenther - July 20, 2011, 12:16 p.m.
This fixes one of the fallouts when boolifying comparisons.  Namely
that we do not manage to propagate multiple times into a condition
statement.  That's because we fail to remove unused defs and thus
do not maintain a single-use chain for the second propagation.
Fixed by removing dead defs after simplifying a condition.

Phi-opt also forgot to set proper locations on the comparison it
emits as separate statement which lead to even more confusing
locations printed in warnings.  Also fixed (with that fix only we report
for the xfailed location which would also be ok).

Bootstrap and regtest running on x86_64-unknown-linux-gnu, I'll commit
this if that succeeds.

Richard.

2011-07-20  Richard Guenther  <rguenther@suse.de>

	* tree-ssa-forwprop.c (forward_propagate_into_comparison):
	Remove dead defining stmts.
	(forward_propagate_into_gimple_cond): Likewise.
	(forward_propagate_into_cond): Simplify.
	(ssa_forward_propagate_and_combine): Handle changed cfg from
	forward_propagate_into_comparison.
	* tree-ssa-phiopt.c (conditional_replacement): Use proper
	locations for newly built statements.

Index: gcc/tree-ssa-phiopt.c
===================================================================
*** gcc/tree-ssa-phiopt.c	(revision 176497)
--- gcc/tree-ssa-phiopt.c	(working copy)
*************** conditional_replacement (basic_block con
*** 544,551 ****
    /* To handle special cases like floating point comparison, it is easier and
       less error-prone to build a tree and gimplify it on the fly though it is
       less efficient.  */
!   cond = fold_build2 (gimple_cond_code (stmt), boolean_type_node,
! 		      gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
  
    /* We need to know which is the true edge and which is the false
       edge so that we know when to invert the condition below.  */
--- 544,552 ----
    /* To handle special cases like floating point comparison, it is easier and
       less error-prone to build a tree and gimplify it on the fly though it is
       less efficient.  */
!   cond = fold_build2_loc (gimple_location (stmt),
! 			  gimple_cond_code (stmt), boolean_type_node,
! 			  gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
  
    /* We need to know which is the true edge and which is the false
       edge so that we know when to invert the condition below.  */
*************** conditional_replacement (basic_block con
*** 554,560 ****
        || (e0 == false_edge && integer_onep (arg0))
        || (e1 == true_edge && integer_zerop (arg1))
        || (e1 == false_edge && integer_onep (arg1)))
!     cond = fold_build1 (TRUTH_NOT_EXPR, TREE_TYPE (cond), cond);
  
    /* Insert our new statements at the end of conditional block before the
       COND_STMT.  */
--- 555,562 ----
        || (e0 == false_edge && integer_onep (arg0))
        || (e1 == true_edge && integer_zerop (arg1))
        || (e1 == false_edge && integer_onep (arg1)))
!     cond = fold_build1_loc (gimple_location (stmt),
! 			    TRUTH_NOT_EXPR, TREE_TYPE (cond), cond);
  
    /* Insert our new statements at the end of conditional block before the
       COND_STMT.  */

Patch

Index: gcc/tree-ssa-forwprop.c
===================================================================
--- gcc/tree-ssa-forwprop.c.orig	2011-07-20 14:04:03.000000000 +0200
+++ gcc/tree-ssa-forwprop.c	2011-07-20 14:02:28.000000000 +0200
@@ -437,29 +437,36 @@  forward_propagate_into_comparison_1 (loc
 
 /* Propagate from the ssa name definition statements of the assignment
    from a comparison at *GSI into the conditional if that simplifies it.
-   Returns true if the stmt was modified, false if not.  */
+   Returns 1 if the stmt was modified and 2 if the CFG needs cleanup,
+   otherwise returns 0.  */
 
-static bool
+static int 
 forward_propagate_into_comparison (gimple_stmt_iterator *gsi)
 {
   gimple stmt = gsi_stmt (*gsi);
   tree tmp;
+  bool cfg_changed = false;
+  tree rhs1 = gimple_assign_rhs1 (stmt);
+  tree rhs2 = gimple_assign_rhs2 (stmt);
 
   /* Combine the comparison with defining statements.  */
   tmp = forward_propagate_into_comparison_1 (gimple_location (stmt),
 					     gimple_assign_rhs_code (stmt),
 					     TREE_TYPE
 					       (gimple_assign_lhs (stmt)),
-					     gimple_assign_rhs1 (stmt),
-					     gimple_assign_rhs2 (stmt));
+					     rhs1, rhs2);
   if (tmp)
     {
       gimple_assign_set_rhs_from_tree (gsi, tmp);
       update_stmt (stmt);
-      return true;
+      if (TREE_CODE (rhs1) == SSA_NAME)
+	cfg_changed |= remove_prop_source_from_use (rhs1);
+      if (TREE_CODE (rhs2) == SSA_NAME)
+	cfg_changed |= remove_prop_source_from_use (rhs2);
+      return cfg_changed ? 2 : 1;
     }
 
-  return false;
+  return 0;
 }
 
 /* Propagate from the ssa name definition statements of COND_EXPR
@@ -472,10 +479,12 @@  forward_propagate_into_comparison (gimpl
 static int
 forward_propagate_into_gimple_cond (gimple stmt)
 {
-  int did_something = 0;
   location_t loc = gimple_location (stmt);
   tree tmp;
   enum tree_code code = gimple_cond_code (stmt);
+  bool cfg_changed = false;
+  tree rhs1 = gimple_cond_lhs (stmt);
+  tree rhs2 = gimple_cond_rhs (stmt);
 
   /* We can do tree combining on SSA_NAME and comparison expressions.  */
   if (TREE_CODE_CLASS (gimple_cond_code (stmt)) != tcc_comparison)
@@ -483,18 +492,13 @@  forward_propagate_into_gimple_cond (gimp
 
   tmp = forward_propagate_into_comparison_1 (loc, code,
 					     boolean_type_node,
-					     gimple_cond_lhs (stmt),
-					     gimple_cond_rhs (stmt));
+					     rhs1, rhs2);
   if (tmp)
     {
       if (dump_file && tmp)
 	{
-	  tree cond = build2 (gimple_cond_code (stmt),
-			      boolean_type_node,
-			      gimple_cond_lhs (stmt),
-			      gimple_cond_rhs (stmt));
 	  fprintf (dump_file, "  Replaced '");
-	  print_generic_expr (dump_file, cond, 0);
+	  print_gimple_expr (dump_file, stmt, 0, 0);
 	  fprintf (dump_file, "' with '");
 	  print_generic_expr (dump_file, tmp, 0);
 	  fprintf (dump_file, "'\n");
@@ -503,14 +507,14 @@  forward_propagate_into_gimple_cond (gimp
       gimple_cond_set_condition_from_tree (stmt, unshare_expr (tmp));
       update_stmt (stmt);
 
-      /* Remove defining statements.  */
-      if (is_gimple_min_invariant (tmp))
-	did_something = 2;
-      else if (did_something == 0)
-	did_something = 1;
+      if (TREE_CODE (rhs1) == SSA_NAME)
+	cfg_changed |= remove_prop_source_from_use (rhs1);
+      if (TREE_CODE (rhs2) == SSA_NAME)
+	cfg_changed |= remove_prop_source_from_use (rhs2);
+      return (cfg_changed || is_gimple_min_invariant (tmp)) ? 2 : 1;
     }
 
-  return did_something;
+  return 0;
 }
 
 
@@ -526,7 +530,6 @@  forward_propagate_into_cond (gimple_stmt
 {
   gimple stmt = gsi_stmt (*gsi_p);
   location_t loc = gimple_location (stmt);
-  int did_something = 0;
   tree tmp = NULL_TREE;
   tree cond = gimple_assign_rhs1 (stmt);
 
@@ -541,7 +544,7 @@  forward_propagate_into_cond (gimple_stmt
       tree name = cond, rhs0;
       gimple def_stmt = get_prop_source_stmt (name, true, NULL);
       if (!def_stmt || !can_propagate_from (def_stmt))
-	return did_something;
+	return 0;
 
       rhs0 = gimple_assign_rhs1 (def_stmt);
       tmp = combine_cond_expr_cond (loc, NE_EXPR, boolean_type_node, rhs0,
@@ -564,14 +567,10 @@  forward_propagate_into_cond (gimple_stmt
       stmt = gsi_stmt (*gsi_p);
       update_stmt (stmt);
 
-      /* Remove defining statements.  */
-      if (is_gimple_min_invariant (tmp))
-	did_something = 2;
-      else if (did_something == 0)
-	did_something = 1;
+      return is_gimple_min_invariant (tmp) ? 2 : 1;
     }
 
-  return did_something;
+  return 0;
 }
 
 /* We've just substituted an ADDR_EXPR into stmt.  Update all the
@@ -2433,11 +2432,15 @@  ssa_forward_propagate_and_combine (void)
 		else if (TREE_CODE_CLASS (code) == tcc_comparison)
 		  {
 		    bool no_warning = gimple_no_warning_p (stmt);
+		    int did_something;
 		    fold_defer_overflow_warnings ();
-		    changed = forward_propagate_into_comparison (&gsi);
+		    did_something = forward_propagate_into_comparison (&gsi);
+		    if (did_something == 2)
+		      cfg_changed = true;
 		    fold_undefer_overflow_warnings
 			(!no_warning && changed,
 			 stmt, WARN_STRICT_OVERFLOW_CONDITIONAL);
+		    changed = did_something != 0;
 		  }
 		else if (code == BIT_AND_EXPR
 			 || code == BIT_IOR_EXPR