[PATCH] Fix __builtin_powi placement (PR52976 follow-up)

["William J. Schmidt" <wschmidt at linux dot vnet dot ibm dot com>]

The emergency patch for PR52976 manipulated the operand rank system to
force inserted __builtin_powi calls to occur before uses of the call
results.  However, this is generally the wrong approach, as it forces
other computations to move unnecessarily, and extends the lifetimes of
other operands.

This patch fixes the problem in the proper way, by letting the rank
system determine where the __builtin_powi call belongs, and moving the
call to that location during the expression rewrite.

Bootstrapped with no new regressions on powerpc64-linux.  SPEC cpu2000
and cpu2006 also build cleanly.  Ok for trunk?

Thanks,
Bill


gcc:

2012-04-17  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* tree-ssa-reassoc.c (add_to_ops_vec_max_rank): Delete.
	(possibly_move_powi): New function.
	(rewrite_expr_tree): Call possibly_move_powi.
	(rewrite_expr_tree_parallel): Likewise.
	(attempt_builtin_powi): Change call of add_to_ops_vec_max_rank to
	call add_to_ops_vec instead.


gcc/testsuite:

2012-04-17  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	gfortran.dg/reassoc_11.f: New test.



Index: gcc/testsuite/gfortran.dg/reassoc_11.f
===================================================================
--- gcc/testsuite/gfortran.dg/reassoc_11.f	(revision 0)
+++ gcc/testsuite/gfortran.dg/reassoc_11.f	(revision 0)
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! { dg-options "-O3 -ffast-math" }
+
+! This tests only for compile-time failure, which formerly occurred
+! when a __builtin_powi was introduced by reassociation in a bad place.
+
+      SUBROUTINE GRDURBAN(URBWSTR, ZIURB, GRIDHT)
+
+      IMPLICIT NONE
+      INTEGER :: I
+      REAL :: SW2, URBWSTR, ZIURB, GRIDHT(87)
+
+      SAVE 
+
+      SW2 = 1.6*(GRIDHT(I)/ZIURB)**0.667*URBWSTR**2
+
+      END
Index: gcc/tree-ssa-reassoc.c
===================================================================
--- gcc/tree-ssa-reassoc.c	(revision 186495)
+++ gcc/tree-ssa-reassoc.c	(working copy)
@@ -544,28 +544,6 @@ add_repeat_to_ops_vec (VEC(operand_entry_t, heap)
   reassociate_stats.pows_encountered++;
 }
 
-/* Add an operand entry to *OPS for the tree operand OP, giving the
-   new entry a larger rank than any other operand already in *OPS.  */
-
-static void
-add_to_ops_vec_max_rank (VEC(operand_entry_t, heap) **ops, tree op)
-{
-  operand_entry_t oe = (operand_entry_t) pool_alloc (operand_entry_pool);
-  operand_entry_t oe1;
-  unsigned i;
-  unsigned max_rank = 0;
-
-  FOR_EACH_VEC_ELT (operand_entry_t, *ops, i, oe1)
-    if (oe1->rank > max_rank)
-      max_rank = oe1->rank;
-
-  oe->op = op;
-  oe->rank = max_rank + 1;
-  oe->id = next_operand_entry_id++;
-  oe->count = 1;
-  VEC_safe_push (operand_entry_t, heap, *ops, oe);
-}
-
 /* Return true if STMT is reassociable operation containing a binary
    operation with tree code CODE, and is inside LOOP.  */
 
@@ -2162,6 +2242,47 @@ remove_visited_stmt_chain (tree var)
     }
 }
 
+/* If OP is an SSA name, find its definition and determine whether it
+   is a call to __builtin_powi.  If so, move the definition prior to
+   STMT.  Only do this during early reassociation.  */
+
+static void
+possibly_move_powi (gimple stmt, tree op)
+{
+  gimple stmt2;
+  tree fndecl;
+  gimple_stmt_iterator gsi1, gsi2;
+
+  if (!first_pass_instance
+      || !flag_unsafe_math_optimizations
+      || TREE_CODE (op) != SSA_NAME)
+    return;
+  
+  stmt2 = SSA_NAME_DEF_STMT (op);
+
+  if (!is_gimple_call (stmt2)
+      || !has_single_use (gimple_call_lhs (stmt2)))
+    return;
+
+  fndecl = gimple_call_fndecl (stmt2);
+
+  if (!fndecl
+      || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+    return;
+
+  switch (DECL_FUNCTION_CODE (fndecl))
+    {
+    CASE_FLT_FN (BUILT_IN_POWI):
+      break;
+    default:
+      return;
+    }
+
+  gsi1 = gsi_for_stmt (stmt);
+  gsi2 = gsi_for_stmt (stmt2);
+  gsi_move_before (&gsi2, &gsi1);
+}
+
 /* This function checks three consequtive operands in
    passed operands vector OPS starting from OPINDEX and
    swaps two operands if it is profitable for binary operation
@@ -2267,6 +2388,8 @@ rewrite_expr_tree (gimple stmt, unsigned int opind
 	      print_gimple_stmt (dump_file, stmt, 0, 0);
 	    }
 
+	  possibly_move_powi (stmt, oe1->op);
+	  possibly_move_powi (stmt, oe2->op);
 	}
       return;
     }
@@ -2312,6 +2435,8 @@ rewrite_expr_tree (gimple stmt, unsigned int opind
 	  fprintf (dump_file, " into ");
 	  print_gimple_stmt (dump_file, stmt, 0, 0);
 	}
+
+      possibly_move_powi (stmt, oe->op);
     }
   /* Recurse on the LHS of the binary operator, which is guaranteed to
      be the non-leaf side.  */
@@ -2485,6 +2610,9 @@ rewrite_expr_tree_parallel (gimple stmt, int width
 	  fprintf (dump_file, " into ");
 	  print_gimple_stmt (dump_file, stmts[i], 0, 0);
 	}
+
+      possibly_move_powi (stmts[i], op1);
+      possibly_move_powi (stmts[i], op2);
     }
 
   remove_visited_stmt_chain (last_rhs1);
@@ -3196,6 +3324,8 @@ attempt_builtin_powi (gimple stmt, VEC(operand_ent
 							   power));
 	      gimple_call_set_lhs (pow_stmt, iter_result);
 	      gimple_set_location (pow_stmt, gimple_location (stmt));
+	      /* Temporarily place the call; we will move it to the
+		 correct place during rewrite_expr.  */
 	      gsi_insert_before (&gsi, pow_stmt, GSI_SAME_STMT);
 
 	      if (dump_file && (dump_flags & TDF_DETAILS))
@@ -3300,10 +3430,8 @@ attempt_builtin_powi (gimple stmt, VEC(operand_ent
 	  gsi_insert_before (&gsi, pow_stmt, GSI_SAME_STMT);
 	}
 
-      /* Append the result of this iteration to the ops vector.
-         Give it a rank higher than all other ranks in the ops vector
-         so that all uses of it will be forced to come after it.  */
-      add_to_ops_vec_max_rank (ops, iter_result);
+      /* Append the result of this iteration to the ops vector.  */
+      add_to_ops_vec (ops, iter_result);
 
       /* Decrement the occurrence count of each element in the product
 	 by the count found above, and remove this many copies of each