diff mbox series

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

Message ID 1624486755-12879-8-git-send-email-apinski@marvell.com
State New
Headers show
Series PHI-OPT move abs_replacement to match.pd | expand

Commit Message

Li, Pan2 via Gcc-patches June 23, 2021, 10:19 p.m. UTC
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(-)

Comments

Jeff Law June 24, 2021, 3:19 p.m. UTC | #1
On 6/23/2021 4:19 PM, apinski--- via Gcc-patches 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.
And I've already ack'd this part :-)  I think it's unchanged since he 
original.

Jeff
diff mbox series

Patch

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 5f099eca343..cd582d08dfc 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,
@@ -2598,134 +2594,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
@@ -3659,7 +3527,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;