[PATCH 0/4] v4 PHI-OPT move abs_replacement to match.pd

[PATCH 0/4] v4 PHI-OPT move abs_replacement to match.pd

[apinski]

From: Andrew Pinski <apinski@marvell.com>

To able to move PHI-OPT's abs_replacement to match.pd, a bunch
of support needed to be added to PHI-OPT.
This is a set of 4 (unapproved) patches which allows us to remove
abs_replacement and even does one set further and does a few extra
transformations that abs_replacement did not do (just because of
the moving from fold to match).

v3 to v4 Changes:
* pushed the approved patches which are not dependent on unapproved ones
* Change 1st patch to only duplicate in one direction and expanded comment
  on why it is safe and what it is needed

Andrew Pinski (4):
  Duplicate the range information of the phi onto the new ssa_name
  Allow match-and-simplified phiopt to run in early phiopt
  Try inverted comparison for match_simplify in phiopt
  Port most of the A CMP 0 ? A : -A to match

 gcc/match.pd                               |  60 +++++
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c |   4 +-
 gcc/tree-ssa-phiopt.c                      | 291 +++++++++------------
 3 files changed, 185 insertions(+), 170 deletions(-)

-- 
2.27.0

[PATCH 1/4] Duplicate the range information of the phi onto the new ssa_name

[apinski]

From: Andrew Pinski <apinski@marvell.com>

Since match_simplify_replacement uses gimple_simplify, there is a new
ssa name created sometimes and then we go and replace the phi edge with
this new ssa name, the range information on the phi is lost.
Placing this in replace_phi_edge_with_variable is the best option instead
of doing it in each time replace_phi_edge_with_variable is called which is
what is done today.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* tree-ssa-phiopt.c (replace_phi_edge_with_variable): Duplicate range
	info if we're the only things setting the target PHI.
	(value_replacement): Don't duplicate range here.
	(minmax_replacement): Likewise.
---
 gcc/tree-ssa-phiopt.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 1777bff2f7c..ab12e85569d 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -391,6 +391,32 @@ replace_phi_edge_with_variable (basic_block cond_block,
   basic_block bb = gimple_bb (phi);
   basic_block block_to_remove;
   gimple_stmt_iterator gsi;
+  tree phi_result = PHI_RESULT (phi);
+
+  /* Duplicate range info if we're the only things setting the target PHI.
+     This is needed as later on, the new_tree will be replacing
+     The assignement of the PHI.
+     For an example:
+     bb1:
+     _4 = min<a_1, 255>
+     goto bb2
+
+     range<-INF,255>
+     a_3 = PHI<_4(1)>
+     bb3:
+
+     use(a_3)
+     And _4 gets prograted into the use of a_3 and losing the range info.
+     This can't be done for more than 2 incoming edges as the progration
+     won't happen.  */
+  if (TREE_CODE (new_tree) == SSA_NAME
+      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
+      && INTEGRAL_TYPE_P (TREE_TYPE (phi_result))
+      && !SSA_NAME_RANGE_INFO (new_tree)
+      && SSA_NAME_RANGE_INFO (phi_result))
+    duplicate_ssa_name_range_info (new_tree,
+				   SSA_NAME_RANGE_TYPE (phi_result),
+				   SSA_NAME_RANGE_INFO (phi_result));
 
   /* Change the PHI argument to new.  */
   SET_USE (PHI_ARG_DEF_PTR (phi, e->dest_idx), new_tree);
@@ -1385,16 +1411,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
 	   <bb 4>:
 	   # u_3 = PHI <u_6(3), 4294967295(2)>  */
       reset_flow_sensitive_info (lhs);
-      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
-	{
-	  /* If available, we can use VR of phi result at least.  */
-	  tree phires = gimple_phi_result (phi);
-	  struct range_info_def *phires_range_info
-	    = SSA_NAME_RANGE_INFO (phires);
-	  if (phires_range_info)
-	    duplicate_ssa_name_range_info (lhs, SSA_NAME_RANGE_TYPE (phires),
-					   phires_range_info);
-	}
       gimple_stmt_iterator gsi_from;
       for (int i = prep_cnt - 1; i >= 0; --i)
 	{
@@ -1794,13 +1810,6 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb,
   gimple_seq stmts = NULL;
   tree phi_result = PHI_RESULT (phi);
   result = gimple_build (&stmts, minmax, TREE_TYPE (phi_result), arg0, arg1);
-  /* Duplicate range info if we're the only things setting the target PHI.  */
-  if (!gimple_seq_empty_p (stmts)
-      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
-      && !POINTER_TYPE_P (TREE_TYPE (phi_result))
-      && SSA_NAME_RANGE_INFO (phi_result))
-    duplicate_ssa_name_range_info (result, SSA_NAME_RANGE_TYPE (phi_result),
-				   SSA_NAME_RANGE_INFO (phi_result));
 
   gsi = gsi_last_bb (cond_bb);
   gsi_insert_seq_before (&gsi, stmts, GSI_NEW_STMT);
-- 
2.27.0

[PATCH 2/4] Allow match-and-simplified phiopt to run in early phiopt

[apinski]

From: Andrew Pinski <apinski@marvell.com>

To move a few things more to match-and-simplify from phiopt,
we need to allow match_simplify_replacement to run in early
phiopt. To do this we add a replacement for gimple_simplify
that is explictly for phiopt.

OK? Bootstrapped and tested on x86_64-linux-gnu with no
regressions.

gcc/ChangeLog:

	* tree-ssa-phiopt.c (match_simplify_replacement):
	Add early_p argument. Call gimple_simplify_phiopt
	instead of gimple_simplify.
	(tree_ssa_phiopt_worker): Update call to
	match_simplify_replacement and allow unconditionally.
	(phiopt_early_allow): New function.
	(gimple_simplify_phiopt): New function.
---
 gcc/tree-ssa-phiopt.c | 89 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 19 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index ab12e85569d..17bc597851b 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -50,12 +50,13 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple-fold.h"
 #include "internal-fn.h"
 #include "gimple-range.h"
+#include "gimple-match.h"
 
 static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
 static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
 				   tree, tree);
 static bool match_simplify_replacement (basic_block, basic_block,
-					edge, edge, gphi *, tree, tree);
+					edge, edge, gphi *, tree, tree, bool);
 static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree,
 						gimple *);
 static int value_replacement (basic_block, basic_block,
@@ -345,9 +346,9 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
 	  /* Do the replacement of conditional if it can be done.  */
 	  if (!early_p && two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
 	    cfgchanged = true;
-	  else if (!early_p
-		   && match_simplify_replacement (bb, bb1, e1, e2, phi,
-						  arg0, arg1))
+	  else if (match_simplify_replacement (bb, bb1, e1, e2, phi,
+					       arg0, arg1,
+					       early_p))
 	    cfgchanged = true;
 	  else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
 	    cfgchanged = true;
@@ -811,6 +812,67 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
   return true;
 }
 
+/* Return TRUE if CODE should be allowed during early phiopt.
+   Currently this is to allow MIN/MAX and ABS/NEGATE.  */
+static bool
+phiopt_early_allow (enum tree_code code)
+{
+  switch (code)
+    {
+      case MIN_EXPR:
+      case MAX_EXPR:
+      case ABS_EXPR:
+      case ABSU_EXPR:
+      case NEGATE_EXPR:
+      case SSA_NAME:
+	return true;
+      default:
+	return false;
+    }
+}
+
+/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
+   Return NULL if nothing can be simplified or the resulting simplified value
+   with parts pushed if EARLY_P was true. Also rejects non allowed tree code
+   if EARLY_P is set.
+   Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries
+   to simplify CMP ? ARG0 : ARG1.  */
+static tree
+gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
+			tree arg0, tree arg1,
+			gimple_seq *seq)
+{
+  tree result;
+  enum tree_code comp_code = gimple_cond_code (comp_stmt);
+  location_t loc = gimple_location (comp_stmt);
+  tree cmp0 = gimple_cond_lhs (comp_stmt);
+  tree cmp1 = gimple_cond_rhs (comp_stmt);
+  /* To handle special cases like floating point comparison, it is easier and
+     less error-prone to build a tree and gimplify it on the fly though it is
+     less efficient.
+     Don't use fold_build2 here as that might create (bool)a instead of just
+     "a != 0".  */
+  tree cond = build2_loc (loc, comp_code, boolean_type_node,
+			  cmp0, cmp1);
+  gimple_match_op op (gimple_match_cond::UNCOND,
+		      COND_EXPR, type, cond, arg0, arg1);
+
+  if (op.resimplify (early_p ? NULL : seq, follow_all_ssa_edges))
+    {
+      /* Early we want only to allow some generated tree codes. */
+      if (!early_p
+	  || op.code.is_tree_code ()
+	  || phiopt_early_allow ((tree_code)op.code))
+	{
+	  result = maybe_push_res_to_seq (&op, seq);
+	  if (result)
+	    return result;
+	}
+    }
+
+  return NULL;
+}
+
 /*  The function match_simplify_replacement does the main work of doing the
     replacement using match and simplify.  Return true if the replacement is done.
     Otherwise return false.
@@ -820,10 +882,9 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
 static bool
 match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
 			    edge e0, edge e1, gphi *phi,
-			    tree arg0, tree arg1)
+			    tree arg0, tree arg1, bool early_p)
 {
   gimple *stmt;
-  tree cond;
   gimple_stmt_iterator gsi;
   edge true_edge, false_edge;
   gimple_seq seq = NULL;
@@ -884,15 +945,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
 
   stmt = last_stmt (cond_bb);
 
-  /* To handle special cases like floating point comparison, it is easier and
-     less error-prone to build a tree and gimplify it on the fly though it is
-     less efficient.
-     Don't use fold_build2 here as that might create (bool)a instead of just
-     "a != 0".  */
-  cond = build2_loc (gimple_location (stmt),
-		     gimple_cond_code (stmt), boolean_type_node,
-		     gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
-
   /* We need to know which is the true edge and which is the false
      edge so that we know when to invert the condition below.  */
   extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
@@ -900,10 +952,9 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
     std::swap (arg0, arg1);
 
   tree type = TREE_TYPE (gimple_phi_result (phi));
-  result = gimple_simplify (COND_EXPR, type,
-			    cond,
-			    arg0, arg1,
-			    &seq, NULL);
+  result = gimple_simplify_phiopt (early_p, type, stmt,
+				   arg0, arg1,
+				   &seq);
   if (!result)
     return false;
 
-- 
2.27.0

[PATCH 3/4] Try inverted comparison for match_simplify in phiopt

[apinski]

From: Andrew Pinski <apinski@marvell.com>

Since match and simplify does not have all of the inverted
comparison patterns, it make sense to just have
phi-opt try to do the inversion and try match and simplify again.

OK? Bootstrapped and tested on x86_64-linux-gnu.

Thanks,
Andrew Pinski

gcc/ChangeLog:

	* tree-ssa-phiopt.c (gimple_simplify_phiopt):
	If "A ? B : C" fails to simplify, try "(!A) ? C : B".
---
 gcc/tree-ssa-phiopt.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 17bc597851b..9bda1b2a397 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -836,7 +836,8 @@ phiopt_early_allow (enum tree_code code)
    with parts pushed if EARLY_P was true. Also rejects non allowed tree code
    if EARLY_P is set.
    Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries
-   to simplify CMP ? ARG0 : ARG1.  */
+   to simplify CMP ? ARG0 : ARG1.
+   Also try to simplify (!CMP) ? ARG0 : ARG1 if the non-inverse failed.  */
 static tree
 gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
 			tree arg0, tree arg1,
@@ -869,6 +870,30 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
 	    return result;
 	}
     }
