On Tue, 3 Jun 2014, Jeff Law wrote:
On 06/03/14 08:08, Richard Biener wrote:
All arguments get the same value (and the PHI in middle-bb is surely
a singleton?), so it's way better to re-materialize the PHI as a
gimple assignment at the start of the basic block. If they are
singletons (which I expect), the easiest way is to propagate their
single argument into all uses and simply remove them.
I was all for propagating, but replace_uses_by calls fold_stmt on the
using statements (that's normal). If I first move the statement and then
replace, I may call fold_stmt on a stmt that isn't dominated by the defs
of all its arguments. If I first replace, the statement I am supposed to
move may have been modified so I need to find out what happened to it. In
the end, the assignment seems easier and safer (and not too bad since this
case almost never happens in practice).
We certainly want to get rid of them :-)
I'd start by finding out which pass left the degenerate, ensure it's not
marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI, then propagate it away.
If it's a systemic problem that a particular pass can leave degenerate PHIs,
then you might want to schedule the phi-only propagator to run after that
pass. It does const/copy propagation seeding from degenerate PHIs, so it
ought to be very fast.
This one comes from VRP2 apparently. But it isn't the only pass that
produces singleton phis. I didn't look at them closely, but I assume LIM
is normal, DOM maybe less so, etc.
bootstrap+testsuite on x86_64-linux-gnu as usual.
2014-06-04 Marc Glisse <marc.gli...@inria.fr>
PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Replace singleton PHIs
with assignments.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.
--
Marc GlisseIndex: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+ int *h = &c;
+ for (; c < 1; c++)
+ {
+ int *i = &a, *k = &a;
+ f = 0;
+ if (b)
+ return 0;
+ if (*h)
+ {
+ int **j = &i;
+ *j = 0;
+ d = 0;
+ }
+ else
+ g = e = 0;
+ if (*h)
+ {
+ int **l = &k;
+ *l = &g;
+ }
+ d &= *h;
+ assert (k == &a || k);
+ assert (i);
+ }
+ return 0;
+}
+
+int
+main ()
+{
+ fn1 ();
+ return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===================================================================
--- gcc/tree-ssa-phiopt.c (revision 211178)
+++ gcc/tree-ssa-phiopt.c (working copy)
@@ -877,21 +877,35 @@ value_replacement (basic_block cond_bb,
&& operand_equal_for_phi_arg_p (rhs2, cond_lhs)
&& neutral_element_p (code_def, cond_rhs, true))
|| (arg1 == rhs2
&& operand_equal_for_phi_arg_p (rhs1, cond_lhs)
&& neutral_element_p (code_def, cond_rhs, false))
|| (operand_equal_for_phi_arg_p (arg1, cond_rhs)
&& (operand_equal_for_phi_arg_p (rhs2, cond_lhs)
|| operand_equal_for_phi_arg_p (rhs1, cond_lhs))
&& absorbing_element_p (code_def, cond_rhs))))
{
+ /* Any PHI node is a singleton and we can replace it with an assignment.
+ We don't just call replace_uses_by because it calls fold_stmt and it
+ becomes hard to make all the right adjustments in the right order. */
gsi = gsi_for_stmt (cond);
+ gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
+ while (!gsi_end_p (psi))
+ {
+ gimple phi_moving = gsi_stmt (psi);
+ tree phi_res = gimple_phi_result (phi_moving);
+ tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
+ gsi_remove (&psi, false);
+ gsi_insert_before (&gsi, gimple_build_assign (phi_res, phi_arg),
+ GSI_SAME_STMT);
+ }
+
gimple_stmt_iterator gsi_from = gsi_for_stmt (assign);
gsi_move_before (&gsi_from, &gsi);
replace_phi_edge_with_variable (cond_bb, e1, phi, lhs);
return 2;
}
return 0;
}
/* The function minmax_replacement does the main work of doing the minmax
