tree-optimization/103596 - fix missed propagation into switches

Message ID 7966o545-4ps8-qsqn-4q75-os829nn1p5rr@fhfr.qr
State New
Headers
Series tree-optimization/103596 - fix missed propagation into switches |

Commit Message

Richard Biener Dec. 7, 2021, 1:07 p.m. UTC
  may_propagate_copy unnecessarily restricts propagating non-abnormals
into places that currently contain an abnormal SSA name but are
not the PHI argument for an abnormal edge.  This causes VN to
not elide a CFG path that it assumes is elided, resulting in
released SSA names in the IL.

The fix is to enhance the may_propagate_copy API to specify the
destination is _not_ a PHI argument.  I chose to not update only
the relevant caller in VN and the may_propagate_copy_into_stmt API
at this point because this is a regression and needs backporting.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2021-12-07  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/103596
	* tree-ssa-sccvn.c (eliminate_dom_walker::eliminate_stmt):
	Note we are not propagating into a PHI argument to may_propagate_copy.
	* tree-ssa-propagate.h (may_propagate_copy): Add
	argument specifying whether we propagate into a PHI arg.
	* tree-ssa-propagate.c (may_propagate_copy): Likewise.
	When not doing so we can replace an abnormal with
	something else.
	(may_propagate_into_stmt): Update may_propagate_copy calls.
	(replace_exp_1): Move propagation checking code to
	propagate_value and rename to ...
	(replace_exp): ... this and elide previous wrapper.
	(propagate_value): Perform checking with adjusted
	may_propagate_copy call and dispatch to replace_exp.

	* gcc.dg/torture/pr103596.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr103596.c | 36 ++++++++++++++
 gcc/tree-ssa-propagate.c                | 62 ++++++++++---------------
 gcc/tree-ssa-propagate.h                |  2 +-
 gcc/tree-ssa-sccvn.c                    |  2 +-
 4 files changed, 62 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr103596.c
  

Patch

diff --git a/gcc/testsuite/gcc.dg/torture/pr103596.c b/gcc/testsuite/gcc.dg/torture/pr103596.c
new file mode 100644
index 00000000000..4f65c36d121
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr103596.c
@@ -0,0 +1,36 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "--param case-values-threshold=1" } */
+
+int n;
+
+void
+qux (int a)
+{
+}
+
+int
+baz (void)
+{
+  return -1;
+}
+
+__attribute__ ((returns_twice)) int
+bar (int b)
+{
+  if (n != 0)
+    {
+      if (b != 2)
+        if (b != 0)
+          return n + b;
+
+      if (n == 2)
+        return 0;
+    }
+}
+
+void
+foo (void)
+{
+  qux (n);
+  bar (baz ());
+}
diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 6d19410caa8..1f2a17f73c9 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -1042,10 +1042,12 @@  substitute_and_fold_engine::substitute_and_fold (basic_block block)
 }
 
 
-/* Return true if we may propagate ORIG into DEST, false otherwise.  */
+/* Return true if we may propagate ORIG into DEST, false otherwise.
+   If DEST_NOT_PHI_ARG_P is true then assume the propagation does
+   not happen into a PHI argument which relaxes some constraints.  */
 
 bool
