diff mbox

TLC to reassoc get_rank

Message ID alpine.LSU.2.11.1505291440570.30088@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener May 29, 2015, 12:41 p.m. UTC 
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

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

	* tree-ssa-reassoc.c (get_rank): Simplify.

Comments

Jakub Jelinek May 29, 2015, 12:44 p.m. UTC | #1 
On Fri, May 29, 2015 at 02:41:28PM +0200, Richard Biener wrote:
> @@ -525,7 +498,11 @@ get_rank (tree e)
>        return (rank + 1);
>      }
>  
> -  /* Globals, etc,  are rank 0 */
> +  /* Constants have rank 0.  */
> +  if (is_gimple_min_invariant (e))
> +    return 0;
> +
> +  /* Constants, globals, etc., are rank 0 */
>    return 0;
>  }

This looks weird.  No reason to test is_gimple_min_invariant
if it returns 0 no matter whether it is true or false.

	Jakub
Richard Biener May 29, 2015, 12:45 p.m. UTC | #2 
On Fri, 29 May 2015, Jakub Jelinek wrote:

> On Fri, May 29, 2015 at 02:41:28PM +0200, Richard Biener wrote:
> > @@ -525,7 +498,11 @@ get_rank (tree e)
> >        return (rank + 1);
> >      }
> >  
> > -  /* Globals, etc,  are rank 0 */
> > +  /* Constants have rank 0.  */
> > +  if (is_gimple_min_invariant (e))
> > +    return 0;
> > +
> > +  /* Constants, globals, etc., are rank 0 */
> >    return 0;
> >  }
> 
> This looks weird.  No reason to test is_gimple_min_invariant
> if it returns 0 no matter whether it is true or false.

Whoops I already adjusted the comment but forgot to remove the
stmt!  (just moved it down after the cheap == SSA_NAME check
initially).

Consider that fixed.

Richard.
diff mbox

Patch

Index: gcc/tree-ssa-reassoc.c
===================================================================
--- gcc/tree-ssa-reassoc.c	(revision 223818)
+++ gcc/tree-ssa-reassoc.c	(working copy)
@@ -417,10 +417,6 @@  insert_operand_rank (tree e, long rank)
 static long
 get_rank (tree e)
 {
-  /* Constants have rank 0.  */
-  if (is_gimple_min_invariant (e))
-    return 0;
-
   /* SSA_NAME's have the rank of the expression they are the result
      of.
      For globals and uninitialized values, the rank is 0.
@@ -460,9 +456,9 @@  get_rank (tree e)
 
   if (TREE_CODE (e) == SSA_NAME)
     {
+      ssa_op_iter iter;
       gimple stmt;
       long rank;
-      int i, n;
       tree op;
 
       if (SSA_NAME_IS_DEFAULT_DEF (e))
@@ -472,8 +468,7 @@  get_rank (tree e)
       if (gimple_code (stmt) == GIMPLE_PHI)
 	return phi_rank (stmt);
 
-      if (!is_gimple_assign (stmt)
-	  || gimple_vdef (stmt))
+      if (!is_gimple_assign (stmt))
 	return bb_rank[gimple_bb (stmt)->index];
 
       /* If we already have a rank for this expression, use that.  */
@@ -484,34 +479,12 @@  get_rank (tree e)
       /* Otherwise, find the maximum rank for the operands.  As an
 	 exception, remove the bias from loop-carried phis when propagating
 	 the rank so that dependent operations are not also biased.  */
+      /* Simply walk over all SSA uses - this takes advatage of the
+         fact that non-SSA operands are is_gimple_min_invariant and
+	 thus have rank 0.  */
       rank = 0;
-      if (gimple_assign_single_p (stmt))
-	{
-	  tree rhs = gimple_assign_rhs1 (stmt);
-	  n = TREE_OPERAND_LENGTH (rhs);
-	  if (n == 0)
-	    rank = propagate_rank (rank, rhs);
-	  else
-	    {
-	      for (i = 0; i < n; i++)
-		{
-		  op = TREE_OPERAND (rhs, i);
-
-		  if (op != NULL_TREE)
-		    rank = propagate_rank (rank, op);
-		}
-	    }
-	}
-      else
-	{
-	  n = gimple_num_ops (stmt);
-	  for (i = 1; i < n; i++)
-	    {
-	      op = gimple_op (stmt, i);
-	      gcc_assert (op);
-	      rank = propagate_rank (rank, op);
-	    }
-	}
+      FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
+	rank = propagate_rank (rank, op);
 
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
@@ -525,7 +498,11 @@  get_rank (tree e)
       return (rank + 1);
     }
 
-  /* Globals, etc,  are rank 0 */
+  /* Constants have rank 0.  */
+  if (is_gimple_min_invariant (e))
+    return 0;
+
+  /* Constants, globals, etc., are rank 0 */
   return 0;
 }