phiprop: Allow for one store inbetween the load and the phi which is being used to insert [PR123120]

So phiprop has one disadvantage is that if there is store between the
phi with the addresses and the new load, phiprop will no do anything.
This means for some C++ code where you have a min of a max (or the opposite),
depending on the argument order of evaluation phiprop might do
the transformation or it might not (see tree-ssa/phiprop-3.C for examples).
So we need to allow skipping of one store inbetween the load and
where the phi is located.

Aggregates include a store when doing phiprop so we need to check
if there are also loads between the original store/load and the
store we are skipping. This can be added afterwards but I didn't
see aggregate case happening enough to make a big dent. I added
testcases (phiprop-{10,11}.c) to make sure cases where the load
would make a different shows up though.

changes since v1:
* v2: rewrite can_handle_load to avoid duplicated skipping store code.

	PR tree-optimization/123120
	PR tree-optimization/116823
gcc/ChangeLog:

	* tree-ssa-phiprop.cc (phiprop_insert_phi): Add other_vuse
	argument, use it instead of the vuse on the use_stmt.
	(can_handle_load): Add aggregate argument. Also return the vuse
	of the load/store when the insert is allowed.
	Skipping over one non-modifying store for !aggregate.
	(propagate_with_phi): Update call to can_handle_load
	and phiprop_insert_phi.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/phiprop-8.c: New test.
	* gcc.dg/tree-ssa/phiprop-9.c: New test.
	* gcc.dg/tree-ssa/phiprop-10.c: New test.
	* gcc.dg/tree-ssa/phiprop-11.c: New test.
	* gcc.dg/tree-ssa/phiprop-12.c: New test.
	* g++.dg/tree-ssa/phiprop-3.C: New test.

Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>

gcc/testsuite/g++.dg/tree-ssa/phiprop-3.C

@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */
+
+/* PR tree-optimization/123120 */
+/* The order of min/max arguments should not cause any issues with phiprop,
+   the stores to the temps should not interfere with the loads from the temp.  */
+
+#include <algorithm>
+
+long f0(short c, short d, long long a)
+{
+   return std::max(std::min((long long)3, a), (long long)d);
+}
+
+long f1(short c, short d, long long a)
+{
+   return std::max((long long)d, std::min((long long)3, a));
+}
+
+/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 4 "phiprop1"} } */
+/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "release_ssa"} } */
+/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "release_ssa"} } */
+

gcc/testsuite/gcc.dg/tree-ssa/phiprop-10.c

@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-sra -fdump-tree-phiprop-details" } */
+/* PR tree-optimization/123120 */
+/* Make sure the store to *e conflicts with the store to *d */
+
+
+struct s1
+{
+  int a;
+};
+
+void f(int *);
+
+void g(struct s1 i, struct s1 *d, struct s1 *e)
+{
+  const struct s1 t = {10};
+  const struct s1 *a;
+  struct s1 t1 = {2};
+  if (t.a < i.a)
+    a = &t;
+  else
+    a = &i;
+  *e = t1;
+  t1 = *a;
+  *d = t1;
+}
+
+/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop1"} } */
+/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop2"} } */
+

gcc/testsuite/gcc.dg/tree-ssa/phiprop-11.c

@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-sra -fdump-tree-phiprop-details" } */
+/* PR tree-optimization/123120 */
+/* Make sure the load *d conflicts with the store to *d so that phiprop
+   does not move the store into conditionals. */
+
+struct s1
+{
+  int a;
+};
+
+void f(int *);
+
+int g(struct s1 i, struct s1 *d, struct s1 *e)
+{
+  const struct s1 t = {10};
+  const struct s1 *a;
+  struct s1 t1 = {2};
+  if (t.a < i.a)
+    a = &t;
+  else
+    a = &i;
+  t1 = *d;
+  *d = *a;
+  return t1.a;
+}
+
+/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop1"} } */
+/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop2"} } */
+

gcc/testsuite/gcc.dg/tree-ssa/phiprop-12.c

