On Sun, Jul 4, 2021 at 8:42 PM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apin...@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.
OK
> gcc/ChangeLog:
>
> PR tree-optimization/101039
> * 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:
>
> PR tree-optimization/101039
> * gcc.dg/tree-ssa/phi-opt-15.c: Update test to expect
> ABSU and still not expect ABS_EXPR.
> * gcc.dg/tree-ssa/phi-opt-23.c: New test.
> * gcc.dg/tree-ssa/phi-opt-24.c: New test.
> ---
> gcc/match.pd | 60 +++++++++
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-15.c | 4 +-
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-23.c | 44 +++++++
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-24.c | 44 +++++++
> gcc/tree-ssa-phiopt.c | 134 +--------------------
> 5 files changed, 152 insertions(+), 134 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-23.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-24.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 4e10d54383c..72860fbd448 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 (type))
> + @1))
> + (simplify
> + (cnd (cmp @0 zerop) integer_zerop (negate@1 @0))
> + (if (!HONOR_SIGNED_ZEROS (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 (type))
> + @0))
> + (simplify
> + (cnd (cmp @0 zerop) @0 integer_zerop)
> + (if (!HONOR_SIGNED_ZEROS (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 (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 (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/testsuite/gcc.dg/tree-ssa/phi-opt-23.c
> b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-23.c
> new file mode 100644
> index 00000000000..ff658cd16a7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-23.c
> @@ -0,0 +1,44 @@
> +/* { dg-options "-O2 -fdump-tree-phiopt" } */
> +
> +int f0(int A)
> +{
> +// A == 0? A : -A same as -A
> + if (A == 0) return A;
> + return -A;
> +}
> +
> +int f1(int A)
> +{
> +// A != 0? A : -A same as A
> + if (A != 0) return A;
> + return -A;
> +}
> +int f2(int A)
> +{
> +// A >= 0? A : -A same as abs (A)
> + if (A >= 0) return A;
> + return -A;
> +}
> +int f3(int A)
> +{
> +// A > 0? A : -A same as abs (A)
> + if (A > 0) return A;
> + return -A;
> +}
> +int f4(int A)
> +{
> +// A <= 0? A : -A same as -abs (A)
> + if (A <= 0) return A;
> + return -A;
> +}
> +int f5(int A)
> +{
> +// A < 0? A : -A same as -abs (A)
> + if (A < 0) return A;
> + return -A;
> +}
> +
> +/* These should be optimized in phiopt1 but is confused by predicts. */
> +/* { dg-final { scan-tree-dump-not "if" "phiopt1" { xfail *-*-* } } } */
> +/* { dg-final { scan-tree-dump-not "if" "phiopt2" } } */
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-24.c
> b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-24.c
> new file mode 100644
> index 00000000000..eb89decb4bf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-24.c
> @@ -0,0 +1,44 @@
> +/* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */
> +
> +float f0(float A)
> +{
> +// A == 0? A : -A same as -A
> + if (A == 0) return A;
> + return -A;
> +}
> +
> +float f1(float A)
> +{
> +// A != 0? A : -A same as A
> + if (A != 0) return A;
> + return -A;
> +}
> +float f2(float A)
> +{
> +// A >= 0? A : -A same as abs (A)
> + if (A >= 0) return A;
> + return -A;
> +}
> +float f3(float A)
> +{
> +// A > 0? A : -A same as abs (A)
> + if (A > 0) return A;
> + return -A;
> +}
> +float f4(float A)
> +{
> +// A <= 0? A : -A same as -abs (A)
> + if (A <= 0) return A;
> + return -A;
> +}
> +float f5(float A)
> +{
> +// A < 0? A : -A same as -abs (A)
> + if (A < 0) return A;
> + return -A;
> +}
> +
> +/* These should be optimized in phiopt1 but is confused by predicts. */
> +/* { dg-final { scan-tree-dump-not "if" "phiopt1" { xfail *-*-* } } } */
> +/* { dg-final { scan-tree-dump-not "if" "phiopt2" } } */
> +
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index fec8c02c062..a1664c1f914 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -64,8 +64,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,
> @@ -352,8 +350,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,
> @@ -2614,134 +2610,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
> @@ -3678,7 +3546,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
>