+  /* Try the inverted comparison, that is !COMP ? ARG1 : ARG0. */
+  comp_code = invert_tree_comparison (comp_code, HONOR_NANS (cmp0));
+
+  if (comp_code == ERROR_MARK)
+    return NULL;
+
+  cond = build2_loc (loc,
+		     comp_code, boolean_type_node,
+		     cmp0, cmp1);
+  gimple_match_op op1 (gimple_match_cond::UNCOND,
+		       COND_EXPR, type, cond, arg1, arg0);
+
+  if (op1.resimplify (early_p ? NULL : seq, follow_all_ssa_edges))
+    {
+      /* Early we want only to allow some generated tree codes. */
+      if (!early_p
+	  || op1.code.is_tree_code ()
+	  || phiopt_early_allow ((tree_code)op1.code))
+	{
+	  result = maybe_push_res_to_seq (&op1, seq);
+	  if (result)
+	    return result;
+	}
+    }
 
   return NULL;
 }
-- 
2.27.0

[PATCH 4/4] Port most of the A CMP 0 ? A : -A to match

[apinski]

From: Andrew Pinski <apinski@marvell.com>

To improve phiopt and be able to remove abs_replacement, this ports
most of "A CMP 0 ? A : -A" from fold_cond_expr_with_comparison to
match.pd.  There is a few extra changes that are needed to remove
the "A CMP 0 ? A : -A" part from fold_cond_expr_with_comparison:
   * Need to handle (A - B) case
   * Need to handle UN* comparisons.

I will handle those in a different patch.

Note phi-opt-15.c test needed to be updated as we get ABSU now
instead of not getting ABS.  When ABSU was added phiopt was not
updated even to use ABSU instead of not creating ABS.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* match.pd (A CMP 0 ? A : -A): New patterns.
	* tree-ssa-phiopt.c (abs_replacement): Delete function.
	(tree_ssa_phiopt_worker): Don't call abs_replacement.
	Update comment about abs_replacement.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/phi-opt-15.c: Update test to expect
	ABSU and still not expect ABS_EXPR.
---
 gcc/match.pd                               |  60 +++++++++
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c |   4 +-
 gcc/tree-ssa-phiopt.c                      | 134 +--------------------
 3 files changed, 64 insertions(+), 134 deletions(-)

diff --git a/gcc/match.pd b/gcc/match.pd
index 39fb57ee1f4..0c790dfa741 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3976,6 +3976,66 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
   (cnd @0 @2 @1)))
 
+/* abs/negative simplifications moved from fold_cond_expr_with_comparison,
+   Need to handle (A - B) case as fold_cond_expr_with_comparison does.
+   Need to handle UN* comparisons.
+
+   None of these transformations work for modes with signed
+   zeros.  If A is +/-0, the first two transformations will
+   change the sign of the result (from +0 to -0, or vice
+   versa).  The last four will fix the sign of the result,
+   even though the original expressions could be positive or
+   negative, depending on the sign of A.
+
+   Note that all these transformations are correct if A is
+   NaN, since the two alternatives (A and -A) are also NaNs.  */
+
+(for cnd (cond vec_cond)
+ /* A == 0? A : -A    same as -A */
+ (for cmp (eq uneq)
+  (simplify
+   (cnd (cmp @0 zerop) @0 (negate@1 @0))
+    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
+     @1))
+  (simplify
+   (cnd (cmp @0 zerop) zerop (negate@1 @0))
+    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
+     @1))
+ )
+ /* A != 0? A : -A    same as A */
+ (for cmp (ne ltgt)
+  (simplify
+   (cnd (cmp @0 zerop) @0 (negate @0))
+    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
+     @0))
+  (simplify
+   (cnd (cmp @0 zerop) @0 zerop)
+    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
+     @0))
+ )
+ /* A >=/> 0? A : -A    same as abs (A) */
+ (for cmp (ge gt)
+  (simplify
+   (cnd (cmp @0 zerop) @0 (negate @0))
+    (if (!HONOR_SIGNED_ZEROS (element_mode (type))
+	 && !TYPE_UNSIGNED (type))
+     (abs @0))))
+ /* A <=/< 0? A : -A    same as -abs (A) */
+ (for cmp (le lt)
+  (simplify
+   (cnd (cmp @0 zerop) @0 (negate @0))
+    (if (!HONOR_SIGNED_ZEROS (element_mode (type))
+	 && !TYPE_UNSIGNED (type))
+     (if (ANY_INTEGRAL_TYPE_P (type)
+	  && !TYPE_OVERFLOW_WRAPS (type))
+      (with {
+	tree utype = unsigned_type_for (type);
+       }
+       (convert (negate (absu:utype @0))))
+       (negate (abs @0)))))
+ )
+)
+
 /* -(type)!A -> (type)A - 1.  */
 (simplify
  (negate (convert?:s (logical_inverted_value:s @0)))
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c
index ac3018ef533..6aec68961cf 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c
@@ -9,4 +9,6 @@ foo (int i)
   return i;
 }
 
