tree-optimization/117965 - phiprop validity checking is too strict

The PR shows that when using std::clamp from the C++ standard library
and there is surrounding code using exceptions then phiprop can fail
to simplify the code so phiopt can turn the clamping into efficient
min/max operations.

The validation code is needlessly complicated, steming from the
time we had memory-SSA with multiple virtual operands.  The following
simplifies this, thereby fixing this issue.

	PR tree-optimization/117965
	* tree-ssa-phiprop.cc (phivn_valid_p): Remove.
	(propagate_with_phi): Pass in virtual PHI node from BB,
	rewrite load motion validity check to require the same
	virtual use along all paths.

	* g++.dg/tree-ssa/pr117965-1.C: New testcase.
	* g++.dg/tree-ssa/pr117965-2.C: Likewise.

3 files changed, 97 insertions, 46 deletions

diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr117965-1.C b/gcc/testsuite/g++.dg/tree-ssa/pr117965-1.C
new file mode 100644
index 0000000..84f0f2b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr117965-1.C
@@ -0,0 +1,28 @@
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-phiopt1" }
+
+void f();
+void f(int&);
+
+static inline const int &
+clamp(const int &v, const int &min, const int &max)
+{
+  const int &t = (v > min) ? v : min;
+  return t > max ? max : t;
+}
+
+void clamp2(int num1) {
+  try {
+  int low = -12, high = 12;
+  f();
+  num1 = clamp(num1, low, high);
+  f(num1);
+  } catch(...)
+  {
+       __builtin_printf("caught.\n");
+        return;
+  }
+}
+
+// { dg-final { scan-tree-dump-times "MAX" 1 "phiopt1" } } */
+// { dg-final { scan-tree-dump-times "MIN" 1 "phiopt1" } } */
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr117965-2.C b/gcc/testsuite/g++.dg/tree-ssa/pr117965-2.C
new file mode 100644
index 0000000..3e94fb3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr117965-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-O2 -fdump-tree-phiopt1" }
+
+#include <iostream>
+#include <algorithm>
+
+void clamp2 ()
+{
+  float low = -1.0f, high = 1.0f;
+  float num1, num2, num3;
+  std::cin >> num1;
+  num1 = std::clamp(num1, low, high);
+  std::cout << num1;
+}
+
+// { dg-final { scan-tree-dump-times " < -1.0" 1 "phiopt1" } }
+// { dg-final { scan-tree-dump-times " \\\? -1.0e\\\+0 : " 1 "phiopt1" } }
+// { dg-final { scan-tree-dump-times " > 1.0" 1 "phiopt1" } }
+// { dg-final { scan-tree-dump-times " \\\? 1.0e\\\+0 : " 1 "phiopt1" } }
diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc
index a2e1fb1..897bd58 100644
--- a/gcc/tree-ssa-phiprop.cc
+++ b/gcc/tree-ssa-phiprop.cc
@@ -99,35 +99,6 @@ struct phiprop_d
   tree vuse;
 };
 
-/* Verify if the value recorded for NAME in PHIVN is still valid at
-   the start of basic block BB.  */
-
-static bool
-phivn_valid_p (struct phiprop_d *phivn, tree name, basic_block bb)
-{
-  tree vuse = phivn[SSA_NAME_VERSION (name)].vuse;
-  gimple *use_stmt;
-  imm_use_iterator ui2;
-  bool ok = true;
-
-  /* The def stmts of the virtual uses need to be dominated by bb.  */
-  gcc_assert (vuse != NULL_TREE);
-
-  FOR_EACH_IMM_USE_STMT (use_stmt, ui2, vuse)
-    {
-      /* If BB does not dominate a VDEF, the value is invalid.  */
-      if ((gimple_vdef (use_stmt) != NULL_TREE
-	   || gimple_code (use_stmt) == GIMPLE_PHI)
-	  && !dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), bb))
-	{
-	  ok = false;
-	  break;
-	}
-    }
-
-  return ok;
-}
-
 /* Insert a new phi node for the dereference of PHI at basic_block
    BB with the virtual operands from USE_STMT.  */
 
@@ -275,12 +246,13 @@ chk_uses (tree, tree *idx, void *data)
       <Lx>:;
    Returns true if a transformation was done and edge insertions
    need to be committed.  Global data PHIVN and N is used to track
-   past transformation results.  We need to be especially careful here
+   past transformation results.  VPHI is the virtual PHI node in BB
+   if there is one.  We need to be especially careful here
    with aliasing issues as we are moving memory reads.  */
 
 static bool
-propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn,
-		    size_t n, bitmap dce_ssa_names)
+propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi,
+		    struct phiprop_d *phivn, size_t n, bitmap dce_ssa_names)
 {
   tree ptr = PHI_RESULT (phi);
   gimple *use_stmt;
@@ -298,6 +270,7 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn,
 	  && TYPE_MODE (TREE_TYPE (TREE_TYPE (ptr))) == BLKmode))
     return false;
 
+  tree up_vuse = NULL_TREE;
   /* Check if we can "cheaply" dereference all phi arguments.  */
   FOR_EACH_PHI_ARG (arg_p, phi, i, SSA_OP_USE)
     {
@@ -315,14 +288,28 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn,
 	    return false;
 	  arg = gimple_assign_rhs1 (def_stmt);
 	}
-      if (TREE_CODE (arg) != ADDR_EXPR
-	  && !(TREE_CODE (arg) == SSA_NAME
+      if (TREE_CODE (arg) == ADDR_EXPR)
+	;
+      /* When we have an SSA name see if we previously encountered a
+	 dereference of it.  */
+      else if (TREE_CODE (arg) == SSA_NAME
 	       && SSA_NAME_VERSION (arg) < n
 	       && phivn[SSA_NAME_VERSION (arg)].value != NULL_TREE
 	       && (!type
 		   || types_compatible_p
-		       (type, TREE_TYPE (phivn[SSA_NAME_VERSION (arg)].value)))
-	       && phivn_valid_p (phivn, arg, bb)))
+		       (type, TREE_TYPE (phivn[SSA_NAME_VERSION (arg)].value))))
+	{
+	  /* The dereference should be under the VUSE that's active in BB.
+	     If the BB has no virtual PHI then record the common "incoming"
+	     vuse.  */
+	  if (vphi)
+	    up_vuse = gimple_phi_arg_def (vphi, phi_arg_index_from_use (arg_p));
+	  if (!up_vuse)
+	    up_vuse = phivn[SSA_NAME_VERSION (arg)].vuse;
+	  else if (up_vuse != phivn[SSA_NAME_VERSION (arg)].vuse)
+	    return false;
+	}
+      else
 	return false;
       if (!type
 	  && TREE_CODE (arg) == SSA_NAME)
@@ -372,17 +359,32 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn,
 	    && !gimple_has_volatile_ops (use_stmt)))
 	continue;
 
-      /* Check if we can move the loads.  The def stmt of the virtual use
-	 needs to be in a different basic block dominating bb.  When the
+      /* Check if we can move the loads.  This is when the virtual use
+	 is the same as the one active at the start of BB which we know
+	 either from its virtual PHI def or from the common incoming
+	 VUSE.  If neither is present make sure the def stmt of the virtual
+	 use is in a different basic block dominating BB.  When the
 	 def is an edge-inserted one we know it dominates us.  */
       vuse = gimple_vuse (use_stmt);
-      def_stmt = SSA_NAME_DEF_STMT (vuse);
-      if (!SSA_NAME_IS_DEFAULT_DEF (vuse)
-	  && (gimple_bb (def_stmt) == bb
-	      || (gimple_bb (def_stmt)
-		  && !dominated_by_p (CDI_DOMINATORS,
-				      bb, gimple_bb (def_stmt)))))
-	goto next;
+      if (vphi)
+	{
+	  if (vuse != gimple_phi_result (vphi))
+	    goto next;
+	}
+      else if (up_vuse)
+	{
+	  if (vuse != up_vuse)
+	    goto next;
+	}
+      else
+	{
+	  def_stmt = SSA_NAME_DEF_STMT (vuse);
+	  if (!SSA_NAME_IS_DEFAULT_DEF (vuse)
+	      && (gimple_bb (def_stmt) == bb
+		  || !dominated_by_p (CDI_DOMINATORS,
+				      bb, gimple_bb (def_stmt))))
+	    goto next;
+	}
 
       /* Found a proper dereference with an aggregate copy.  Just
          insert aggregate copies on the edges instead.  */
@@ -535,8 +537,10 @@ pass_phiprop::execute (function *fun)
          edges avoid blocks with abnormal predecessors.  */
       if (bb_has_abnormal_pred (bb))
 	continue;
+      gphi *vphi = get_virtual_phi (bb);
       for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-	did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n, dce_ssa_names);
+	did_something |= propagate_with_phi (bb, vphi, gsi.phi (),
+					     phivn, n, dce_ssa_names);
     }
 
   if (did_something)