[PATCH] Improve stmt folding with match-and-simplify

[PATCH] Improve stmt folding with match-and-simplify

[Richard Biener]

This avoids regressions when removing the remaining dispatching to
fold () from fold_stmt.  First, when a valueization is rejected by
stmt replacement (for example when propagating an abnormal, where
the patch improves some cases here as well) then we fail to do
operand order canonicalization on the non-valueized stmt.  The
patch fixes this by explicitely canonicalizing operands of the
original stmt first.  Second, some uses of abnormals on a stmt
replacement are ok.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2015-07-29  Richard Biener  <rguenther@suse.de>

	* gimple-fold.c (has_use_on_stmt): New function.
	(replace_stmt_with_simplification): Use it to allow
	abnormals originally referenced in the stmt.
	(fold_stmt_1): Canonicalize operand order.

Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 226308)
+++ gcc/gimple-fold.c	(working copy)
@@ -3307,6 +3307,19 @@ gimple_fold_call (gimple_stmt_iterator *
 }
 
 
+/* Return true whether NAME has a use on STMT.  */
+
+static bool
+has_use_on_stmt (tree name, gimple stmt)
+{
+  imm_use_iterator iter;
+  use_operand_p use_p;
+  FOR_EACH_IMM_USE_FAST (use_p, iter, name)
+    if (USE_STMT (use_p) == stmt)
+      return true;
+  return false;
+}
+
 /* Worker for fold_stmt_1 dispatch to pattern based folding with
    gimple_simplify.
 
@@ -3322,15 +3335,20 @@ replace_stmt_with_simplification (gimple
   gimple stmt = gsi_stmt (*gsi);
 
   /* Play safe and do not allow abnormals to be mentioned in
-     newly created statements.  See also maybe_push_res_to_seq.  */
+     newly created statements.  See also maybe_push_res_to_seq.
+     As an exception allow such uses if there was a use of the
+     same SSA name on the old stmt.  */
   if ((TREE_CODE (ops[0]) == SSA_NAME
-       && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[0]))
+       && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[0])
+       && !has_use_on_stmt (ops[0], stmt))
       || (ops[1]
 	  && TREE_CODE (ops[1]) == SSA_NAME
-	  && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[1]))
+	  && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[1])
+	  && !has_use_on_stmt (ops[1], stmt))
       || (ops[2]
 	  && TREE_CODE (ops[2]) == SSA_NAME
-	  && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[2])))
+	  && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[2])
+	  && !has_use_on_stmt (ops[2], stmt)))
     return false;
 
   if (gcond *cond_stmt = dyn_cast <gcond *> (stmt))
@@ -3531,7 +3549,8 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
      after propagation.
      ???  This shouldn't be done in generic folding but in the
      propagation helpers which also know whether an address was
-     propagated.  */
+     propagated.
+     Also canonicalize operand order.  */
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
@@ -3547,6 +3566,27 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
 	      && maybe_canonicalize_mem_ref_addr (lhs))
 	    changed = true;
 	}
+      else
+	{
+	  /* Canonicalize operand order.  */
+	  enum tree_code code = gimple_assign_rhs_code (stmt);
+	  if (TREE_CODE_CLASS (code) == tcc_comparison
+	      || commutative_tree_code (code)
+	      || commutative_ternary_tree_code (code))
+	    {
+	      tree rhs1 = gimple_assign_rhs1 (stmt);
+	      tree rhs2 = gimple_assign_rhs2 (stmt);
+	      if (tree_swap_operands_p (rhs1, rhs2, false))
+		{
+		  gimple_assign_set_rhs1 (stmt, rhs2);
+		  gimple_assign_set_rhs2 (stmt, rhs1);
+		  if (TREE_CODE_CLASS (code) == tcc_comparison)
+		    gimple_assign_set_rhs_code (stmt,
+						swap_tree_comparison (code));
+		  changed = true;
+		}
+	    }
+	}
       break;
     case GIMPLE_CALL:
       {
@@ -3597,6 +3637,21 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
 	    changed = true;
 	}
       break;
+    case GIMPLE_COND:
+      {
+	/* Canonicalize operand order.  */
+	tree lhs = gimple_cond_lhs (stmt);
+	tree rhs = gimple_cond_rhs (stmt);
+	if (tree_swap_operands_p (lhs, rhs, false))
+	  {
+	    gcond *gc = as_a <gcond *> (stmt);
+	    gimple_cond_set_lhs (gc, rhs);
+	    gimple_cond_set_rhs (gc, lhs);
+	    gimple_cond_set_code (gc,
+				  swap_tree_comparison (gimple_cond_code (gc)));
+	    changed = true;
+	  }
+      }
     default:;
     }
 

Re: [PATCH] Improve stmt folding with match-and-simplify

[Marek Polacek]

On Wed, Jul 29, 2015 at 09:44:36AM +0200, Richard Biener wrote:
> @@ -3547,6 +3566,27 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>  	      && maybe_canonicalize_mem_ref_addr (lhs))
>  	    changed = true;
>  	}
> +      else
> +	{
> +	  /* Canonicalize operand order.  */
> +	  enum tree_code code = gimple_assign_rhs_code (stmt);
> +	  if (TREE_CODE_CLASS (code) == tcc_comparison
> +	      || commutative_tree_code (code)
> +	      || commutative_ternary_tree_code (code))
> +	    {
> +	      tree rhs1 = gimple_assign_rhs1 (stmt);
> +	      tree rhs2 = gimple_assign_rhs2 (stmt);
> +	      if (tree_swap_operands_p (rhs1, rhs2, false))
> +		{
> +		  gimple_assign_set_rhs1 (stmt, rhs2);
> +		  gimple_assign_set_rhs2 (stmt, rhs1);
> +		  if (TREE_CODE_CLASS (code) == tcc_comparison)

Is the second check for tcc_comparison needed?  We checked for that a few
lines above.

> +		    gimple_assign_set_rhs_code (stmt,
> +						swap_tree_comparison (code));
> +		  changed = true;

	Marek

Re: [PATCH] Improve stmt folding with match-and-simplify

[Richard Biener]

On Tue, 11 Aug 2015, Marek Polacek wrote:

> On Wed, Jul 29, 2015 at 09:44:36AM +0200, Richard Biener wrote:
> > @@ -3547,6 +3566,27 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
> >  	      && maybe_canonicalize_mem_ref_addr (lhs))
> >  	    changed = true;
> >  	}
> > +      else
> > +	{
> > +	  /* Canonicalize operand order.  */
> > +	  enum tree_code code = gimple_assign_rhs_code (stmt);
> > +	  if (TREE_CODE_CLASS (code) == tcc_comparison
> > +	      || commutative_tree_code (code)
> > +	      || commutative_ternary_tree_code (code))
> > +	    {
> > +	      tree rhs1 = gimple_assign_rhs1 (stmt);
> > +	      tree rhs2 = gimple_assign_rhs2 (stmt);
> > +	      if (tree_swap_operands_p (rhs1, rhs2, false))
> > +		{
> > +		  gimple_assign_set_rhs1 (stmt, rhs2);
> > +		  gimple_assign_set_rhs2 (stmt, rhs1);
> > +		  if (TREE_CODE_CLASS (code) == tcc_comparison)
> 
> Is the second check for tcc_comparison needed?  We checked for that a few
> lines above.
> 
> > +		    gimple_assign_set_rhs_code (stmt,
> > +						swap_tree_comparison (code));
> > +		  changed = true;

Yep because it might also be a non-comparison.

> 	Marek
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)