On Mon, Nov 15, 2021 at 1:09 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apin...@marvell.com>
>
> For this PR, we have:
> if (d_5 < 0)
> goto <bb 3>; [INV]
> else
> goto <bb 4>; [INV]
>
> <bb 3> :
> v_7 = c_4 | -128;
>
> <bb 4> :
> # v_1 = PHI <c_4(2), v_7(3)>
>
> Which PHI-OPT will try to simplify
> "(d_5 < 0) ? (c_4 | -128) : c_4" which is not handled currently.
> This adds a few patterns which allows to try to see if (a ? CST : CST1)
> where CST1 is either 0, 1 or -1 depending on the operator.
> Note to optimize this case always, we should check to make sure that
> the a?CST:CST1 gets simplified to not include the conditional expression.
> The ! flag does not work as we want to have more simplifcations than just
> when we simplify it to a leaf node (SSA_NAME or CONSTANT). This adds a new
> flag ^ to genmatch which says the simplification should happen but not down
> to the same kind of node.
> We could allow this for !GIMPLE and use fold_* rather than fold_buildN but I
> didn't see any use of it for now.
>
> Also all of these patterns need to be done late as other optimizations can be
> done without them.
>
> OK? Bootstrapped and tested on x86_64 with no regressions.
>
> gcc/ChangeLog:
>
> * doc/match-and-simplify.texi: Document ^ flag.
> * genmatch.c (expr::expr): Add Setting of force_simplify.
> (expr): Add force_simplify field.
> (expr::gen_transform): Add support for force_simplify field.
> (parser::parse_expr): Add parsing of ^ flag for the expr.
> * match.pd: New patterns to optimize "a ? (b op CST) : b".
> ---
> gcc/doc/match-and-simplify.texi | 16 +++++++++++++
> gcc/genmatch.c | 35 ++++++++++++++++++++++++++--
> gcc/match.pd | 41 +++++++++++++++++++++++++++++++++
> 3 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/doc/match-and-simplify.texi b/gcc/doc/match-and-simplify.texi
> index e7e5a4f7299..4e3407c0263 100644
> --- a/gcc/doc/match-and-simplify.texi
> +++ b/gcc/doc/match-and-simplify.texi
> @@ -377,6 +377,22 @@ of the @code{vec_cond} expression but only if the actual
> plus
> operations both simplify. Note this is currently only supported
> for code generation targeting @code{GIMPLE}.
>
> +Another modifier for generated expressions is @code{^} which
> +tells the machinery to only consider the simplification in case
> +the marked expression simplified away from the original code.
> +Consider for example
> +
> +@smallexample
> +(simplify
> + (cond @@0 (plus:s @@1 INTEGER_CST@@2) @@1)
> + (plus @@1 (cond^ @@0 @@2 @{ build_zero_cst (type); @})))
> +@end smallexample
> +
> +which moves the inner @code{plus} operation to the outside of the
> +@code{cond} expression but only if the actual cond operation simplify
> +wayaway from cond. Note this is currently only supported for code
s/wayaway/away/
> +generation targeting @code{GIMPLE}.
> +
> As intermediate conversions are often optional there is a way to
> avoid the need to repeat patterns both with and without such
> conversions. Namely you can mark a conversion as being optional
> diff --git a/gcc/genmatch.c b/gcc/genmatch.c
> index 95248455ec5..2dca1141df6 100644
> --- a/gcc/genmatch.c
> +++ b/gcc/genmatch.c
> @@ -698,12 +698,13 @@ public:
> : operand (OP_EXPR, loc), operation (operation_),
> ops (vNULL), expr_type (NULL), is_commutative (is_commutative_),
> is_generic (false), force_single_use (false), force_leaf (false),
> - opt_grp (0) {}
> + force_simplify(false), opt_grp (0) {}
> expr (expr *e)
> : operand (OP_EXPR, e->location), operation (e->operation),
> ops (vNULL), expr_type (e->expr_type), is_commutative
> (e->is_commutative),
> is_generic (e->is_generic), force_single_use (e->force_single_use),
> - force_leaf (e->force_leaf), opt_grp (e->opt_grp) {}
> + force_leaf (e->force_leaf), force_simplify(e->force_simplify),
> + opt_grp (e->opt_grp) {}
> void append_op (operand *op) { ops.safe_push (op); }
> /* The operator and its operands. */
> id_base *operation;
> @@ -721,6 +722,9 @@ public:
> /* Whether in the result expression this should be a leaf node
> with any children simplified down to simple operands. */
> bool force_leaf;
> + /* Whether in the result expression this should be a node
> + with any children simplified down not to use the original operator. */
> + bool force_simplify;
> /* If non-zero, the group for optional handling. */
> unsigned char opt_grp;
> virtual void gen_transform (FILE *f, int, const char *, bool, int,
> @@ -2527,6 +2531,17 @@ expr::gen_transform (FILE *f, int indent, const char
> *dest, bool gimple,
> fprintf (f, ", _o%d[%u]", depth, i);
> fprintf (f, ");\n");
> fprintf_indent (f, indent, "tem_op.resimplify (lseq, valueize);\n");
I wonder if with force_simplify we should pass NULL as lseq to resimplify?
That is, should we allow (plus^ (convert @0) @1) to simplify to
(convert (plus @0 @1')) if @1 is trivially convertible for example?
"away from" is a bit of a grey area here. I see you want a COND_EXPR
to become a non-COND_EXPR
> + if (force_simplify)
> + {
> + fprintf_indent (f, indent, "if (tem_op.code.is_tree_code ())\n");
> + fprintf_indent (f, indent, " {\n");
> + indent+=4;
> + fprintf_indent (f, indent, "if (((tree_code)tem_op.code) == %s)\n",
> + opr_name);
> + fprintf_indent (f, indent, " goto %s;\n", fail_label);
> + indent-=4;
> + fprintf_indent (f, indent, " }\n");
> + }
> fprintf_indent (f, indent,
> "_r%d = maybe_push_res_to_seq (&tem_op, %s);\n", depth,
> !force_leaf ? "lseq" : "NULL");
> @@ -4304,6 +4319,22 @@ parser::parse_expr ()
> e->force_leaf = true;
> }
>
> + if (!parsing_match_operand
> + && token->type == CPP_XOR
> + && !(token->flags & PREV_WHITE))
> + {
> + if (!gimple)
> + fatal_at (token, "forcing simplification is not supported for
> GENERIC");
> + if (e->force_leaf)
> + fatal_at (token, "forcing simplification and forcing to a leaf is not
> "
> + "supported");
> + if (e->operation->kind != id_base::CODE)
> + fatal_at (token, "forcing simplification is not supported on non-CODE
> "
> + "operations");
> + eat_token (CPP_XOR);
> + e->force_simplify = true;
> + }
> +
> if (token->type == CPP_COLON
> && !(token->flags & PREV_WHITE))
> {
> diff --git a/gcc/match.pd b/gcc/match.pd
> index df31964e02f..c66e918bb65 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4186,6 +4186,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> )
> #endif
>
> +#if GIMPLE
> +(if (canonicalize_math_p ())
> +/* These patterns are mostly used by PHIOPT to move some operations outside
> of
> + the if statements. They should be done late because it gives jump
> threading
> + and few other passes to reduce what is going on. */
but canonicalize_math_p is _early_ ...?
> +/* a ? x op CST : x -> x op (a ? CST : 0) if (a ? CST : 0) can be
> simplified. */
> + (for op (plus minus bit_ior bit_xor lshift rshift lrotate rrotate)
> + (simplify
> + (cond @0 (op:s @1 INTEGER_CST@2) @1)
> + (op @1 (cond^ @0 @2 { build_zero_cst (type); }))
> + )
> + (simplify
> + (cond @0 @1 (op:s @1 INTEGER_CST@2))
> + (op @1 (cond^ @0 { build_zero_cst (type); } @2))
> + )
> + )
> +/* a ? x op CST : x -> x op (a ? CST : 1) if (a ? CST : 1) can be
> simplified. */
> + (for op (mult trunc_div ceil_div floor_div)
> + (simplify
> + (cond @0 (op:s @1 INTEGER_CST@2) @1)
> + (op @1 (cond^ @0 @2 { build_one_cst (type); }))
> + )
> + (simplify
> + (cond @0 @1 (op:s @1 INTEGER_CST@2))
> + (op @1 (cond^ @0 { build_one_cst (type); } @2))
> + )
> + )
> +/* a ? x op CST : x -> x op (a ? CST : -1) if (a ? CST : -1) can be
> simplified. */
> + (for op (bit_and)
> + (simplify
> + (cond @0 (op @1 INTEGER_CST@2) @1)
> + (op @1 (cond^ @0 @2 { build_all_ones_cst (type); }))
> + )
> + (simplify
> + (cond @0 @1 (op @1 INTEGER_CST@2))
> + (op @1 (cond^ @0 { build_all_ones_cst (type); } @2))
> + )
> + )
> +)
I wonder if phiopt could simply try simplifying (cond ...) itself,
thus programmatically do what you do with ^ and those extra patterns.
Richard.
> +#endif
> +
> /* Simplification moved from fold_cond_expr_with_comparison. It may also
> be extended. */
> /* This pattern implements two kinds simplification:
> --
> 2.17.1
>