-/* { dg-final { scan-tree-dump-not "ABS" "optimized" } } */
+/* We should not have ABS_EXPR but ABSU_EXPR instead. */
+/* { dg-final { scan-tree-dump-not "ABS_EXPR" "optimized" } } */
+/* { dg-final { scan-tree-dump "ABSU" "optimized" } } */
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 9bda1b2a397..97540e30d55 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -63,8 +63,6 @@ static int value_replacement (basic_block, basic_block,
 			      edge, edge, gphi *, tree, tree);
 static bool minmax_replacement (basic_block, basic_block,
 				edge, edge, gphi *, tree, tree);
-static bool abs_replacement (basic_block, basic_block,
-			     edge, edge, gphi *, tree, tree);
 static bool spaceship_replacement (basic_block, basic_block,
 				   edge, edge, gphi *, tree, tree);
 static bool cond_removal_in_popcount_clz_ctz_pattern (basic_block, basic_block,
@@ -350,8 +348,6 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
 					       arg0, arg1,
 					       early_p))
 	    cfgchanged = true;
-	  else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
-	    cfgchanged = true;
 	  else if (!early_p
 		   && cond_removal_in_popcount_clz_ctz_pattern (bb, bb1, e1,
 								e2, phi, arg0,
@@ -2606,134 +2602,6 @@ cond_removal_in_popcount_clz_ctz_pattern (basic_block cond_bb,
   return true;
 }
 
-/*  The function absolute_replacement does the main work of doing the absolute
-    replacement.  Return true if the replacement is done.  Otherwise return
-    false.
-    bb is the basic block where the replacement is going to be done on.  arg0
-    is argument 0 from the phi.  Likewise for arg1.  */
-
-static bool
-abs_replacement (basic_block cond_bb, basic_block middle_bb,
-		 edge e0 ATTRIBUTE_UNUSED, edge e1,
-		 gphi *phi, tree arg0, tree arg1)
-{
-  tree result;
-  gassign *new_stmt;
-  gimple *cond;
-  gimple_stmt_iterator gsi;
-  edge true_edge, false_edge;
-  gimple *assign;
-  edge e;
-  tree rhs, lhs;
-  bool negate;
-  enum tree_code cond_code;
-
-  /* If the type says honor signed zeros we cannot do this
-     optimization.  */
-  if (HONOR_SIGNED_ZEROS (arg1))
-    return false;
-
-  /* OTHER_BLOCK must have only one executable statement which must have the
-     form arg0 = -arg1 or arg1 = -arg0.  */
-
-  assign = last_and_only_stmt (middle_bb);
-  /* If we did not find the proper negation assignment, then we cannot
-     optimize.  */
-  if (assign == NULL)
-    return false;
-
-  /* If we got here, then we have found the only executable statement
-     in OTHER_BLOCK.  If it is anything other than arg = -arg1 or
-     arg1 = -arg0, then we cannot optimize.  */
-  if (gimple_code (assign) != GIMPLE_ASSIGN)
-    return false;
-
-  lhs = gimple_assign_lhs (assign);
-
-  if (gimple_assign_rhs_code (assign) != NEGATE_EXPR)
-    return false;
-
-  rhs = gimple_assign_rhs1 (assign);
-
-  /* The assignment has to be arg0 = -arg1 or arg1 = -arg0.  */
-  if (!(lhs == arg0 && rhs == arg1)
-      && !(lhs == arg1 && rhs == arg0))
-    return false;
-
-  cond = last_stmt (cond_bb);
-  result = PHI_RESULT (phi);
-
-  /* Only relationals comparing arg[01] against zero are interesting.  */
-  cond_code = gimple_cond_code (cond);
-  if (cond_code != GT_EXPR && cond_code != GE_EXPR
-      && cond_code != LT_EXPR && cond_code != LE_EXPR)
-    return false;
-
-  /* Make sure the conditional is arg[01] OP y.  */
-  if (gimple_cond_lhs (cond) != rhs)
-    return false;
-
-  if (FLOAT_TYPE_P (TREE_TYPE (gimple_cond_rhs (cond)))
-	       ? real_zerop (gimple_cond_rhs (cond))
-	       : integer_zerop (gimple_cond_rhs (cond)))
-    ;
-  else
-    return false;
-
-  /* We need to know which is the true edge and which is the false
-     edge so that we know if have abs or negative abs.  */
-  extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
-
-  /* For GT_EXPR/GE_EXPR, if the true edge goes to OTHER_BLOCK, then we
-     will need to negate the result.  Similarly for LT_EXPR/LE_EXPR if
-     the false edge goes to OTHER_BLOCK.  */
-  if (cond_code == GT_EXPR || cond_code == GE_EXPR)
-    e = true_edge;
-  else
-    e = false_edge;
-
-  if (e->dest == middle_bb)
-    negate = true;
-  else
-    negate = false;
-
-  /* If the code negates only iff positive then make sure to not
-     introduce undefined behavior when negating or computing the absolute.
-     ???  We could use range info if present to check for arg1 == INT_MIN.  */
-  if (negate
-      && (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg1))
-	  && ! TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1))))
-    return false;
-
-  result = duplicate_ssa_name (result, NULL);
-
-  if (negate)
-    lhs = make_ssa_name (TREE_TYPE (result));
-  else
-    lhs = result;
-
-  /* Build the modify expression with abs expression.  */
-  new_stmt = gimple_build_assign (lhs, ABS_EXPR, rhs);
-
-  gsi = gsi_last_bb (cond_bb);
-  gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT);
-
-  if (negate)
-    {
-      /* Get the right GSI.  We want to insert after the recently
-	 added ABS_EXPR statement (which we know is the first statement
-	 in the block.  */
-      new_stmt = gimple_build_assign (result, NEGATE_EXPR, lhs);
-
-      gsi_insert_after (&gsi, new_stmt, GSI_NEW_STMT);
-    }
-
-  replace_phi_edge_with_variable (cond_bb, e1, phi, result);
-
-  /* Note that we optimized this PHI.  */
-  return true;
-}
-
 /* Auxiliary functions to determine the set of memory accesses which
    can't trap because they are preceded by accesses to the same memory
    portion.  We do that for MEM_REFs, so we only need to track
@@ -3670,7 +3538,7 @@ gate_hoist_loads (void)
    ABS Replacement
    ---------------
 
-   This transformation, implemented in abs_replacement, replaces
+   This transformation, implemented in match_simplify_replacement, replaces
 
      bb0:
        if (a >= 0) goto bb2; else goto bb1;
-- 
2.27.0

Re: [PATCH 3/4] Try inverted comparison for match_simplify in phiopt

[Bernhard Reutner-Fischer]

On 28 June 2021 01:24:59 CEST, apinski--- via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>From: Andrew Pinski <apinski@marvell.com>
>
>Since match and simplify does not have all of the inverted
>comparison patterns, it make sense to just have
>phi-opt try to do the inversion and try match and simplify again.
>
>OK? Bootstrapped and tested on x86_64-linux-gnu.
>
>Thanks,
>Andrew Pinski
>
>gcc/ChangeLog:
>
>	* tree-ssa-phiopt.c (gimple_simplify_phiopt):
>	If "A ? B : C" fails to simplify, try "(!A) ? C : B".
>---
> gcc/tree-ssa-phiopt.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
>diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
>index 17bc597851b..9bda1b2a397 100644
>--- a/gcc/tree-ssa-phiopt.c
>+++ b/gcc/tree-ssa-phiopt.c
>@@ -836,7 +836,8 @@ phiopt_early_allow (enum tree_code code)
>with parts pushed if EARLY_P was true. Also rejects non allowed tree
>code
>    if EARLY_P is set.
>Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and
>tries
>-   to simplify CMP ? ARG0 : ARG1.  */
>+   to simplify CMP ? ARG0 : ARG1.
>+   Also try to simplify (!CMP) ? ARG0 : ARG1 if the non-inverse
>failed.  */

I think you need to swap the args above?

thanks,

Re: [PATCH 1/4] Duplicate the range information of the phi onto the new ssa_name