@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-phiprop-details" } */
+/* PR tree-optimization/123120 */
+/* Make sure the load *d conflicts with the store to *d so that phiprop
+   does not move the store into conditionals. */
+
+struct s1
+{
+  int a;
+};
+
+void f(int *);
+
+int g(struct s1 i, struct s1 *d, struct s1 *e)
+{
+  const struct s1 t = {10};
+  const struct s1 *a;
+  struct s1 t1 = {2};
+  if (t.a < i.a)
+    a = &t;
+  else
+    a = &i;
+  int t2 = d->a;
+  *d = *a;
+  return t2;
+}
+
+/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop1"} } */
+/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" "phiprop2"} } */
+

gcc/testsuite/gcc.dg/tree-ssa/phiprop-8.c

@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */
+
+/* PR tree-optimization/116823 */
+/* The clobber on b should not get in the way of phiprop here. */
+/* We should have MIN_EXPR early on. */
+
+void f(int *);
+
+int g(int i)
+{
+  const int t = 10;
+  const int *a;
+  {
+    int b;
+    f(&b);
+    if (t < i)
+      a = &t;
+    else
+      a = &i;
+  }
+  return *a;
+}
+
+/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */
+/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "release_ssa"} } */
+

gcc/testsuite/gcc.dg/tree-ssa/phiprop-9.c

@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */
+
+/* PR tree-optimization/123120 */
+/* The store to *d should not get in the way of phiprop here. */
+/* We should have MIN_EXPR early on. */
+
+void f(int *);
+
+int g(int i, int *d)
+{
+  const int t = 10;
+  const int *a;
+  if (t < i)
+    a = &t;
+  else
+    a = &i;
+  *d = 1;
+  return *a;
+}
+
+/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */
+/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "release_ssa"} } */
+

gcc/tree-ssa-phiprop.cc

@@ -101,12 +101,14 @@ struct phiprop_d
 };
 
 /* Insert a new phi node for the dereference of PHI at basic_block
-   BB with the virtual operands from USE_STMT.  */
+   BB with the virtual operands from USE_STMT. The vuse for
+   the load will be set to OTHER_VUSE unless there is virtual op
+   phi for BB.  */
 
 static tree
 phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt,
 		    struct phiprop_d *phivn, size_t n,
-		    bitmap dce_ssa_names)
+		    bitmap dce_ssa_names, tree other_vuse)
 {
   tree res;
   gphi *new_phi = NULL;
@@ -176,7 +178,7 @@ phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt,
 	      if (vphi)
 		vuse = PHI_ARG_DEF_FROM_EDGE (vphi, e);
 	      else
-		vuse = gimple_vuse (use_stmt);
+		vuse = other_vuse;
 	    }
 	  else
 	    /* For the aggregate copy case updating virtual operands
@@ -243,26 +245,85 @@ chk_uses (tree, tree *idx, void *data)
    incoming VUSE (up_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.  */
