On Tue, Jun 3, 2014 at 3:48 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
> Hello,
>
> apparently it is possible to have a PHI in the middle basic block of
> value_replacement, so I need to move it as well when I move the statement
> and remove the block.
>
> Bootstrap and testsuite on x86_64-linux-gnu (re-running for various reasons
> but they had completed successfully yesterday).
>
> 2014-06-03 Marc Glisse <marc.gli...@inria.fr>
>
> PR tree-optimization/61385
> gcc/
> * tree-ssa-phiopt.c (value_replacement): Copy PHI nodes before
> removing the basic block.
> gcc/testsuite/
> * gcc.dg/tree-ssa/pr61385.c: New file.
>
> --
> Marc Glisse
> Index: 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,20 +877,39 @@ 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))))
> {
> + /* Move potential PHI nodes. */
> + gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
> + while (!gsi_end_p (psi))
> + {
> + gimple phi_moving = gsi_stmt (psi);
> + gimple newphi = create_phi_node (gimple_phi_result (phi_moving),
> + cond_bb);
> + int nargs = cond_bb->preds->length();
> + location_t loc = gimple_phi_arg_location (phi_moving, 0);
> + tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
> + for (int i = 0; i < nargs; ++i)
> + {
> + edge e = (*cond_bb->preds)[i];
> + add_phi_arg (newphi, phi_arg, e, loc);
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.
Richard.
> + }
> + update_stmt (newphi);
> + gsi_remove (&psi, false);
> + }
> +
> gsi = gsi_for_stmt (cond);
> 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;
> }
>
>