[Richard Biener]

On Mon, Jun 28, 2021 at 1:26 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> Since match_simplify_replacement uses gimple_simplify, there is a new
> ssa name created sometimes and then we go and replace the phi edge with
> this new ssa name, the range information on the phi is lost.
> Placing this in replace_phi_edge_with_variable is the best option instead
> of doing it in each time replace_phi_edge_with_variable is called which is
> what is done today.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.c (replace_phi_edge_with_variable): Duplicate range
>         info if we're the only things setting the target PHI.
>         (value_replacement): Don't duplicate range here.
>         (minmax_replacement): Likewise.
> ---
>  gcc/tree-ssa-phiopt.c | 43 ++++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 1777bff2f7c..ab12e85569d 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -391,6 +391,32 @@ replace_phi_edge_with_variable (basic_block cond_block,
>    basic_block bb = gimple_bb (phi);
>    basic_block block_to_remove;
>    gimple_stmt_iterator gsi;
> +  tree phi_result = PHI_RESULT (phi);
> +
> +  /* Duplicate range info if we're the only things setting the target PHI.
> +     This is needed as later on, the new_tree will be replacing
> +     The assignement of the PHI.
> +     For an example:
> +     bb1:
> +     _4 = min<a_1, 255>
> +     goto bb2
> +
> +     range<-INF,255>
> +     a_3 = PHI<_4(1)>
> +     bb3:
> +
> +     use(a_3)
> +     And _4 gets prograted into the use of a_3 and losing the range info.
> +     This can't be done for more than 2 incoming edges as the progration
> +     won't happen.  */
> +  if (TREE_CODE (new_tree) == SSA_NAME
> +      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
> +      && INTEGRAL_TYPE_P (TREE_TYPE (phi_result))
> +      && !SSA_NAME_RANGE_INFO (new_tree)
> +      && SSA_NAME_RANGE_INFO (phi_result))
> +    duplicate_ssa_name_range_info (new_tree,
> +                                  SSA_NAME_RANGE_TYPE (phi_result),
> +                                  SSA_NAME_RANGE_INFO (phi_result));
>
>    /* Change the PHI argument to new.  */
>    SET_USE (PHI_ARG_DEF_PTR (phi, e->dest_idx), new_tree);
> @@ -1385,16 +1411,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
>            <bb 4>:
>            # u_3 = PHI <u_6(3), 4294967295(2)>  */
>        reset_flow_sensitive_info (lhs);
> -      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
> -       {
> -         /* If available, we can use VR of phi result at least.  */
> -         tree phires = gimple_phi_result (phi);
> -         struct range_info_def *phires_range_info
> -           = SSA_NAME_RANGE_INFO (phires);
> -         if (phires_range_info)
> -           duplicate_ssa_name_range_info (lhs, SSA_NAME_RANGE_TYPE (phires),
> -                                          phires_range_info);
> -       }
>        gimple_stmt_iterator gsi_from;
>        for (int i = prep_cnt - 1; i >= 0; --i)
>         {
> @@ -1794,13 +1810,6 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb,
>    gimple_seq stmts = NULL;
>    tree phi_result = PHI_RESULT (phi);
>    result = gimple_build (&stmts, minmax, TREE_TYPE (phi_result), arg0, arg1);
> -  /* Duplicate range info if we're the only things setting the target PHI.  */
> -  if (!gimple_seq_empty_p (stmts)
> -      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
> -      && !POINTER_TYPE_P (TREE_TYPE (phi_result))
> -      && SSA_NAME_RANGE_INFO (phi_result))
> -    duplicate_ssa_name_range_info (result, SSA_NAME_RANGE_TYPE (phi_result),
> -                                  SSA_NAME_RANGE_INFO (phi_result));
>
>    gsi = gsi_last_bb (cond_bb);
>    gsi_insert_seq_before (&gsi, stmts, GSI_NEW_STMT);
> --
> 2.27.0
>

Re: [PATCH 2/4] Allow match-and-simplified phiopt to run in early phiopt

[Richard Biener]

On Mon, Jun 28, 2021 at 1:27 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> To move a few things more to match-and-simplify from phiopt,
> we need to allow match_simplify_replacement to run in early
> phiopt. To do this we add a replacement for gimple_simplify
> that is explictly for phiopt.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no
> regressions.

OK.

> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.c (match_simplify_replacement):
>         Add early_p argument. Call gimple_simplify_phiopt
>         instead of gimple_simplify.
>         (tree_ssa_phiopt_worker): Update call to
>         match_simplify_replacement and allow unconditionally.
>         (phiopt_early_allow): New function.
>         (gimple_simplify_phiopt): New function.
> ---
>  gcc/tree-ssa-phiopt.c | 89 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 70 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index ab12e85569d..17bc597851b 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -50,12 +50,13 @@ along with GCC; see the file COPYING3.  If not see
>  #include "gimple-fold.h"
>  #include "internal-fn.h"
>  #include "gimple-range.h"
> +#include "gimple-match.h"
>
>  static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
>  static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
>                                    tree, tree);
>  static bool match_simplify_replacement (basic_block, basic_block,
> -                                       edge, edge, gphi *, tree, tree);
> +                                       edge, edge, gphi *, tree, tree, bool);
>  static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree,
>                                                 gimple *);
>  static int value_replacement (basic_block, basic_block,
> @@ -345,9 +346,9 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
>           /* Do the replacement of conditional if it can be done.  */
>           if (!early_p && two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
>             cfgchanged = true;
> -         else if (!early_p
> -                  && match_simplify_replacement (bb, bb1, e1, e2, phi,
> -                                                 arg0, arg1))
> +         else if (match_simplify_replacement (bb, bb1, e1, e2, phi,
> +                                              arg0, arg1,
> +                                              early_p))
>             cfgchanged = true;
>           else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
>             cfgchanged = true;
> @@ -811,6 +812,67 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
>    return true;
>  }
>
> +/* Return TRUE if CODE should be allowed during early phiopt.
> +   Currently this is to allow MIN/MAX and ABS/NEGATE.  */
> +static bool
> +phiopt_early_allow (enum tree_code code)
> +{
> +  switch (code)
> +    {
> +      case MIN_EXPR:
> +      case MAX_EXPR:
> +      case ABS_EXPR:
> +      case ABSU_EXPR:
> +      case NEGATE_EXPR:
> +      case SSA_NAME:
> +       return true;
> +      default:
> +       return false;
> +    }
> +}
> +
> +/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
> +   Return NULL if nothing can be simplified or the resulting simplified value
> +   with parts pushed if EARLY_P was true. Also rejects non allowed tree code
> +   if EARLY_P is set.
> +   Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries
> +   to simplify CMP ? ARG0 : ARG1.  */
> +static tree
> +gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> +                       tree arg0, tree arg1,
> +                       gimple_seq *seq)
> +{
> +  tree result;
> +  enum tree_code comp_code = gimple_cond_code (comp_stmt);
> +  location_t loc = gimple_location (comp_stmt);
> +  tree cmp0 = gimple_cond_lhs (comp_stmt);
> +  tree cmp1 = gimple_cond_rhs (comp_stmt);
> +  /* To handle special cases like floating point comparison, it is easier and
> +     less error-prone to build a tree and gimplify it on the fly though it is
> +     less efficient.
> +     Don't use fold_build2 here as that might create (bool)a instead of just
> +     "a != 0".  */
> +  tree cond = build2_loc (loc, comp_code, boolean_type_node,
> +                         cmp0, cmp1);
> +  gimple_match_op op (gimple_match_cond::UNCOND,
> +                     COND_EXPR, type, cond, arg0, arg1);
> +
> +  if (op.resimplify (early_p ? NULL : seq, follow_all_ssa_edges))
> +    {
> +      /* Early we want only to allow some generated tree codes. */
> +      if (!early_p
> +         || op.code.is_tree_code ()
> +         || phiopt_early_allow ((tree_code)op.code))
> +       {
> +         result = maybe_push_res_to_seq (&op, seq);
> +         if (result)
> +           return result;
> +       }
> +    }
> +
> +  return NULL;
> +}
> +
>  /*  The function match_simplify_replacement does the main work of doing the
>      replacement using match and simplify.  Return true if the replacement is done.
>      Otherwise return false.
> @@ -820,10 +882,9 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
>  static bool
>  match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>                             edge e0, edge e1, gphi *phi,
> -                           tree arg0, tree arg1)
> +                           tree arg0, tree arg1, bool early_p)
>  {
>    gimple *stmt;
> -  tree cond;
>    gimple_stmt_iterator gsi;
>    edge true_edge, false_edge;
>    gimple_seq seq = NULL;
> @@ -884,15 +945,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>
>    stmt = last_stmt (cond_bb);
>
> -  /* To handle special cases like floating point comparison, it is easier and
> -     less error-prone to build a tree and gimplify it on the fly though it is
> -     less efficient.
> -     Don't use fold_build2 here as that might create (bool)a instead of just
> -     "a != 0".  */
> -  cond = build2_loc (gimple_location (stmt),
> -                    gimple_cond_code (stmt), boolean_type_node,
> -                    gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
> -
>    /* We need to know which is the true edge and which is the false
>       edge so that we know when to invert the condition below.  */
>    extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
> @@ -900,10 +952,9 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>      std::swap (arg0, arg1);
>
>    tree type = TREE_TYPE (gimple_phi_result (phi));
> -  result = gimple_simplify (COND_EXPR, type,
> -                           cond,
> -                           arg0, arg1,
> -                           &seq, NULL);
> +  result = gimple_simplify_phiopt (early_p, type, stmt,
> +                                  arg0, arg1,
> +                                  &seq);
>    if (!result)
>      return false;
>
> --
> 2.27.0
>

Re: [PATCH 3/4] Try inverted comparison for match_simplify in phiopt

[Richard Biener]

On Mon, Jun 28, 2021 at 1:28 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> Since match and simplify does not have all of the inverted
> comparison patterns, it make sense to just have
> phi-opt try to do the inversion and try match and simplify again.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu.

OK with the comment fix suggested by Bernhard.  I think
if the match fails you have to somewhere discard the
sequence, since it can still end up with partly pushed
stmts - otherwise you'll slowly leak SSA names.  Theres
gimple_seq_discard () for this.  It's probably best to do it
in gimple_simplify_phiopt after each failed try.

Richard.

> Thanks,
> Andrew Pinski
>
> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.c (gimple_simplify_phiopt):
>         If "A ? B : C" fails to simplify, try "(!A) ? C : B".
> ---
>  gcc/tree-ssa-phiopt.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 17bc597851b..9bda1b2a397 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -836,7 +836,8 @@ phiopt_early_allow (enum tree_code code)
>     with parts pushed if EARLY_P was true. Also rejects non allowed tree code
>     if EARLY_P is set.
>     Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries
> -   to simplify CMP ? ARG0 : ARG1.  */
> +   to simplify CMP ? ARG0 : ARG1.
> +   Also try to simplify (!CMP) ? ARG0 : ARG1 if the non-inverse failed.  */
>  static tree
>  gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
>                         tree arg0, tree arg1,
> @@ -869,6 +870,30 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
>             return result;
>         }
>      }
> +  /* Try the inverted comparison, that is !COMP ? ARG1 : ARG0. */
> +  comp_code = invert_tree_comparison (comp_code, HONOR_NANS (cmp0));
> +
> +  if (comp_code == ERROR_MARK)
> +    return NULL;
> +
> +  cond = build2_loc (loc,
> +                    comp_code, boolean_type_node,
> +                    cmp0, cmp1);
> +  gimple_match_op op1 (gimple_match_cond::UNCOND,
> +                      COND_EXPR, type, cond, arg1, arg0);
> +
> +  if (op1.resimplify (early_p ? NULL : seq, follow_all_ssa_edges))
> +    {
> +      /* Early we want only to allow some generated tree codes. */
> +      if (!early_p
> +         || op1.code.is_tree_code ()
> +         || phiopt_early_allow ((tree_code)op1.code))
> +       {
> +         result = maybe_push_res_to_seq (&op1, seq);
> +         if (result)
> +           return result;
> +       }
> +    }
>
>    return NULL;
>  }
> --
> 2.27.0
>

