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

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

[Jakub Jelinek]

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?

2022-04-11  Jakub Jelinek  <jakub@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

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

[Richard Biener]

> Am 11.04.2022 um 19:00 schrieb Jakub Jelinek <jakub@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  <jakub@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
>