[PATCH PR82726][1/2]Revert previous fixes for PR70754 and PR79663

[Bin Cheng <Bin dot Cheng at arm dot com>]

Hi,
When fixing PR70754, I thought the issue only happens for ZERO-length chains.
Well, that's apparently not true with PR82726.
The whole story is, with chain combination/re-association, new stmts may be
created/inserted at position not dominating following uses.  This happens in
two scenarios:
  1) Zero length chains, as in PR70754.
  2) Non-zero chains with multiple zero distance references.
PR82726 falls in case 2).  Because zero distance references are root of the
chain, they don't inherit values from loop carried PHIs.  In code generation,
we still need to be careful not inserting use before definitions.

Previous fix to PR70754 tries to find dominance position for insertion when
combining all references.  I could do the similar thing on top of that fix,
but it would be inefficient/complicated because we should only do that for
zero distance references in a non-zero length combined chain.

This patch set fixes both PRs in the opposite way: Instead of finding dominance
insertion position for root reference, we re-sort zero-distance references of
combined chain by their position information so that new root reference must
dominate others.  This should be more efficient because we avoid function call
to stmt_dominates_stmt_p.

This is the first patch reverting r244815 and r245689.

Bootstrap and test on x86_64 and AArch64 in patch set.  Is it OK?

Thanks,
bin
2017-11-02  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/82726
	Revert
	2017-01-23  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/70754
	* tree-predcom.c (stmt_combining_refs): New parameter INSERT_BEFORE.
	(reassociate_to_the_same_stmt): New parameter INSERT_BEFORE.  Insert
	combined stmt before it if not NULL.
	(combine_chains): Process refs reversely and compute dominance point
	for root ref.

	Revert
	2017-02-23  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/79663
	* tree-predcom.c (combine_chains): Process refs in reverse order
	only for ZERO length chains, and add explaining comment.

From 408c86c33670ce64e9872fa9d4cc66fe0b3bffa4 Mon Sep 17 00:00:00 2001
From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com>
Date: Wed, 1 Nov 2017 12:53:43 +0000
Subject: [PATCH 1/2] revert-244815-245689.txt

---
 gcc/tree-predcom.c | 64 +++++++++++-------------------------------------------
 1 file changed, 13 insertions(+), 51 deletions(-)

diff --git a/gcc/tree-predcom.c b/gcc/tree-predcom.c
index fdb32f1..24d7c9c 100644
--- a/gcc/tree-predcom.c
+++ b/gcc/tree-predcom.c
@@ -2520,11 +2520,10 @@ remove_name_from_operation (gimple *stmt, tree op)
 }
 
 /* Reassociates the expression in that NAME1 and NAME2 are used so that they
-   are combined in a single statement, and returns this statement.  Note the
-   statement is inserted before INSERT_BEFORE if it's not NULL.  */
+   are combined in a single statement, and returns this statement.  */
 
 static gimple *