Re: [PATCH 4/4] Port most of the A CMP 0 ? A : -A to match

[Richard Biener]

On Mon, Jun 28, 2021 at 1:29 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> To improve phiopt and be able to remove abs_replacement, this ports
> most of "A CMP 0 ? A : -A" from fold_cond_expr_with_comparison to
> match.pd.  There is a few extra changes that are needed to remove
> the "A CMP 0 ? A : -A" part from fold_cond_expr_with_comparison:
>    * Need to handle (A - B) case
>    * Need to handle UN* comparisons.
>
> I will handle those in a different patch.
>
> Note phi-opt-15.c test needed to be updated as we get ABSU now
> instead of not getting ABS.  When ABSU was added phiopt was not
> updated even to use ABSU instead of not creating ABS.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
>         * match.pd (A CMP 0 ? A : -A): New patterns.
>         * tree-ssa-phiopt.c (abs_replacement): Delete function.
>         (tree_ssa_phiopt_worker): Don't call abs_replacement.
>         Update comment about abs_replacement.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/phi-opt-15.c: Update test to expect
>         ABSU and still not expect ABS_EXPR.
> ---
>  gcc/match.pd                               |  60 +++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c |   4 +-
>  gcc/tree-ssa-phiopt.c                      | 134 +--------------------
>  3 files changed, 64 insertions(+), 134 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 39fb57ee1f4..0c790dfa741 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3976,6 +3976,66 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
>    (cnd @0 @2 @1)))
>
> +/* abs/negative simplifications moved from fold_cond_expr_with_comparison,
> +   Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> +   Need to handle UN* comparisons.
> +
> +   None of these transformations work for modes with signed
> +   zeros.  If A is +/-0, the first two transformations will
> +   change the sign of the result (from +0 to -0, or vice
> +   versa).  The last four will fix the sign of the result,
> +   even though the original expressions could be positive or
> +   negative, depending on the sign of A.
> +
> +   Note that all these transformations are correct if A is
> +   NaN, since the two alternatives (A and -A) are also NaNs.  */
> +
> +(for cnd (cond vec_cond)
> + /* A == 0? A : -A    same as -A */
> + (for cmp (eq uneq)
> +  (simplify
> +   (cnd (cmp @0 zerop) @0 (negate@1 @0))
> +    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))

I think you can drop element_mode () calls, the HONOR_* stuff
should work on compound types as well.

> +     @1))
> +  (simplify
> +   (cnd (cmp @0 zerop) zerop (negate@1 @0))

So why do we need this special case?  zerop matches both
-0. and 0. but with constants and !HONOR_SIGNED_ZEROS
operand_equal_p as used by match should make that equal
to a mismatching sign zero in the (negate..) arm as well?  And
of course the negate should have been constant folded then.

Same for the other cases below, otherwise looks OK to me.

Thanks,
Richard.

> +    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> +     @1))
> + )
> + /* A != 0? A : -A    same as A */
> + (for cmp (ne ltgt)
> +  (simplify
> +   (cnd (cmp @0 zerop) @0 (negate @0))
> +    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> +     @0))
> +  (simplify
> +   (cnd (cmp @0 zerop) @0 zerop)
> +    (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> +     @0))
> + )
> + /* A >=/> 0? A : -A    same as abs (A) */
> + (for cmp (ge gt)
> +  (simplify
> +   (cnd (cmp @0 zerop) @0 (negate @0))
> +    (if (!HONOR_SIGNED_ZEROS (element_mode (type))
> +        && !TYPE_UNSIGNED (type))
> +     (abs @0))))
> + /* A <=/< 0? A : -A    same as -abs (A) */
> + (for cmp (le lt)
> +  (simplify
> +   (cnd (cmp @0 zerop) @0 (negate @0))
> +    (if (!HONOR_SIGNED_ZEROS (element_mode (type))
> +        && !TYPE_UNSIGNED (type))
> +     (if (ANY_INTEGRAL_TYPE_P (type)
> +         && !TYPE_OVERFLOW_WRAPS (type))
> +      (with {
> +       tree utype = unsigned_type_for (type);
> +       }
> +       (convert (negate (absu:utype @0))))
> +       (negate (abs @0)))))
> + )
> +)
> +
>  /* -(type)!A -> (type)A - 1.  */
>  (simplify
>   (negate (convert?:s (logical_inverted_value:s @0)))
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c
> index ac3018ef533..6aec68961cf 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c
> @@ -9,4 +9,6 @@ foo (int i)
>    return i;
>  }
>
> -/* { dg-final { scan-tree-dump-not "ABS" "optimized" } } */
> +/* We should not have ABS_EXPR but ABSU_EXPR instead. */
> +/* { dg-final { scan-tree-dump-not "ABS_EXPR" "optimized" } } */
> +/* { dg-final { scan-tree-dump "ABSU" "optimized" } } */
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 9bda1b2a397..97540e30d55 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -63,8 +63,6 @@ static int value_replacement (basic_block, basic_block,
>                               edge, edge, gphi *, tree, tree);
>  static bool minmax_replacement (basic_block, basic_block,
>                                 edge, edge, gphi *, tree, tree);
> -static bool abs_replacement (basic_block, basic_block,
> -                            edge, edge, gphi *, tree, tree);
>  static bool spaceship_replacement (basic_block, basic_block,
>                                    edge, edge, gphi *, tree, tree);
>  static bool cond_removal_in_popcount_clz_ctz_pattern (basic_block, basic_block,
> @@ -350,8 +348,6 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
>                                                arg0, arg1,
>                                                early_p))
>             cfgchanged = true;
> -         else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
> -           cfgchanged = true;
>           else if (!early_p
>                    && cond_removal_in_popcount_clz_ctz_pattern (bb, bb1, e1,
>                                                                 e2, phi, arg0,
> @@ -2606,134 +2602,6 @@ cond_removal_in_popcount_clz_ctz_pattern (basic_block cond_bb,
>    return true;
>  }
>
> -/*  The function absolute_replacement does the main work of doing the absolute
> -    replacement.  Return true if the replacement is done.  Otherwise return
> -    false.
> -    bb is the basic block where the replacement is going to be done on.  arg0
> -    is argument 0 from the phi.  Likewise for arg1.  */
> -
> -static bool
> -abs_replacement (basic_block cond_bb, basic_block middle_bb,
> -                edge e0 ATTRIBUTE_UNUSED, edge e1,
> -                gphi *phi, tree arg0, tree arg1)
> -{
> -  tree result;
> -  gassign *new_stmt;
> -  gimple *cond;
> -  gimple_stmt_iterator gsi;
> -  edge true_edge, false_edge;
> -  gimple *assign;
> -  edge e;
> -  tree rhs, lhs;
> -  bool negate;
> -  enum tree_code cond_code;
> -
> -  /* If the type says honor signed zeros we cannot do this
> -     optimization.  */
> -  if (HONOR_SIGNED_ZEROS (arg1))
> -    return false;
> -
> -  /* OTHER_BLOCK must have only one executable statement which must have the
> -     form arg0 = -arg1 or arg1 = -arg0.  */
> -
> -  assign = last_and_only_stmt (middle_bb);
> -  /* If we did not find the proper negation assignment, then we cannot
> -     optimize.  */
> -  if (assign == NULL)
> -    return false;
> -
> -  /* If we got here, then we have found the only executable statement
> -     in OTHER_BLOCK.  If it is anything other than arg = -arg1 or
> -     arg1 = -arg0, then we cannot optimize.  */
> -  if (gimple_code (assign) != GIMPLE_ASSIGN)
> -    return false;
> -
> -  lhs = gimple_assign_lhs (assign);
> -
> -  if (gimple_assign_rhs_code (assign) != NEGATE_EXPR)
> -    return false;
> -
> -  rhs = gimple_assign_rhs1 (assign);
> -
> -  /* The assignment has to be arg0 = -arg1 or arg1 = -arg0.  */
> -  if (!(lhs == arg0 && rhs == arg1)
> -      && !(lhs == arg1 && rhs == arg0))
> -    return false;
> -
> -  cond = last_stmt (cond_bb);
> -  result = PHI_RESULT (phi);
> -
> -  /* Only relationals comparing arg[01] against zero are interesting.  */
> -  cond_code = gimple_cond_code (cond);
> -  if (cond_code != GT_EXPR && cond_code != GE_EXPR
> -      && cond_code != LT_EXPR && cond_code != LE_EXPR)
> -    return false;
> -
> -  /* Make sure the conditional is arg[01] OP y.  */
> -  if (gimple_cond_lhs (cond) != rhs)
> -    return false;
> -
> -  if (FLOAT_TYPE_P (TREE_TYPE (gimple_cond_rhs (cond)))
> -              ? real_zerop (gimple_cond_rhs (cond))
> -              : integer_zerop (gimple_cond_rhs (cond)))
> -    ;
> -  else
> -    return false;
> -
> -  /* We need to know which is the true edge and which is the false
> -     edge so that we know if have abs or negative abs.  */
> -  extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
> -
> -  /* For GT_EXPR/GE_EXPR, if the true edge goes to OTHER_BLOCK, then we
> -     will need to negate the result.  Similarly for LT_EXPR/LE_EXPR if
> -     the false edge goes to OTHER_BLOCK.  */
> -  if (cond_code == GT_EXPR || cond_code == GE_EXPR)
> -    e = true_edge;
> -  else
> -    e = false_edge;
> -
> -  if (e->dest == middle_bb)
> -    negate = true;
> -  else
> -    negate = false;
> -
> -  /* If the code negates only iff positive then make sure to not
> -     introduce undefined behavior when negating or computing the absolute.
> -     ???  We could use range info if present to check for arg1 == INT_MIN.  */
> -  if (negate
> -      && (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg1))
> -         && ! TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1))))
> -    return false;
> -
> -  result = duplicate_ssa_name (result, NULL);
> -
> -  if (negate)
> -    lhs = make_ssa_name (TREE_TYPE (result));
> -  else
> -    lhs = result;
> -
> -  /* Build the modify expression with abs expression.  */
> -  new_stmt = gimple_build_assign (lhs, ABS_EXPR, rhs);
> -
> -  gsi = gsi_last_bb (cond_bb);
> -  gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT);
> -
> -  if (negate)
> -    {
> -      /* Get the right GSI.  We want to insert after the recently
> -        added ABS_EXPR statement (which we know is the first statement
> -        in the block.  */
> -      new_stmt = gimple_build_assign (result, NEGATE_EXPR, lhs);
> -
> -      gsi_insert_after (&gsi, new_stmt, GSI_NEW_STMT);
> -    }
> -
> -  replace_phi_edge_with_variable (cond_bb, e1, phi, result);
> -
> -  /* Note that we optimized this PHI.  */
> -  return true;
> -}
> -
>  /* Auxiliary functions to determine the set of memory accesses which
>     can't trap because they are preceded by accesses to the same memory
>     portion.  We do that for MEM_REFs, so we only need to track
> @@ -3670,7 +3538,7 @@ gate_hoist_loads (void)
>     ABS Replacement
>     ---------------
>
> -   This transformation, implemented in abs_replacement, replaces
> +   This transformation, implemented in match_simplify_replacement, replaces
>
>       bb0:
>         if (a >= 0) goto bb2; else goto bb1;
> --
> 2.27.0
>

Re: [PATCH 1/4] Duplicate the range information of the phi onto the new ssa_name

[Martin Sebor]

On 6/27/21 5:24 PM, apinski--- via Gcc-patches wrote:
> From: Andrew Pinski <apinski@marvell.com>
> 
> Since match_simplify_replacement uses gimple_simplify, there is a new
> ssa name created sometimes and then we go and replace the phi edge with
> this new ssa name, the range information on the phi is lost.
> Placing this in replace_phi_edge_with_variable is the best option instead
> of doing it in each time replace_phi_edge_with_variable is called which is
> what is done today.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* tree-ssa-phiopt.c (replace_phi_edge_with_variable): Duplicate range
> 	info if we're the only things setting the target PHI.
> 	(value_replacement): Don't duplicate range here.
> 	(minmax_replacement): Likewise.
> ---
>   gcc/tree-ssa-phiopt.c | 43 ++++++++++++++++++++++++++-----------------
>   1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 1777bff2f7c..ab12e85569d 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -391,6 +391,32 @@ replace_phi_edge_with_variable (basic_block cond_block,
>     basic_block bb = gimple_bb (phi);
>     basic_block block_to_remove;
>     gimple_stmt_iterator gsi;
> +  tree phi_result = PHI_RESULT (phi);
> +
> +  /* Duplicate range info if we're the only things setting the target PHI.

I'm not too familiar with the code so the comments are helpful.  But
I don't understand what you mean by "we're the only things" above.
(What's "we" and what might be some of the other "things?")  Can you
please clarify that comment?

> +     This is needed as later on, the new_tree will be replacing
> +     The assignement of the PHI.
> +     For an example:
> +     bb1:
> +     _4 = min<a_1, 255>
> +     goto bb2
> +
> +     range<-INF,255>
> +     a_3 = PHI<_4(1)>
> +     bb3:
> +
> +     use(a_3)
> +     And _4 gets prograted into the use of a_3 and losing the range info.
> +     This can't be done for more than 2 incoming edges as the progration
> +     won't happen.  */