-static bool
-can_handle_load (gimple *load_stmt, basic_block bb,
-		 gphi *vphi, tree up_vuse)
+   is an edge-inserted one we know it dominates us.
+   Returns the vuse to use for the inserting.  NULL_TREE
+   is returned when we can't do the insert.  */
+
+static tree
+can_handle_load (gimple *load_stmt,
+		 basic_block bb,
+		 gphi *vphi, tree up_vuse, bool aggregate)
 {
   tree vuse = gimple_vuse (load_stmt);
-  if (vphi)
-    return vuse == gimple_phi_result (vphi);
-  if (up_vuse)
-    return vuse == up_vuse;
-  gimple *def_stmt = SSA_NAME_DEF_STMT (vuse);
   /* If the load does not have a store beforehand,
      then we can do the load in conditional. */
   if (SSA_NAME_IS_DEFAULT_DEF (vuse))
-    return true;
-  if (gimple_bb (def_stmt) != bb
+    {
+      /* For loads that have no stores before, there should be no
+	 vphi.  */
+      gcc_checking_assert (!vphi);
+      /* The common vuse is the same as the default or there is none. */
+      gcc_checking_assert (!up_vuse || up_vuse == vuse);
+      return vuse;
+    }
+
+  /* If we have a vphi, then that needs to be end point.
+     If we have a common incoming vuse, that needs to be the end point.  */
+  tree expected_vuse = NULL_TREE;
+  if (vphi)
+    expected_vuse = gimple_phi_result (vphi);
+  else if (up_vuse)
+    expected_vuse = up_vuse;
+
+  /* If the vuse on the load is the same as the expected vuse,
+     there are no stores inbetween.  */
+  if (vuse == expected_vuse)
+    return vuse;
+  /* Try to see if the store does not effect the load.  */
+  gimple *other_store = SSA_NAME_DEF_STMT (vuse);
+  /* Only handling the case where the store is in the same
+     bb as the phi.  */
+  if (gimple_bb (other_store) == bb)
+    {
+      /* For aggregates, skipping the store is too
+	 hard to handle as you need to check for loads
+	 and it is not worth the extra checks. */
+      if (aggregate)
+	return NULL_TREE;
+      tree src = gimple_assign_rhs1 (load_stmt);
+      ao_ref read;
+      ao_ref_init (&read, src);
+      if (stmt_may_clobber_ref_p_1 (other_store, &read, false))
+	return NULL_TREE;
+      vuse = gimple_vuse (other_store);
+      /* If that skipped store was the first store in program,
+	 then we can do the load conditional.  */
+      if (SSA_NAME_IS_DEFAULT_DEF (vuse))
+	{
+	  /* For loads that have no stores before, there should be no
+	     vphi.  */
+	  gcc_checking_assert (!vphi);
+	  /* The common vuse is the same as the default or there is none. */
+	  gcc_checking_assert (!up_vuse || up_vuse == vuse);
+	  return vuse;
+	}
+      other_store = SSA_NAME_DEF_STMT (vuse);
+      /* If the new vuse (after skipping) is the same as expected
+	 then that is the vuse to return.  */
+      if (vuse == expected_vuse)
+	return vuse;
+      if (gimple_bb (other_store) == bb)
+	return NULL_TREE;
+    }
+
+  /* If there was no an expected vuse then see if the vuse dominates the phi of
+      the address.  */
+  if (!expected_vuse
       && dominated_by_p (CDI_DOMINATORS,
-			 bb, gimple_bb (def_stmt)))
-    return true;
-  return false;
+			 bb, gimple_bb (other_store)))
+    return vuse;
+
+  return NULL_TREE;
 }
 
 /* Propagate between the phi node arguments of PHI in BB and phi result
@@ -385,13 +446,15 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi,
 	    && !gimple_has_volatile_ops (use_stmt)))
 	continue;
 
-      if (!can_handle_load (use_stmt, bb, vphi, up_vuse))
-	continue;
-
       bool aggregate = false;
       if (!is_gimple_reg_type (TREE_TYPE (gimple_assign_lhs (use_stmt))))
 	aggregate = true;
 
+      tree other_vuse;
+      other_vuse = can_handle_load (use_stmt, bb, vphi, up_vuse, aggregate);
+      if (!other_vuse)
+	continue;
+
       if ((canpossible_trap || aggregate)
 	  && !dom_info_available_p (cfun, CDI_POST_DOMINATORS))
 	calculate_dominance_info (CDI_POST_DOMINATORS);
@@ -459,7 +522,8 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi,
 		goto next;
 	    }
 
-	  phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names);
+	  phiprop_insert_phi (bb, phi, use_stmt, phivn, n,
+			      dce_ssa_names, other_vuse);
 
 	  /* Remove old stmt. The phi and all of maybe its depedencies
 	     will be removed later via simple_dce_from_worklist. */
@@ -492,7 +556,8 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi *phi,
       else
 	{
 	  tree vuse = gimple_vuse (use_stmt);
-	  res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names);
+	  res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n,
+				    dce_ssa_names, other_vuse);
 	  type = TREE_TYPE (res);
 
 	  /* Remember the value we created for *ptr.  */