Re: [PATCH] PHIOPT: Move two_value_replacement to match.pd

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Apr 28, 2023 at 5:31 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This patch converts two_value_replacement function
> into a match.pd pattern.
> It is a direct translation with only one minor change,
> does not check for the {0,+-1} case as that is handled
> before in match.pd so there is no reason to do the extra
> check for it.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with
> no regressions.

OK.

Thanks,
Richard.

> gcc/ChangeLog:
>
>         PR tree-optimization/100958
>         * tree-ssa-phiopt.cc (two_value_replacement): Remove.
>         (pass_phiopt::execute): Don't call two_value_replacement.
>         * match.pd (a !=/== CST1 ? CST2 : CST3): Add pattern to
>         handle what two_value_replacement did.
> ---
>  gcc/match.pd           |  94 ++++++++++++++++++++++++
>  gcc/tree-ssa-phiopt.cc | 157 +----------------------------------------
>  2 files changed, 96 insertions(+), 155 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 31fe5093218..e17597ead26 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4632,6 +4632,100 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    )
>   )
>  )
> +
> +/* Optimize
> +   # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
> +   x_5 ? cstN ? cst4 : cst3
> +   # op is == or != and N is 1 or 2
> +   to r_6 = x_5 + (min (cst3, cst4) - cst1) or
> +   r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
> +   of cst3 and cst4 is smaller.
> +   This was originally done by two_value_replacement in phiopt (PR 88676).  */
> +(for eqne (ne eq)
> + (simplify
> +  (cond (eqne SSA_NAME@0 INTEGER_CST@1) INTEGER_CST@2 INTEGER_CST@3)
> +  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +       && INTEGRAL_TYPE_P (type)
> +       && (wi::to_widest (@2) + 1 == wi::to_widest (@3)
> +           || wi::to_widest (@2) == wi::to_widest (@3) + 1))
> +   (with {
> +     value_range r;
> +     get_range_query (cfun)->range_of_expr (r, @0);
> +     if (r.undefined_p ())
> +       r.set_varying (TREE_TYPE (@0));
> +
> +     wide_int min = r.lower_bound ();
> +     wide_int max = r.upper_bound ();
> +    }
> +    (if (min + 1 == max
> +        && (wi::to_wide (@1) == min
> +            || wi::to_wide (@1) == max))
> +     (with {
> +       tree arg0 = @2, arg1 = @3;
> +       tree type1;
> +       if ((eqne == EQ_EXPR) ^ (wi::to_wide (@1) == min))
> +         std::swap (arg0, arg1);
> +       if (TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (type))
> +        {
> +          /* Avoid performing the arithmetics in bool type which has different
> +             semantics, otherwise prefer unsigned types from the two with
> +            the same precision.  */
> +          if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE
> +              || !TYPE_UNSIGNED (type))
> +            type1 = TREE_TYPE (@0);
> +          else
> +            type1 = TREE_TYPE (arg0);
> +        }
> +       else if (TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (type))
> +        type1 = TREE_TYPE (@0);
> +       else
> +        type1 = type;
> +       min = wide_int::from (min, TYPE_PRECISION (type1),
> +                            TYPE_SIGN (TREE_TYPE (@0)));
> +       wide_int a = wide_int::from (wi::to_wide (arg0), TYPE_PRECISION (type1),
> +                                   TYPE_SIGN (type));
> +       enum tree_code code;
> +       wi::overflow_type ovf;
> +       if (tree_int_cst_lt (arg0, arg1))
> +        {
> +          code = PLUS_EXPR;
> +          a -= min;
> +          if (!TYPE_UNSIGNED (type1))
> +            {
> +              /* lhs is known to be in range [min, min+1] and we want to add a
> +                 to it.  Check if that operation can overflow for those 2 values
> +                 and if yes, force unsigned type.  */
> +              wi::add (min + (wi::neg_p (a) ? 0 : 1), a, SIGNED, &ovf);
> +              if (ovf)
> +                type1 = unsigned_type_for (type1);
> +            }
> +        }
> +       else
> +        {
> +          code = MINUS_EXPR;
> +          a += min;
> +          if (!TYPE_UNSIGNED (type1))
> +            {
> +              /* lhs is known to be in range [min, min+1] and we want to subtract
> +                 it from a.  Check if that operation can overflow for those 2
> +                 values and if yes, force unsigned type.  */
> +              wi::sub (a, min + (wi::neg_p (min) ? 0 : 1), SIGNED, &ovf);
> +              if (ovf)
> +               type1 = unsigned_type_for (type1);
> +            }
> +        }
> +       tree arg = wide_int_to_tree (type1, a);
> +      }
> +      (if (code == PLUS_EXPR)
> +       (convert (plus (convert:type1 @0) { arg; }))
> +       (convert (minus { arg; } (convert:type1 @0)))
> +      )
> +     )
> +    )
> +   )
> +  )
> + )
> +)
>  #endif
>
>  (simplify
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index 7fc6ac17b4a..4b43f1abdbc 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -373,155 +373,6 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi *phi,
>    return newphi;
>  }
>
> -/* Optimize
> -   # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
> -   if (x_5 op cstN) # where op is == or != and N is 1 or 2
> -     goto bb3;
> -   else
> -     goto bb4;
> -   bb3:
> -   bb4:
> -   # r_6 = PHI<cst3(2), cst4(3)> # where cst3 == cst4 + 1 or cst4 == cst3 + 1
> -
> -   to r_6 = x_5 + (min (cst3, cst4) - cst1) or
> -   r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
> -   of cst3 and cst4 is smaller.  */
> -
> -static bool
> -two_value_replacement (basic_block cond_bb, basic_block middle_bb,
> -                      edge e1, gphi *phi, tree arg0, tree arg1)
> -{
> -  /* Only look for adjacent integer constants.  */
> -  if (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
> -      || !INTEGRAL_TYPE_P (TREE_TYPE (arg1))
> -      || TREE_CODE (arg0) != INTEGER_CST
> -      || TREE_CODE (arg1) != INTEGER_CST
> -      || (tree_int_cst_lt (arg0, arg1)
> -         ? wi::to_widest (arg0) + 1 != wi::to_widest (arg1)
> -         : wi::to_widest (arg1) + 1 != wi::to_widest (arg0)))
> -    return false;
> -
> -  if (!empty_block_p (middle_bb))
> -    return false;
> -
> -  gcond *stmt = as_a <gcond *> (*gsi_last_bb (cond_bb));
> -  tree lhs = gimple_cond_lhs (stmt);
> -  tree rhs = gimple_cond_rhs (stmt);
> -
> -  if (TREE_CODE (lhs) != SSA_NAME
> -      || !INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> -      || TREE_CODE (rhs) != INTEGER_CST)
> -    return false;
> -
> -  switch (gimple_cond_code (stmt))
> -    {
> -    case EQ_EXPR:
> -    case NE_EXPR:
> -      break;
> -    default:
> -      return false;
> -    }
> -
> -  /* Defer boolean x ? 0 : {1,-1} or x ? {1,-1} : 0 to
> -     match_simplify_replacement.  */
> -  if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE
> -      && (integer_zerop (arg0)
> -         || integer_zerop (arg1)
> -         || TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE
> -         || (TYPE_PRECISION (TREE_TYPE (arg0))
> -             <= TYPE_PRECISION (TREE_TYPE (lhs)))))
> -    return false;
> -
> -  value_range r;
> -  get_range_query (cfun)->range_of_expr (r, lhs);
> -  if (r.undefined_p ())
> -    r.set_varying (TREE_TYPE (lhs));
> -  wide_int min = r.lower_bound ();
> -  wide_int max = r.upper_bound ();
> -
> -  if (min + 1 != max
> -      || (wi::to_wide (rhs) != min
> -         && wi::to_wide (rhs) != max))
> -    return false;
> -
> -  /* 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.  */
> -  edge true_edge, false_edge;
> -  extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
> -  if ((gimple_cond_code (stmt) == EQ_EXPR)
> -      ^ (wi::to_wide (rhs) == max)
> -      ^ (e1 == false_edge))
> -    std::swap (arg0, arg1);
> -
> -  tree type;
> -  if (TYPE_PRECISION (TREE_TYPE (lhs)) == TYPE_PRECISION (TREE_TYPE (arg0)))
> -    {
> -      /* Avoid performing the arithmetics in bool type which has different
> -        semantics, otherwise prefer unsigned types from the two with
> -        the same precision.  */
> -      if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE
> -         || !TYPE_UNSIGNED (TREE_TYPE (arg0)))
> -       type = TREE_TYPE (lhs);
> -      else
> -       type = TREE_TYPE (arg0);
> -    }
> -  else if (TYPE_PRECISION (TREE_TYPE (lhs)) > TYPE_PRECISION (TREE_TYPE (arg0)))
> -    type = TREE_TYPE (lhs);
> -  else
> -    type = TREE_TYPE (arg0);
> -
> -  min = wide_int::from (min, TYPE_PRECISION (type),
> -                       TYPE_SIGN (TREE_TYPE (lhs)));
> -  wide_int a = wide_int::from (wi::to_wide (arg0), TYPE_PRECISION (type),
> -                              TYPE_SIGN (TREE_TYPE (arg0)));
> -  enum tree_code code;
> -  wi::overflow_type ovf;
> -  if (tree_int_cst_lt (arg0, arg1))
> -    {
> -      code = PLUS_EXPR;
> -      a -= min;
> -      if (!TYPE_UNSIGNED (type))
> -       {
> -         /* lhs is known to be in range [min, min+1] and we want to add a
> -            to it.  Check if that operation can overflow for those 2 values
> -            and if yes, force unsigned type.  */
> -         wi::add (min + (wi::neg_p (a) ? 0 : 1), a, SIGNED, &ovf);
> -         if (ovf)
> -           type = unsigned_type_for (type);
> -       }
> -    }
> -  else
> -    {
> -      code = MINUS_EXPR;
> -      a += min;
> -      if (!TYPE_UNSIGNED (type))
> -       {
> -         /* lhs is known to be in range [min, min+1] and we want to subtract
> -            it from a.  Check if that operation can overflow for those 2
> -            values and if yes, force unsigned type.  */
> -         wi::sub (a, min + (wi::neg_p (min) ? 0 : 1), SIGNED, &ovf);
> -         if (ovf)
> -           type = unsigned_type_for (type);
> -       }
> -    }
> -
> -  tree arg = wide_int_to_tree (type, a);
> -  gimple_seq stmts = NULL;
> -  lhs = gimple_convert (&stmts, type, lhs);
> -  tree new_rhs;
> -  if (code == PLUS_EXPR)
> -    new_rhs = gimple_build (&stmts, PLUS_EXPR, type, lhs, arg);
> -  else
> -    new_rhs = gimple_build (&stmts, MINUS_EXPR, type, arg, lhs);
> -  new_rhs = gimple_convert (&stmts, TREE_TYPE (arg0), new_rhs);
> -  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> -  gsi_insert_seq_before (&gsi, stmts, GSI_SAME_STMT);
> -
> -  replace_phi_edge_with_variable (cond_bb, e1, phi, new_rhs);
> -
> -  /* Note that we optimized this PHI.  */
> -  return true;
> -}
>
>  /* Return TRUE if SEQ/OP pair should be allowed during early phiopt.
>     Currently this is to allow MIN/MAX and ABS/NEGATE and constants.  */
> @@ -4170,12 +4021,8 @@ pass_phiopt::execute (function *)
>         }
>
>        /* Do the replacement of conditional if it can be done.  */
> -      if (!early_p
> -         && !diamond_p
> -         && two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
> -       cfgchanged = true;
> -      else if (match_simplify_replacement (bb, bb1, bb2, e1, e2, phi,
> -                                          arg0, arg1, early_p, diamond_p))
> +      if (match_simplify_replacement (bb, bb1, bb2, e1, e2, phi,
> +                                     arg0, arg1, early_p, diamond_p))
>         cfgchanged = true;
>        else if (!early_p
>                && !diamond_p
> --
> 2.31.1
>