Presumably you mean propagated and propagation above?

Martin

> +  if (TREE_CODE (new_tree) == SSA_NAME
> +      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
> +      && INTEGRAL_TYPE_P (TREE_TYPE (phi_result))
> +      && !SSA_NAME_RANGE_INFO (new_tree)
> +      && SSA_NAME_RANGE_INFO (phi_result))
> +    duplicate_ssa_name_range_info (new_tree,
> +				   SSA_NAME_RANGE_TYPE (phi_result),
> +				   SSA_NAME_RANGE_INFO (phi_result));
>   
>     /* Change the PHI argument to new.  */
>     SET_USE (PHI_ARG_DEF_PTR (phi, e->dest_idx), new_tree);
> @@ -1385,16 +1411,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
>   	   <bb 4>:
>   	   # u_3 = PHI <u_6(3), 4294967295(2)>  */
>         reset_flow_sensitive_info (lhs);
> -      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
> -	{
> -	  /* If available, we can use VR of phi result at least.  */
> -	  tree phires = gimple_phi_result (phi);
> -	  struct range_info_def *phires_range_info
> -	    = SSA_NAME_RANGE_INFO (phires);
> -	  if (phires_range_info)
> -	    duplicate_ssa_name_range_info (lhs, SSA_NAME_RANGE_TYPE (phires),
> -					   phires_range_info);
> -	}
>         gimple_stmt_iterator gsi_from;
>         for (int i = prep_cnt - 1; i >= 0; --i)
>   	{
> @@ -1794,13 +1810,6 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb,
>     gimple_seq stmts = NULL;
>     tree phi_result = PHI_RESULT (phi);
>     result = gimple_build (&stmts, minmax, TREE_TYPE (phi_result), arg0, arg1);
> -  /* Duplicate range info if we're the only things setting the target PHI.  */
> -  if (!gimple_seq_empty_p (stmts)
> -      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
> -      && !POINTER_TYPE_P (TREE_TYPE (phi_result))
> -      && SSA_NAME_RANGE_INFO (phi_result))
> -    duplicate_ssa_name_range_info (result, SSA_NAME_RANGE_TYPE (phi_result),
> -				   SSA_NAME_RANGE_INFO (phi_result));
>   
>     gsi = gsi_last_bb (cond_bb);
>     gsi_insert_seq_before (&gsi, stmts, GSI_NEW_STMT);
> 

Re: [PATCH 2/4] Allow match-and-simplified phiopt to run in early phiopt

[Martin Sebor]

On 6/27/21 5:24 PM, apinski--- via Gcc-patches wrote:
> From: Andrew Pinski <apinski@marvell.com>
> 
> To move a few things more to match-and-simplify from phiopt,
> we need to allow match_simplify_replacement to run in early
> phiopt. To do this we add a replacement for gimple_simplify
> that is explictly for phiopt.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no
> regressions.
> 
> gcc/ChangeLog:
> 
> 	* tree-ssa-phiopt.c (match_simplify_replacement):
> 	Add early_p argument. Call gimple_simplify_phiopt
> 	instead of gimple_simplify.
> 	(tree_ssa_phiopt_worker): Update call to
> 	match_simplify_replacement and allow unconditionally.
> 	(phiopt_early_allow): New function.
> 	(gimple_simplify_phiopt): New function.
> ---
>   gcc/tree-ssa-phiopt.c | 89 ++++++++++++++++++++++++++++++++++---------
>   1 file changed, 70 insertions(+), 19 deletions(-)
> 
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index ab12e85569d..17bc597851b 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -50,12 +50,13 @@ along with GCC; see the file COPYING3.  If not see
>   #include "gimple-fold.h"
>   #include "internal-fn.h"
>   #include "gimple-range.h"
> +#include "gimple-match.h"
>   
>   static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
>   static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
>   				   tree, tree);
>   static bool match_simplify_replacement (basic_block, basic_block,
> -					edge, edge, gphi *, tree, tree);
> +					edge, edge, gphi *, tree, tree, bool);
>   static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree,
>   						gimple *);
>   static int value_replacement (basic_block, basic_block,
> @@ -345,9 +346,9 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
>   	  /* Do the replacement of conditional if it can be done.  */
>   	  if (!early_p && two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
>   	    cfgchanged = true;
> -	  else if (!early_p
> -		   && match_simplify_replacement (bb, bb1, e1, e2, phi,
> -						  arg0, arg1))
> +	  else if (match_simplify_replacement (bb, bb1, e1, e2, phi,
> +					       arg0, arg1,
> +					       early_p))
>   	    cfgchanged = true;
>   	  else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
>   	    cfgchanged = true;
> @@ -811,6 +812,67 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
>     return true;
>   }
>   
> +/* Return TRUE if CODE should be allowed during early phiopt.
> +   Currently this is to allow MIN/MAX and ABS/NEGATE.  */
> +static bool
> +phiopt_early_allow (enum tree_code code)
> +{
> +  switch (code)
> +    {
> +      case MIN_EXPR:
> +      case MAX_EXPR:
> +      case ABS_EXPR:
> +      case ABSU_EXPR:
> +      case NEGATE_EXPR:
> +      case SSA_NAME:
> +	return true;
> +      default:
> +	return false;
> +    }
> +}
> +
> +/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
> +   Return NULL if nothing can be simplified or the resulting simplified value
> +   with parts pushed if EARLY_P was true. Also rejects non allowed tree code
> +   if EARLY_P is set.
> +   Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries
> +   to simplify CMP ? ARG0 : ARG1.  */
> +static tree
> +gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> +			tree arg0, tree arg1,
> +			gimple_seq *seq)
> +{
> +  tree result;
> +  enum tree_code comp_code = gimple_cond_code (comp_stmt);
> +  location_t loc = gimple_location (comp_stmt);
> +  tree cmp0 = gimple_cond_lhs (comp_stmt);
> +  tree cmp1 = gimple_cond_rhs (comp_stmt);
> +  /* To handle special cases like floating point comparison, it is easier and
> +     less error-prone to build a tree and gimplify it on the fly though it is
> +     less efficient.
> +     Don't use fold_build2 here as that might create (bool)a instead of just
> +     "a != 0".  */
> +  tree cond = build2_loc (loc, comp_code, boolean_type_node,
> +			  cmp0, cmp1);
> +  gimple_match_op op (gimple_match_cond::UNCOND,
> +		      COND_EXPR, type, cond, arg0, arg1);
> +
> +  if (op.resimplify (early_p ? NULL : seq, follow_all_ssa_edges))
> +    {
> +      /* Early we want only to allow some generated tree codes. */
> +      if (!early_p
> +	  || op.code.is_tree_code ()
> +	  || phiopt_early_allow ((tree_code)op.code))
> +	{
> +	  result = maybe_push_res_to_seq (&op, seq);
> +	  if (result)
> +	    return result;

It looks to me like the last if statement is redundant and could
be replaced by

   return maybe_push_res_to_seq (&op, seq);

thus also making the result variable redundant, further simplifying
the code.

Martin

> +	}
> +    }
> +
> +  return NULL;
> +}
> +
>   /*  The function match_simplify_replacement does the main work of doing the
>       replacement using match and simplify.  Return true if the replacement is done.
>       Otherwise return false.
> @@ -820,10 +882,9 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
>   static bool
>   match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>   			    edge e0, edge e1, gphi *phi,
> -			    tree arg0, tree arg1)
> +			    tree arg0, tree arg1, bool early_p)
>   {
>     gimple *stmt;
> -  tree cond;
>     gimple_stmt_iterator gsi;
>     edge true_edge, false_edge;
>     gimple_seq seq = NULL;
> @@ -884,15 +945,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>   
>     stmt = last_stmt (cond_bb);
>   
> -  /* To handle special cases like floating point comparison, it is easier and
> -     less error-prone to build a tree and gimplify it on the fly though it is
> -     less efficient.
> -     Don't use fold_build2 here as that might create (bool)a instead of just
> -     "a != 0".  */
> -  cond = build2_loc (gimple_location (stmt),
> -		     gimple_cond_code (stmt), boolean_type_node,
> -		     gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
> -
>     /* We need to know which is the true edge and which is the false
>        edge so that we know when to invert the condition below.  */
>     extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
> @@ -900,10 +952,9 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>       std::swap (arg0, arg1);
>   
>     tree type = TREE_TYPE (gimple_phi_result (phi));
> -  result = gimple_simplify (COND_EXPR, type,
> -			    cond,
> -			    arg0, arg1,
> -			    &seq, NULL);
> +  result = gimple_simplify_phiopt (early_p, type, stmt,
> +				   arg0, arg1,
> +				   &seq);
>     if (!result)
>       return false;
>   
> 

Re: [PATCH 2/4] Allow match-and-simplified phiopt to run in early phiopt

[Andrew Pinski]

On Tue, Jun 29, 2021 at 12:14 PM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On 6/27/21 5:24 PM, apinski--- via Gcc-patches wrote:
> > From: Andrew Pinski <apinski@marvell.com>
> >
> > To move a few things more to match-and-simplify from phiopt,
> > we need to allow match_simplify_replacement to run in early
> > phiopt. To do this we add a replacement for gimple_simplify
> > that is explictly for phiopt.
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no
> > regressions.
> >
> > gcc/ChangeLog:
> >
> >       * tree-ssa-phiopt.c (match_simplify_replacement):
> >       Add early_p argument. Call gimple_simplify_phiopt
> >       instead of gimple_simplify.
> >       (tree_ssa_phiopt_worker): Update call to
> >       match_simplify_replacement and allow unconditionally.
> >       (phiopt_early_allow): New function.
> >       (gimple_simplify_phiopt): New function.
> > ---
> >   gcc/tree-ssa-phiopt.c | 89 ++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 70 insertions(+), 19 deletions(-)
> >
> > diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> > index ab12e85569d..17bc597851b 100644
> > --- a/gcc/tree-ssa-phiopt.c
> > +++ b/gcc/tree-ssa-phiopt.c
> > @@ -50,12 +50,13 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "gimple-fold.h"
> >   #include "internal-fn.h"
> >   #include "gimple-range.h"
> > +#include "gimple-match.h"
> >
> >   static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
> >   static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
> >                                  tree, tree);
> >   static bool match_simplify_replacement (basic_block, basic_block,
> > -                                     edge, edge, gphi *, tree, tree);
> > +                                     edge, edge, gphi *, tree, tree, bool);
> >   static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree,
> >                                               gimple *);
> >   static int value_replacement (basic_block, basic_block,
> > @@ -345,9 +346,9 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
> >         /* Do the replacement of conditional if it can be done.  */
> >         if (!early_p && two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
> >           cfgchanged = true;
> > -       else if (!early_p
> > -                && match_simplify_replacement (bb, bb1, e1, e2, phi,
> > -                                               arg0, arg1))
> > +       else if (match_simplify_replacement (bb, bb1, e1, e2, phi,
> > +                                            arg0, arg1,
> > +                                            early_p))
> >           cfgchanged = true;
> >         else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
> >           cfgchanged = true;
> > @@ -811,6 +812,67 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
> >     return true;
> >   }
> >
> > +/* Return TRUE if CODE should be allowed during early phiopt.
> > +   Currently this is to allow MIN/MAX and ABS/NEGATE.  */
> > +static bool
> > +phiopt_early_allow (enum tree_code code)
> > +{
> > +  switch (code)
> > +    {
> > +      case MIN_EXPR:
> > +      case MAX_EXPR:
> > +      case ABS_EXPR:
> > +      case ABSU_EXPR:
> > +      case NEGATE_EXPR:
> > +      case SSA_NAME:
> > +     return true;
> > +      default:
> > +     return false;
> > +    }
> > +}
> > +
> > +/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
> > +   Return NULL if nothing can be simplified or the resulting simplified value
> > +   with parts pushed if EARLY_P was true. Also rejects non allowed tree code
> > +   if EARLY_P is set.
> > +   Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries
> > +   to simplify CMP ? ARG0 : ARG1.  */
> > +static tree
> > +gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> > +                     tree arg0, tree arg1,
> > +                     gimple_seq *seq)
> > +{
> > +  tree result;
> > +  enum tree_code comp_code = gimple_cond_code (comp_stmt);
> > +  location_t loc = gimple_location (comp_stmt);
> > +  tree cmp0 = gimple_cond_lhs (comp_stmt);
> > +  tree cmp1 = gimple_cond_rhs (comp_stmt);
> > +  /* To handle special cases like floating point comparison, it is easier and
> > +     less error-prone to build a tree and gimplify it on the fly though it is
> > +     less efficient.
> > +     Don't use fold_build2 here as that might create (bool)a instead of just
> > +     "a != 0".  */
> > +  tree cond = build2_loc (loc, comp_code, boolean_type_node,
> > +                       cmp0, cmp1);
> > +  gimple_match_op op (gimple_match_cond::UNCOND,
> > +                   COND_EXPR, type, cond, arg0, arg1);
> > +
> > +  if (op.resimplify (early_p ? NULL : seq, follow_all_ssa_edges))
> > +    {
> > +      /* Early we want only to allow some generated tree codes. */
> > +      if (!early_p
> > +       || op.code.is_tree_code ()
> > +       || phiopt_early_allow ((tree_code)op.code))
> > +     {
> > +       result = maybe_push_res_to_seq (&op, seq);
> > +       if (result)
> > +         return result;
>
> It looks to me like the last if statement is redundant and could
> be replaced by
>
>    return maybe_push_res_to_seq (&op, seq);
>
> thus also making the result variable redundant, further simplifying
> the code.

The next patch after this one will change it anyways.

Thanks,
Andrew


>
> Martin
>
> > +     }
> > +    }
> > +
> > +  return NULL;
> > +}
> > +
> >   /*  The function match_simplify_replacement does the main work of doing the
> >       replacement using match and simplify.  Return true if the replacement is done.
> >       Otherwise return false.
> > @@ -820,10 +882,9 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
> >   static bool
> >   match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
> >                           edge e0, edge e1, gphi *phi,
> > -                         tree arg0, tree arg1)
> > +                         tree arg0, tree arg1, bool early_p)
> >   {
> >     gimple *stmt;
> > -  tree cond;
> >     gimple_stmt_iterator gsi;
> >     edge true_edge, false_edge;
> >     gimple_seq seq = NULL;
> > @@ -884,15 +945,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
> >
> >     stmt = last_stmt (cond_bb);
> >
> > -  /* To handle special cases like floating point comparison, it is easier and
> > -     less error-prone to build a tree and gimplify it on the fly though it is
> > -     less efficient.
> > -     Don't use fold_build2 here as that might create (bool)a instead of just
> > -     "a != 0".  */
> > -  cond = build2_loc (gimple_location (stmt),
> > -                  gimple_cond_code (stmt), boolean_type_node,
> > -                  gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
> > -
> >     /* We need to know which is the true edge and which is the false
> >        edge so that we know when to invert the condition below.  */
> >     extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
> > @@ -900,10 +952,9 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
> >       std::swap (arg0, arg1);
> >
> >     tree type = TREE_TYPE (gimple_phi_result (phi));
> > -  result = gimple_simplify (COND_EXPR, type,
> > -                         cond,
> > -                         arg0, arg1,
> > -                         &seq, NULL);
> > +  result = gimple_simplify_phiopt (early_p, type, stmt,
> > +                                arg0, arg1,
> > +                                &seq);
> >     if (!result)
> >       return false;
> >
> >
>