-reassociate_to_the_same_stmt (tree name1, tree name2, gimple *insert_before)
+reassociate_to_the_same_stmt (tree name1, tree name2)
 {
   gimple *stmt1, *stmt2, *root1, *root2, *s1, *s2;
   gassign *new_stmt, *tmp_stmt;
@@ -2581,12 +2580,6 @@ reassociate_to_the_same_stmt (tree name1, tree name2, gimple *insert_before)
   var = create_tmp_reg (type, "predreastmp");
   new_name = make_ssa_name (var);
   new_stmt = gimple_build_assign (new_name, code, name1, name2);
-  if (insert_before && stmt_dominates_stmt_p (insert_before, s1))
-    bsi = gsi_for_stmt (insert_before);
-  else
-    bsi = gsi_for_stmt (s1);
-
-  gsi_insert_before (&bsi, new_stmt, GSI_SAME_STMT);
 
   var = create_tmp_reg (type, "predreastmp");
   tmp_name = make_ssa_name (var);
@@ -2603,6 +2596,7 @@ reassociate_to_the_same_stmt (tree name1, tree name2, gimple *insert_before)
   s1 = gsi_stmt (bsi);
   update_stmt (s1);
 
+  gsi_insert_before (&bsi, new_stmt, GSI_SAME_STMT);
   gsi_insert_before (&bsi, tmp_stmt, GSI_SAME_STMT);
 
   return new_stmt;
@@ -2611,11 +2605,10 @@ reassociate_to_the_same_stmt (tree name1, tree name2, gimple *insert_before)
 /* Returns the statement that combines references R1 and R2.  In case R1
    and R2 are not used in the same statement, but they are used with an
    associative and commutative operation in the same expression, reassociate
-   the expression so that they are used in the same statement.  The combined
-   statement is inserted before INSERT_BEFORE if it's not NULL.  */
+   the expression so that they are used in the same statement.  */
 
 static gimple *
-stmt_combining_refs (dref r1, dref r2, gimple *insert_before)
+stmt_combining_refs (dref r1, dref r2)
 {
   gimple *stmt1, *stmt2;
   tree name1 = name_for_ref (r1);
@@ -2626,7 +2619,7 @@ stmt_combining_refs (dref r1, dref r2, gimple *insert_before)
   if (stmt1 == stmt2)
     return stmt1;
 
-  return reassociate_to_the_same_stmt (name1, name2, insert_before);
+  return reassociate_to_the_same_stmt (name1, name2);
 }
 
 /* Tries to combine chains CH1 and CH2 together.  If this succeeds, the
@@ -2639,7 +2632,7 @@ combine_chains (chain_p ch1, chain_p ch2)
   enum tree_code op = ERROR_MARK;
   bool swap = false;
   chain_p new_chain;
-  int i, j, num;
+  unsigned i;
   gimple *root_stmt;
   tree rslt_type = NULL_TREE;
 
@@ -2661,9 +2654,6 @@ combine_chains (chain_p ch1, chain_p ch2)
 	return NULL;
     }
 
-  ch1->combined = true;
-  ch2->combined = true;
-
   if (swap)
     std::swap (ch1, ch2);
 
@@ -2675,45 +2665,15 @@ combine_chains (chain_p ch1, chain_p ch2)
   new_chain->rslt_type = rslt_type;
   new_chain->length = ch1->length;
 
-  gimple *insert = NULL;
-  num = ch1->refs.length ();
-  i = (new_chain->length == 0) ? num - 1 : 0;
-  j = (new_chain->length == 0) ? -1 : 1;
-  /* For ZERO length chain, process refs in reverse order so that dominant
-     position is ready when it comes to the root ref.
-     For non-ZERO length chain, process refs in order.  See PR79663.  */
-  for (; num > 0; num--, i += j)
-    {
-      r1 = ch1->refs[i];
-      r2 = ch2->refs[i];
+  for (i = 0; (ch1->refs.iterate (i, &r1)
+	       && ch2->refs.iterate (i, &r2)); i++)
+    {
       nw = XCNEW (struct dref_d);
+      nw->stmt = stmt_combining_refs (r1, r2);
       nw->distance = r1->distance;
 
-      /* For ZERO length chain, insert combined stmt of root ref at dominant
-	 position.  */
-      nw->stmt = stmt_combining_refs (r1, r2, i == 0 ? insert : NULL);
-      /* For ZERO length chain, record dominant position where combined stmt
-	 of root ref should be inserted.  In this case, though all root refs
-	 dominate following ones, it's possible that combined stmt doesn't.
-	 See PR70754.  */
-      if (new_chain->length == 0
-	  && (insert == NULL || stmt_dominates_stmt_p (nw->stmt, insert)))
-	insert = nw->stmt;
-
       new_chain->refs.safe_push (nw);
     }
-  if (new_chain->length == 0)
-    {
-      /* Restore the order for ZERO length chain's refs.  */
-      num = new_chain->refs.length () >> 1;
-      for (i = 0, j = new_chain->refs.length () - 1; i < num; i++, j--)
-	std::swap (new_chain->refs[i], new_chain->refs[j]);
-
-      /* For ZERO length chain, has_max_use_after must be true since root
-	 combined stmt must dominates others.  */
-      new_chain->has_max_use_after = true;
-      return new_chain;
-    }
 
   new_chain->has_max_use_after = false;
   root_stmt = get_chain_root (new_chain)->stmt;
@@ -2727,6 +2687,8 @@ combine_chains (chain_p ch1, chain_p ch2)
 	}
     }
 
+  ch1->combined = true;
+  ch2->combined = true;
   return new_chain;
 }
 
-- 
1.9.1