Re: PR61385: phiopt drops some PHIs

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Jun 4, 2014 at 1:20 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Wed, 4 Jun 2014, Richard Biener wrote:
>
>> So I'd say we should instead simply bail out if the middle-bb has a PHI
>> node.
>
>
> Sounds good to me, so I am testing the mini-patch I had originally posted to
> bugzilla:

Ok.

Thanks,
Richard.

>
> 2014-06-04  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR tree-optimization/61385
> gcc/
>         * tree-ssa-phiopt.c (value_replacement): Punt if there are PHI
> nodes.
>
> 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 211221)
> +++ gcc/tree-ssa-phiopt.c       (working copy)
> @@ -842,20 +842,24 @@ value_replacement (basic_block cond_bb,
>    /* Now optimize (x != 0) ? x + y : y to just y.
>       The following condition is too restrictive, there can easily be
> another
>       stmt in middle_bb, for instance a CONVERT_EXPR for the second
> argument.  */
>    gimple assign = last_and_only_stmt (middle_bb);
>    if (!assign || gimple_code (assign) != GIMPLE_ASSIGN
>        || gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS
>        || (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
>           && !POINTER_TYPE_P (TREE_TYPE (arg0))))
>      return 0;
>
> +  /* Punt if there are (degenerate) PHIs in middle_bb, there should not be.
> */
> +  if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
> +    return 0;
> +
>    /* Only transform if it removes the condition.  */
>    if (!single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)), e0,
> e1))
>      return 0;
>
>    /* Size-wise, this is always profitable.  */
>    if (optimize_bb_for_speed_p (cond_bb)
>        /* The special case is useless if it has a low probability.  */
>        && profile_status_for_fn (cfun) != PROFILE_ABSENT
>        && EDGE_PRED (middle_bb, 0)->probability < PROB_EVEN
>        /* If assign is cheap, there is no point avoiding it.  */
>