-may_propagate_copy (tree dest, tree orig)
+may_propagate_copy (tree dest, tree orig, bool dest_not_phi_arg_p)
 {
   tree type_d = TREE_TYPE (dest);
   tree type_o = TREE_TYPE (orig);
@@ -1065,8 +1067,10 @@  may_propagate_copy (tree dest, tree orig)
 	   && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig))
     return false;
   /* Similarly if DEST flows in from an abnormal edge then the copy cannot be
-     propagated.  */
-  else if (TREE_CODE (dest) == SSA_NAME
+     propagated.  If we know we do not propagate into a PHI argument this
+     does not apply.  */
+  else if (!dest_not_phi_arg_p
+	   && TREE_CODE (dest) == SSA_NAME
 	   && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (dest))
     return false;
 
@@ -1100,9 +1104,9 @@  may_propagate_copy_into_stmt (gimple *dest, tree orig)
      for the expression, so we delegate to may_propagate_copy.  */
 
   if (gimple_assign_single_p (dest))
-    return may_propagate_copy (gimple_assign_rhs1 (dest), orig);
+    return may_propagate_copy (gimple_assign_rhs1 (dest), orig, true);
   else if (gswitch *dest_swtch = dyn_cast <gswitch *> (dest))
-    return may_propagate_copy (gimple_switch_index (dest_swtch), orig);
+    return may_propagate_copy (gimple_switch_index (dest_swtch), orig, true);
 
   /* In other cases, the expression is not materialized, so there
      is no destination to pass to may_propagate_copy.  On the other
@@ -1140,25 +1144,19 @@  may_propagate_copy_into_asm (tree dest ATTRIBUTE_UNUSED)
 }
 
 
-/* Common code for propagate_value and replace_exp.
+/* Replace *OP_P with value VAL (assumed to be a constant or another SSA_NAME).
 
-   Replace use operand OP_P with VAL.  FOR_PROPAGATION indicates if the
-   replacement is done to propagate a value or not.  */
+   Use this version when not const/copy propagating values.  For example,
+   PRE uses this version when building expressions as they would appear
+   in specific blocks taking into account actions of PHI nodes.
 
-static void
-replace_exp_1 (use_operand_p op_p, tree val,
-    	       bool for_propagation ATTRIBUTE_UNUSED)
-{
-  if (flag_checking)
-    {
-      tree op = USE_FROM_PTR (op_p);
-      gcc_assert (!(for_propagation
-		  && TREE_CODE (op) == SSA_NAME
-		  && TREE_CODE (val) == SSA_NAME
-		  && !may_propagate_copy (op, val)));
-    }
+   The statement in which an expression has been replaced should be
+   folded using fold_stmt_inplace.  */
 
-  if (TREE_CODE (val) == SSA_NAME)
+void
+replace_exp (use_operand_p op_p, tree val)
+{
+  if (TREE_CODE (val) == SSA_NAME || CONSTANT_CLASS_P (val))
     SET_USE (op_p, val);
   else
     SET_USE (op_p, unshare_expr (val));
@@ -1174,22 +1172,10 @@  replace_exp_1 (use_operand_p op_p, tree val,
 void
 propagate_value (use_operand_p op_p, tree val)
 {
-  replace_exp_1 (op_p, val, true);
-}
-
-/* Replace *OP_P with value VAL (assumed to be a constant or another SSA_NAME).
-
-   Use this version when not const/copy propagating values.  For example,
-   PRE uses this version when building expressions as they would appear
-   in specific blocks taking into account actions of PHI nodes.
-
-   The statement in which an expression has been replaced should be
-   folded using fold_stmt_inplace.  */
-
-void
-replace_exp (use_operand_p op_p, tree val)
-{
-  replace_exp_1 (op_p, val, false);
+  if (flag_checking)
+    gcc_assert (may_propagate_copy (USE_FROM_PTR (op_p), val,
+				    !is_a <gphi *> (USE_STMT (op_p))));
+  replace_exp (op_p, val);
 }
 
 
diff --git a/gcc/tree-ssa-propagate.h b/gcc/tree-ssa-propagate.h
index 5257fbb0cf8..87a94ad17c3 100644
--- a/gcc/tree-ssa-propagate.h
+++ b/gcc/tree-ssa-propagate.h
@@ -65,7 +65,7 @@  enum ssa_prop_result {
 
 extern void move_ssa_defining_stmt_for_defs (gimple *, gimple *);
 extern bool stmt_makes_single_store (gimple *);
-extern bool may_propagate_copy (tree, tree);
+extern bool may_propagate_copy (tree, tree, bool = false);
 extern bool may_propagate_copy_into_stmt (gimple *, tree);
 extern bool may_propagate_copy_into_asm (tree);
 extern void propagate_value (use_operand_p, tree);
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index d31bf329d2e..16c93d1be78 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -6607,7 +6607,7 @@  eliminate_dom_walker::eliminate_stmt (basic_block b, gimple_stmt_iterator *gsi)
 	   at the definition are also available at uses.  */
 	sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), use);
       if (sprime && sprime != use
-	  && may_propagate_copy (use, sprime)
+	  && may_propagate_copy (use, sprime, true)
 	  /* We substitute into debug stmts to avoid excessive
 	     debug temporaries created by removed stmts, but we need
 	     to avoid doing so for inserted sprimes as we never want