Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]

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

On Wed, Nov 23, 2022 at 8:09 AM HAO CHEN GUI via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>   There is a new insn on my target, which has a nested if_then_else and
> set -1, 0 and 1 according to a comparison.
>
>    [(set (match_operand:SI 0 "gpc_reg_operand" "=r")
>          (if_then_else:SI (lt (match_operand:CC 1 "cc_reg_operand" "y")
>                               (const_int 0))
>                           (const_int -1)
>                           (if_then_else (gt (match_dup 1)
>                                             (const_int 0))
>                                         (const_int 1)
>                                         (const_int 0))))]
>
>   In ifcvt pass, it probably contains a comparison, a branch, a setcc
> and a constant set.
>
>     8: r122:CC=cmp(r120:DI#0,r121:DI#0)
>     9: pc={(r122:CC<0)?L29:pc}
>
>    14: r118:SI=r122:CC>0
>
>    29: L29:
>     5: r118:SI=0xffffffffffffffff
>
>   This patch adds the new conversion into ifcvt and convert this kind of
> branch into a nested if-then-else insn if the target supports such
> pattern.
>
>   HAVE_ternary_conditional_set indicates if the target has such nested
> if-then-else insn. It's set in genconfig. noce_try_ternary_cset will be
> executed to detect suitable pattern and convert it to the nested
> if-then-else insn if HAVE_ternary_conditional_set is set. The hook
> TARGET_NOCE_TERNARY_CSET_P detects target specific pattern and output
> conditions and setting integers for the nested if-then-else.
>
>   Bootstrapped and tested on powerpc64-linux BE/LE and x86 with no
> regressions. Is this okay for trunk? Any recommendations? Thanks a lot.

Wouldn't we usually either add an optab or try to recog a canonical
RTL form instead of adding a new target hook for things like this?

> ChangeLog
> 2022-11-23  Haochen Gui <guihaoc@linux.ibm.com>
>
> gcc/
>         * doc/tm.texi: Regenerate.
>         * doc/tm.texi.in (TARGET_NOCE_TERNARY_CSET_P): Document new hook.
>         * genconfig.cc (have_ternary_cset_flag): New.
>         (walk_insn_part): Detect nested if-then-else with const_int setting
>         and set have_ternary_cset_flag.
>         (HAVE_ternary_conditional_set): Define.
>         * ifcvt.cc (noce_emit_ternary_cset): New function to emit nested
>         if-then-else insns.
>         (noce_try_ternary_cset): Detect ternary conditional set and emit the
>         insn.
>         (noce_process_if_block): Try to do ternary condition set convertion
>         when a target supports ternary conditional set insn.
>         * target.def (noce_ternary_cset_p): New hook.
>         * targhooks.cc (default_noce_ternary_cset_p): New function.
>         * targhooks.h (default_noce_ternary_cset_p): New declare.
>
>
> patch.diff
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 92bda1a7e14..9823eccbe68 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -7094,6 +7094,15 @@ the @code{POLY_VALUE_MIN}, @code{POLY_VALUE_MAX} and
>  implementation returns the lowest possible value of @var{val}.
>  @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_NOCE_TERNARY_CSET_P (struct noce_if_info *@var{if_info}, rtx *@var{outer_cond}, rtx *@var{inner_cond}, int *@var{int1}, int *@var{int2}, int *@var{int3})
> +This hook returns true if the if-then-else-join blocks describled in
> +@code{if_info} can be converted to a ternary conditional set implemented by
> +a nested if-then-else insn.  The @code{int1}, @code{int2} and @code{int3}
> +are three possible results of the nested if-then-else insn.
> +@code{outer_cond} and @code{inner_cond} are the conditions for outer and
> +if-then-else.
> +@end deftypefn
> +
>  @node Scheduling
>  @section Adjusting the Instruction Scheduler
>
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 112462310b1..1d6f28cc50a 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -4631,6 +4631,8 @@ Define this macro if a non-short-circuit operation produced by
>
>  @hook TARGET_ESTIMATED_POLY_VALUE
>
> +@hook TARGET_NOCE_TERNARY_CSET_P
> +
>  @node Scheduling
>  @section Adjusting the Instruction Scheduler
>
> diff --git a/gcc/genconfig.cc b/gcc/genconfig.cc
> index b7c6b48eec6..902c832cf5a 100644
> --- a/gcc/genconfig.cc
> +++ b/gcc/genconfig.cc
> @@ -33,6 +33,7 @@ static int max_recog_operands;  /* Largest operand number seen.  */
>  static int max_dup_operands;    /* Largest number of match_dup in any insn.  */
>  static int max_clobbers_per_insn;
>  static int have_cmove_flag;
> +static int have_ternary_cset_flag;
>  static int have_cond_exec_flag;
>  static int have_lo_sum_flag;
>  static int have_rotate_flag;
> @@ -136,6 +137,12 @@ walk_insn_part (rtx part, int recog_p, int non_pc_set_src)
>           && GET_CODE (XEXP (part, 1)) == MATCH_OPERAND
>           && GET_CODE (XEXP (part, 2)) == MATCH_OPERAND)
>         have_cmove_flag = 1;
> +      else if (recog_p && non_pc_set_src
> +              && GET_CODE (XEXP (part, 1)) == CONST_INT
> +              && GET_CODE (XEXP (part, 2)) == IF_THEN_ELSE
> +              && GET_CODE (XEXP (XEXP (part, 2), 1)) == CONST_INT
> +              && GET_CODE (XEXP (XEXP (part, 2), 2)) == CONST_INT)
> +       have_ternary_cset_flag = 1;
>        break;
>
>      case COND_EXEC:
> @@ -328,6 +335,11 @@ main (int argc, const char **argv)
>    else
>      printf ("#define HAVE_conditional_move 0\n");
>
> +  if (have_ternary_cset_flag)
> +    printf ("#define HAVE_ternary_conditional_set 1\n");
> +  else
> +    printf ("#define HAVE_ternary_conditional_set 0\n");
> +
>    if (have_cond_exec_flag)
>      printf ("#define HAVE_conditional_execution 1\n");
>    else
> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
> index eb8efb89a89..774dff21719 100644
> --- a/gcc/ifcvt.cc
> +++ b/gcc/ifcvt.cc
> @@ -1830,6 +1830,42 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code,
>      return NULL_RTX;
>  }
>
> +/*  Emit a nested if-then-else insn.  */
> +
> +static rtx
> +noce_emit_ternary_cset (rtx x, rtx outer_cond, rtx inner_cond,
> +                       int a, int b, int c)
> +{
> +  rtx target;
> +  machine_mode mode;
> +  machine_mode orig_mode = GET_MODE (x);
> +
> +  FOR_EACH_MODE_FROM (mode, orig_mode)
> +    {
> +      rtx inner_if_then_else = gen_rtx_IF_THEN_ELSE (mode, inner_cond,
> +                                                    GEN_INT (b), GEN_INT (c));
> +      rtx outer_if_then_else = gen_rtx_IF_THEN_ELSE (mode, outer_cond,
> +                                                    GEN_INT (a),
> +                                                    inner_if_then_else);
> +      target = (mode == orig_mode) ? x : gen_reg_rtx (mode);
> +      rtx set = gen_rtx_SET (target, outer_if_then_else);
> +      start_sequence ();
> +      rtx_insn *insn = emit_insn (set);
> +
> +      if (recog_memoized (insn) >= 0)
> +       {
> +         rtx_insn *seq = get_insns ();
> +         end_sequence ();
> +         emit_insn (seq);
> +
> +         return target;
> +       }
> +    }
> +
> +  end_sequence ();
> +  return NULL_RTX;
> +}
> +
>  /* Try only simple constants and registers here.  More complex cases
>     are handled in noce_try_cmove_arith after noce_try_store_flag_arith
>     has had a go at it.  */
> @@ -2987,6 +3023,43 @@ noce_try_bitop (struct noce_if_info *if_info)
>    return TRUE;
>  }
>
> +/* Try to find pattern "a < b ? -1 : (a > b ? 1 : 0);" and convert it to
> +   a nested if-then-else insn.  */
> +
> +static int
> +noce_try_ternary_cset (struct noce_if_info *if_info)
> +{
> +  rtx outer_cond = NULL_RTX, inner_cond = NULL_RTX;
> +  int int1 = 0, int2 = 0, int3 = 0;
> +
> +  if (targetm.noce_ternary_cset_p (if_info, &outer_cond, &inner_cond,
> +                                  &int1, &int2, &int3))
> +    {
> +      start_sequence ();
> +      rtx target = noce_emit_ternary_cset (if_info->x, outer_cond, inner_cond,
> +                                          int1, int2, int3);
> +      if (target)
> +       {
> +         rtx_insn *ifcvt_seq;
> +
> +         if (target != if_info->x)
> +           noce_emit_move_insn (if_info->x, target);
> +
> +         ifcvt_seq = end_ifcvt_sequence (if_info);
> +         if (!ifcvt_seq)
> +           return false;
> +
> +         emit_insn_before_setloc (ifcvt_seq, if_info->jump,
> +                                  INSN_LOCATION (if_info->insn_a));
> +         if_info->transform_name = "noce_try_ternary_cset";
> +         return true;
> +       }
> +
> +      end_sequence ();
> +    }
> +
> +  return false;
> +}
>
>  /* Similar to get_condition, only the resulting condition must be
>     valid at JUMP, instead of at EARLIEST.
> @@ -3963,6 +4036,9 @@ noce_process_if_block (struct noce_if_info *if_info)
>    if (HAVE_conditional_move
>        && noce_try_cmove (if_info))
>      goto success;
> +  if (HAVE_ternary_conditional_set
> +      && noce_try_ternary_cset (if_info))
> +    goto success;
>    if (! targetm.have_conditional_execution ())
>      {
>        if (noce_try_addcc (if_info))
> diff --git a/gcc/target.def b/gcc/target.def
> index 2a7fa68f83d..1b983bf852e 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -3907,6 +3907,20 @@ candidate as a replacement for the if-convertible sequence described in\n\
>  bool, (rtx_insn *seq, struct noce_if_info *if_info),
>  default_noce_conversion_profitable_p)
>
> +/* Return true if the if-then-else-join blocks can be converted to a nested\n\
> +if-then-else insn.  */
> +DEFHOOK
> +(noce_ternary_cset_p,
> + "This hook returns true if the if-then-else-join blocks describled in\n\
> +@code{if_info} can be converted to a ternary conditional set implemented by\n\
> +a nested if-then-else insn.  The @code{int1}, @code{int2} and @code{int3}\n\
> +are three possible results of the nested if-then-else insn.\n\
> +@code{outer_cond} and @code{inner_cond} are the conditions for outer and\n\
> +if-then-else.",
> +bool, (struct noce_if_info *if_info, rtx *outer_cond, rtx *inner_cond,
> +       int *int1, int *int2, int *int3),
> +default_noce_ternary_cset_p)
> +
>  /* Return true if new_addr should be preferred over the existing address used by
>     memref in insn.  */
>  DEFHOOK
> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> index b15ae19bcb6..179324e3f0b 100644
> --- a/gcc/targhooks.cc
> +++ b/gcc/targhooks.cc
> @@ -2673,4 +2673,17 @@ default_gcov_type_size (void)
>    return TYPE_PRECISION (long_long_integer_type_node) > 32 ? 64 : 32;
>  }
>
> +/* The default implementation of TARGET_NOCE_TERNARY_CSET_P.  */
> +
> +bool
> +default_noce_ternary_cset_p (struct noce_if_info *if_info ATTRIBUTE_UNUSED,
> +                            rtx *outer_cond ATTRIBUTE_UNUSED,
> +                            rtx *inner_cond ATTRIBUTE_UNUSED,
> +                            int *int1 ATTRIBUTE_UNUSED,
> +                            int *int2 ATTRIBUTE_UNUSED,
> +                            int *int3 ATTRIBUTE_UNUSED)
> +{
> +  return false;
> +}
> +
>  #include "gt-targhooks.h"
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index ecce55ebe79..a58bc8ef6a6 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -296,5 +296,7 @@ extern rtx default_memtag_extract_tag (rtx, rtx);
>  extern rtx default_memtag_untagged_pointer (rtx, rtx);
>
>  extern HOST_WIDE_INT default_gcov_type_size (void);
> +extern bool default_noce_ternary_cset_p (struct noce_if_info *, rtx *, rtx *,
> +                                        int *, int *, int *);
>
>  #endif /* GCC_TARGHOOKS_H */