On Mon, Jun 21, 2021 at 2:50 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Sat, Jun 19, 2021 at 11:44 PM apinski--- via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > From: Andrew Pinski <apin...@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 need to mark some match patterns
> > if they can be done in early phiopt or not.
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no
> > regressions.
>
> I wonder if it would be easier to change
>
> result = gimple_simplify (COND_EXPR, type,
> cond,
> arg0, arg1,
> &seq, NULL);
>
> to
>
> gimple_match_op op (gimple_match_cond::UNCOND,
> COND_EXPR, type, arg0, arg1);
> if (op.resimplify (early_p ? NULL : &seq, NULL))
> {
> /* Early we only want MIN/MAX, etc. */
> if (early_p
> && (!op.code.is_tree_code ()
> || (tree_code)op.code != ...))
> return false;
> result = mabe_push_res_to_seq (op, &seq);
> if (!result)
> ...
>
> thus avoid complex transforms by passing a NULL seq. 'op'
> will be the piecewise result which you can check before
> eventually re-materializing it as GIMPLE stmt via
> mabe_push_res_to_seq.
This sounds like a good idea and I have an implementation for it
and it seems to work.
>
> Note I didn't really try to look at the ultimate results of allowing
> all phi-opts early but the testsuite fallout was spectacular as
> (too) simple jump-threading tests fail since the code is no longer
> jumpy as we arrive at the first DOM/VRP ...
One thing I noticed when I tried having non-early mode before was that
the if-to-switch optimization also needed "a ? CST1 : CST2" conversion
to happen early. I did not check out fully or even record which testcases
failed.
Thanks,
Andrew
>
> Richard.
>
> > gcc/ChangeLog:
> >
> > * generic-match-head.c (phiopt_earlymode): New function.
> > * gimple-match-head.c (phiopt_earlymode): New function.
> > * match.pd (A ? CST0 : CST1): Disable for early phiopt.
> > (x >= 0 ? ~y : y): Likewise.
> > (x >= 0 ? y : ~y): Likewise.
> > * tree-pass.h (PROP_gimple_lomp_dev): Increment bit by one.
> > (PROP_rtl_split_insns): Likewise.
> > (PROP_phioptearly): New define.
> > * tree-ssa-phiopt.c (tree_ssa_phiopt_worker): Set and unset
> > PROP_phioptearly on curr_properties if early.
> > ---
> > gcc/generic-match-head.c | 7 +++++
> > gcc/gimple-match-head.c | 7 +++++
> > gcc/match.pd | 76
> > ++++++++++++++++++++++++++----------------------
> > gcc/tree-pass.h | 5 ++--
> > gcc/tree-ssa-phiopt.c | 8 +++--
> > 5 files changed, 63 insertions(+), 40 deletions(-)
> >
> > diff --git a/gcc/generic-match-head.c b/gcc/generic-match-head.c
> > index f426208..90ebf84 100644
> > --- a/gcc/generic-match-head.c
> > +++ b/gcc/generic-match-head.c
> > @@ -91,6 +91,13 @@ optimize_vectors_before_lowering_p ()
> > return true;
> > }
> >
> > +/* Return true if phiopt is in early mode. */
> > +static inline bool
> > +phiopt_earlymode ()
> > +{
> > + return false;
> > +}
> > +
> > /* Return true if successive divisions can be optimized.
> > Defer to GIMPLE opts. */
> >
> > diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
> > index 7112c11..1eafbb7 100644
> > --- a/gcc/gimple-match-head.c
> > +++ b/gcc/gimple-match-head.c
> > @@ -1159,6 +1159,13 @@ canonicalize_math_after_vectorization_p ()
> > return !cfun || (cfun->curr_properties & PROP_gimple_lvec) != 0;
> > }
> >
> > +/* Return true if phiopt is in early mode. */
> > +static inline bool
> > +phiopt_earlymode ()
> > +{
> > + return !cfun || (cfun->curr_properties & PROP_phioptearly) != 0;
> > +}
> > +
> > /* Return true if we can still perform transformations that may introduce
> > vector operations that are not supported by the target. Vector lowering
> > normally handles those, but after that pass, it becomes unsafe. */
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 39fb57e..f38baf2 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -3728,39 +3728,40 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > #if GIMPLE
> > (simplify
> > (cond @0 INTEGER_CST@1 INTEGER_CST@2)
> > - (switch
> > - (if (integer_zerop (@2))
> > - (switch
> > - /* a ? 1 : 0 -> a if 0 and 1 are integral types. */
> > - (if (integer_onep (@1))
> > - (convert (convert:boolean_type_node @0)))
> > - /* a ? -1 : 0 -> -a. */
> > - (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
> > - (negate (convert (convert:boolean_type_node @0))))
> > - /* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
> > - (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@1))
> > - (with {
> > - tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
> > - }
> > - (lshift (convert (convert:boolean_type_node @0)) { shift; })))))
> > - (if (integer_zerop (@1))
> > - (with {
> > - tree booltrue = constant_boolean_node (true, boolean_type_node);
> > - }
> > + (if (!phiopt_earlymode ())
> > + (switch
> > + (if (integer_zerop (@2))
> > (switch
> > - /* a ? 0 : 1 -> !a. */
> > - (if (integer_onep (@2))
> > - (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
> > - /* a ? -1 : 0 -> -(!a). */
> > - (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> > - (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue;
> > } ))))
> > - /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> > - (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
> > + /* a ? 1 : 0 -> a if 0 and 1 are integral types. */
> > + (if (integer_onep (@1))
> > + (convert (convert:boolean_type_node @0)))
> > + /* a ? -1 : 0 -> -a. */
> > + (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
> > + (negate (convert (convert:boolean_type_node @0))))
> > + /* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
> > + (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@1))
> > (with {
> > - tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
> > + tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
> > }
> > - (lshift (convert (bit_xor (convert:boolean_type_node @0) {
> > booltrue; } ))
> > - { shift; }))))))))
> > + (lshift (convert (convert:boolean_type_node @0)) { shift; })))))
> > + (if (integer_zerop (@1))
> > + (with {
> > + tree booltrue = constant_boolean_node (true, boolean_type_node);
> > + }
> > + (switch
> > + /* a ? 0 : 1 -> !a. */
> > + (if (integer_onep (@2))
> > + (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
> > + /* a ? -1 : 0 -> -(!a). */
> > + (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> > + (negate (convert (bit_xor (convert:boolean_type_node @0) {
> > booltrue; } ))))
> > + /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> > + (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
> > + (with {
> > + tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
> > + }
> > + (lshift (convert (bit_xor (convert:boolean_type_node @0) {
> > booltrue; } ))
> > + { shift; })))))))))
> > #endif
> >
> > /* Simplification moved from fold_cond_expr_with_comparison. It may also
> > @@ -4891,7 +4892,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > /* x >= 0 ? ~y : y into ~((x >> (prec-1)) ^ y). */
> > (simplify
> > (cond (cmp @0 integer_zerop) (bit_not @1) @1)
> > - (if (INTEGRAL_TYPE_P (type)
> > + (if (!phiopt_earlymode ()
> > + && INTEGRAL_TYPE_P (type)
> > && INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > && !TYPE_UNSIGNED (TREE_TYPE (@0))
> > && TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (type))
> > @@ -4906,7 +4908,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > /* x >= 0 ? y : ~y into (x >> (prec-1)) ^ y. */
> > (simplify
> > (cond (cmp @0 integer_zerop) @1 (bit_not @1))
> > - (if (INTEGRAL_TYPE_P (type)
> > + (if (!phiopt_earlymode ()
> > + && INTEGRAL_TYPE_P (type)
> > && INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > && !TYPE_UNSIGNED (TREE_TYPE (@0))
> > && TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (type))
> > @@ -4924,7 +4927,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > (cond
> > (ne (bit_and @0 integer_pow2p@1) integer_zerop)
> > INTEGER_CST@2 integer_zerop)
> > - (if (!POINTER_TYPE_P (type) && integer_pow2p (@2))
> > + (if (!phiopt_earlymode ()
> > + && !POINTER_TYPE_P (type) && integer_pow2p (@2))
> > (with {
> > int shift = (wi::exact_log2 (wi::to_wide (@2))
> > - wi::exact_log2 (wi::to_wide (@1)));
> > @@ -4942,7 +4946,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > ncmp (ge lt)
> > (simplify
> > (cmp (bit_and (convert?@2 @0) integer_pow2p@1) integer_zerop)
> > - (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > + (if (!phiopt_earlymode ()
> > + && INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > && type_has_mode_precision_p (TREE_TYPE (@0))
> > && element_precision (@2) >= element_precision (@0)
> > && wi::only_sign_bit_p (wi::to_wide (@1), element_precision (@0)))
> > @@ -4955,7 +4960,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > (cond
> > (lt @0 integer_zerop)
> > INTEGER_CST@1 integer_zerop)
> > - (if (integer_pow2p (@1)
> > + (if (!phiopt_earlymode ()
> > + && integer_pow2p (@1)
> > && !TYPE_UNSIGNED (TREE_TYPE (@0)))
> > (with {
> > int shift = element_precision (@0) - wi::exact_log2 (wi::to_wide (@1))
> > - 1;
> > diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
> > index 15693fe..07ee151 100644
> > --- a/gcc/tree-pass.h
> > +++ b/gcc/tree-pass.h
> > @@ -222,8 +222,9 @@ protected:
> > of math functions; the
> > current choices have
> > been optimized. */
> > -#define PROP_gimple_lomp_dev (1 << 16) /* done omp_device_lower */
> > -#define PROP_rtl_split_insns (1 << 17) /* RTL has insns split. */
> > +#define PROP_phioptearly (1 << 16) /* Sets while phiopt is in
> > early mode. */
> > +#define PROP_gimple_lomp_dev (1 << 17) /* done omp_device_lower */
> > +#define PROP_rtl_split_insns (1 << 18) /* RTL has insns split. */
> >
> > #define PROP_gimple \
> > (PROP_gimple_any | PROP_gimple_lcf | PROP_gimple_leh | PROP_gimple_lomp)
> > diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> > index 3b3762a..8b289be 100644
> > --- a/gcc/tree-ssa-phiopt.c
> > +++ b/gcc/tree-ssa-phiopt.c
> > @@ -175,6 +175,8 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool
> > do_hoist_loads, bool early_p)
> > unsigned n, i;
> > bool cfgchanged = false;
> > hash_set<tree> *nontrap = 0;
> > + if (early_p)
> > + cfun->curr_properties |= PROP_phioptearly;
> >
> > calculate_dominance_info (CDI_DOMINATORS);
> >
> > @@ -345,9 +347,8 @@ 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))
> > cfgchanged = true;
> > else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
> > cfgchanged = true;
> > @@ -364,6 +365,7 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool
> > do_hoist_loads, bool early_p)
> > }
> >
> > free (bb_order);
> > + cfun->curr_properties &= ~PROP_phioptearly;
> >
> > if (do_store_elim)
> > delete nontrap;
> > --
> > 1.8.3.1
> >
