Re: [PATCH] phiopt: Fix up debug handling in the (x != cst1 ? x : cst2) != cst3 opt [PR105218]

[Richard Biener via Gcc-patches]

> Am 11.04.2022 um 19:00 schrieb Jakub Jelinek <ja...@redhat.com>:
> 
> Hi!
> 
> In the PR104639 optimization, I've added code to emit
>  # DEBUG D#1 => arg != carg ? arg : oarg
> instruction and replace debug uses of the phi with that debug
> temp, so that the debug info is still accurrate.
> Unfortunately, that is only correct if the middle-bb and
> phi bb contain 1 and 2 predecessors, i.e. the ones that
> we are using in the optimization (in particular middle-bb has
> cond-bb as pred and phi bb cond-bb and middle-bb).
> If that is not the case, then we can reach these from another bb
> and so the arg SSA_NAME might not be valid there (its definition
> doesn't dominate all incoming edges), or, even if it is valid,
> might be wrong-debug, e.g. phi argument from some unrelated other
> incoming edge might have the carg value that the debug stmt
> remaps to oarg.  In theory we could check for that case and
> if middle-bb doesn't have a single pred or phi bb 2 preds
> check if arg SSA_NAME dominates the phi bb and if all other
> phi arguments are expr_not_equal_to the carg value, but this patch
> just uses a simpler approach and resets already if we have some
> extra incoming edges.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard 


> 2022-04-11  Jakub Jelinek  <ja...@redhat.com>
> 
>    PR tree-optimization/105218
>    * tree-ssa-phiopt.cc (value_replacement): If middle_bb has
>    more than one predecessor or phi's bb more than 2 predecessors,
>    reset phi result uses instead of adding a debug temp.
> 
>    * gcc.dg/pr105218.c: New test.
> 
> --- gcc/tree-ssa-phiopt.cc.jj    2022-04-11 10:44:20.282985872 +0200
> +++ gcc/tree-ssa-phiopt.cc    2022-04-11 16:16:06.645348016 +0200
> @@ -1454,6 +1454,7 @@ value_replacement (basic_block cond_bb,
>          imm_use_iterator imm_iter;
>          tree phires = gimple_phi_result (phi);
>          tree temp = NULL_TREE;
> +          bool reset_p = false;
> 
>          /* Add # DEBUG D#1 => arg != carg ? arg : oarg.  */
>          FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, phires)
> @@ -1462,6 +1463,16 @@ value_replacement (basic_block cond_bb,
>            continue;
>              if (temp == NULL_TREE)
>            {
> +              if (!single_pred_p (middle_bb)
> +                  || EDGE_COUNT (gimple_bb (phi)->preds) != 2)
> +                {
> +                  /* But only if middle_bb has a single
> +                 predecessor and phi bb has two, otherwise
> +                 we could use a SSA_NAME not usable in that
> +                 place or wrong-debug.  */
> +                  reset_p = true;
> +                  break;
> +                }
>              gimple_stmt_iterator gsi
>                = gsi_after_labels (gimple_bb (phi));
>              tree type = TREE_TYPE (phires);
> @@ -1476,6 +1487,8 @@ value_replacement (basic_block cond_bb,
>            replace_exp (use_p, temp);
>              update_stmt (use_stmt);
>            }
> +          if (reset_p)
> +            reset_debug_uses (phi);
>        }
>        }
>      if (equal_p)
> --- gcc/testsuite/gcc.dg/pr105218.c.jj    2022-04-11 16:09:15.172101168 +0200
> +++ gcc/testsuite/gcc.dg/pr105218.c    2022-04-11 16:08:59.387321866 +0200
> @@ -0,0 +1,16 @@
> +/* PR tree-optimization/105218 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +int a, c;
> +void bar (void);
> +
> +void
> +foo (void)
> +{
> +  int b = 131;
> +  if (a)
> +    b = c == 2 ? 1 : c;
> +  while (b)
> +    bar ();
> +}
> 
>    Jakub
>