cselim: Allow middle_bb to have more than one statement [PR124405]

Currently, cselim requires the middle_bb to have only a single statement. This
patch relaxes this restriction as long as all uses (except the rhs) in the
single stmatement are also available where we insert to.

Bootstrapped and tested on x86_64-linux-gnu and aarch64-linux-gnu.

Changes since v1:
* v2: Revert changes to trailing_store_in_bb and do not call
      trailing_store_in_bb to get split_assign.
      Remove some unnecessary checks in cselim_candidate.
      Pass the result of cselim_candidate which is the candidate store for
      cselim as an argument to cond_store_replacement instead.
* v3: Simplify cselim_candidate by always calling trailing_store_in_bb and
      removing the legality checks.
      Generalize the cond_store_replacement legality checks to account for the
      case when the middle_bb can have more than one statement.

	PR tree-optimization/124405

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (cond_store_replacement): Make ASSIGN (the
	candidate store for cselim) an argument of the function instead. Update
	legality check due to relaxing the restriction that middle_bb can have
	only a single statement.
	(cselim_candidate): New.
	(pass_cselim::execute): Call cselim_candidate and pass the result to
	cond_store_replacement.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/pr124405.c: New test.

Signed-off-by: Pengxuan Zheng <pengxuan.zheng@oss.qualcomm.com>

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr124405.c b/gcc/testsuite/gcc.dg/tree-ssa/pr124405.c
new file mode 100644 (file)
index 0000000..9ba230d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr124405.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-cselim-details" } */
+
+void
+f (int *a, int b)
+{
+  *a &= ~3;
+  if (b)
+    *a |= 1;
+}
+
+/* { dg-final { scan-tree-dump-times "Conditional store replacement happened" 1 "cselim"} } */

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 1bdb37092990408cbdbf25b4bdd9b7a909b43083..6a7e4c70722afe6bf3e13569bb3cc109217187c7 100644 (file)
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -2999,16 +2999,16 @@ get_non_trapping (void)
    JOIN_BB:
      some more
 
-   We check that MIDDLE_BB contains only one store, that that store
+   ASSIGN is a store in MIDDLE_BB which is the candidate for cselim.  We check
+   that MIDDLE_BB contains only one store (i.e., ASSIGN), that that store
    doesn't trap (not via NOTRAP, but via checking if an access to the same
-   memory location dominates us, or the store is to a local addressable
-   object) and that the store has a "simple" RHS.  */
+   memory location dominates us, or the store is to a local addressable object)
+   and that the store has a "simple" RHS.  */
 
 static bool
-cond_store_replacement (basic_block middle_bb, basic_block join_bb,
-                       edge e0, edge e1, hash_set<tree> *nontrap)
+cond_store_replacement (basic_block middle_bb, basic_block join_bb, edge e0,
+                       edge e1, gimple *assign, hash_set<tree> *nontrap)
 {
-  gimple *assign = last_and_only_stmt (middle_bb);
   tree lhs, rhs, name, name2;
   gphi *newphi;
   gassign *new_stmt;
@@ -3021,11 +3021,6 @@ cond_store_replacement (basic_block middle_bb, basic_block join_bb,
       || gimple_has_volatile_ops (assign))
     return false;
 
-  /* And no PHI nodes so all uses in the single stmt are also
-     available where we insert to.  */
-  if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
-    return false;
-
   locus = gimple_location (assign);
   lhs = gimple_assign_lhs (assign);
   rhs = gimple_assign_rhs1 (assign);
@@ -3034,6 +3029,20 @@ cond_store_replacement (basic_block middle_bb, basic_block join_bb,
       || !is_gimple_reg_type (TREE_TYPE (lhs)))
     return false;
 
+  /* Make sure all uses (except the rhs) in the single stmt are also available
+     where we insert to.  */
+  ssa_op_iter iter;
+  tree use;
+  FOR_EACH_SSA_TREE_OPERAND (use, assign, iter, SSA_OP_USE)
+    {
+      if (use == rhs)
+       continue;
+
+      gimple *stmt = SSA_NAME_DEF_STMT (use);
+      if (stmt && gimple_bb (stmt) == middle_bb)
+       return false;
+    }
+
   /* Prove that we can move the store down.  We could also check
      TREE_THIS_NOTRAP here, but in that case we also could move stores,
      whose value is not available readily, which we want to avoid.  */
@@ -3297,6 +3306,20 @@ trailing_store_in_bb (basic_block bb, tree vdef, gphi *vphi, bool onlyonestore)
   return store;
 }
 
+/* Return the only store in MIDDLE_BB as the candidate store for cselim.  Return
+   NULL if no candidate can be found.  */
+
+static gimple *
+cselim_candidate (basic_block middle_bb, basic_block join_bb, edge e0)
+{
+  gphi *vphi = get_virtual_phi (join_bb);
+  if (!vphi)
+    return NULL;
+
+  tree middle_vdef = PHI_ARG_DEF_FROM_EDGE (vphi, e0);
+  return trailing_store_in_bb (middle_bb, middle_vdef, vphi, true);
+}
+
 /* Limited Conditional store replacement.  We already know
    that the recognized pattern looks like so:
 
@@ -4268,7 +4291,9 @@ pass_cselim::execute (function *)
         optimization if the join block has more than two predecessors.  */
       if (EDGE_COUNT (bb2->preds) > 2)
        return;
-      if (cond_store_replacement (bb1, bb2, e1, e2, nontrap))
+
+      gimple *assign = cselim_candidate (bb1, bb2, e1);
+      if (cond_store_replacement (bb1, bb2, e1, e2, assign, nontrap))
        cfgchanged = true;
     };