Re: PR61385: phiopt drops some PHIs

[Marc Glisse <marc dot glisse at inria dot fr>]

On Tue, 3 Jun 2014, Richard Biener wrote:

> On Tue, Jun 3, 2014 at 3:48 PM, Marc Glisse <marc.glisse@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.glisse@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?),

Yes, there is a single incoming edge to middle-bb.

> so it's way better to re-materialize the PHI as a
> gimple assignment at the start of the basic block.

I thought there might be a reason why it was a PHI and not an assignment 
so I wasn't sure doing that would be ok. It is indeed much easier, so I'll 
do that...

> If they are
> singletons (which I expect), the easiest way is to propagate their
> single argument into all uses and simply remove them.

... or indeed that, now that I have found a function called 
replace_uses_by which should do exactly what I need.

Thanks,

--
Marc Glisse