Re: [PATCH] Don't transform virtual PHIs into COND_EXPRs in tree ifcvt (PR tree-optimization/34140)

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Nov 28, 2007 2:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On the attached testcase loop unswitching creates from admitedly
> superfluous virtual PHI:
>   # tD.1550_45 = PHI <tD.1550_47(13), tD.1550_47(11)>
>   # tD.1550_46 = VDEF <tD.1550_45> { tD.1550 }
>   tD.1550.tD.1542[iD.1549_43].sD.1540 = prephitmp.14D.1578_44;
> created by loop unswitching a COND_EXPR, which is not valid
> gimple:
> tD.1550_45 = D.1561_49 != 0 ? tD.1550_47 : tD.1550_47;
> (t is an aggregate).  While perhaps loop unswitching should be fixed not to
> create PHIs with all args identical, I believe various other optimizations
> can create these and even if it would be desirable to optimize them out,
> we probably wouldn't do it after every pass, so I think the primary
> bug is on the tree-if-conv.c side.  Only non-virtual PHIs can be really
> converted, and as all PHIs on BBs other than loop header must be converted,
> we should IMHO just not if-convert such loops.
>
> I have tested that on no make check testcase other than the newly added one
> we hit this.  Bootstrap/regression testing on x86_64-linux pending, ok
> for trunk if it succeeds?

Yes, this makes perfect sense.

Thanks,
Richard.

> 2007-11-28  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/34140
>         * tree-if-conv.c (if_convertible_phi_p): Fail if BB other than
>         loop->header has virtual phi nodes.
>
>         * gcc.c-torture/compile/20071128-1.c: New test.
>
> --- gcc/tree-if-conv.c.jj       2007-11-02 19:02:47.000000000 +0100
> +++ gcc/tree-if-conv.c  2007-11-28 13:15:37.000000000 +0100
> @@ -302,7 +302,8 @@ tree_if_convert_cond_expr (struct loop *
>     and it belongs to basic block BB.
>     PHI is not if-convertible
>     - if it has more than 2 arguments.
> -   - Virtual PHI is immediately used in another PHI node.  */
> +   - Virtual PHI is immediately used in another PHI node.
> +   - Virtual PHI on BB other than header.  */
>
>  static bool
>  if_convertible_phi_p (struct loop *loop, basic_block bb, tree phi)
> @@ -324,6 +325,13 @@ if_convertible_phi_p (struct loop *loop,
>      {
>        imm_use_iterator imm_iter;
>        use_operand_p use_p;
> +
> +      if (bb != loop->header)
> +       {
> +         if (dump_file && (dump_flags & TDF_DETAILS))
> +           fprintf (dump_file, "Virtual phi not on loop header.\n");
> +         return false;
> +       }
>        FOR_EACH_IMM_USE_FAST (use_p, imm_iter, PHI_RESULT (phi))
>         {
>           if (TREE_CODE (USE_STMT (use_p)) == PHI_NODE)
> --- gcc/testsuite/gcc.c-torture/compile/20071128-1.c.jj 2007-11-28 13:32:06.000000000 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/20071128-1.c    2007-11-28 13:45:27.000000000 +0100
> @@ -0,0 +1,24 @@
> +/* PR tree-optimization/34140 */
> +/* Testcase by Martin Michlmayr <tbm@cyrius.com> */
> +
> +struct S
> +{
> +  unsigned int s;
> +};
> +struct T
> +{
> +  struct S t[2];
> +  unsigned int u : 1;
> +};
> +
> +void
> +foo (int x, int y, int z)
> +{
> +  int i;
> +  struct T t;
> +
> +  t.u = t.u;
> +  for (i = 0; i < x; i++)
> +    if (z != 1)
> +      t.t[i].s = y || t.u;
> +}
>
>         Jakub
>