public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Pinski <pinskia@gmail.com>
To: Robin Dapp <rdapp.gcc@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
	Richard Biener <rguenther@suse.de>
Subject: Re: [PATCH] ifcvt/vect: Emit COND_ADD for conditional scalar reduction.
Date: Thu, 2 Nov 2023 16:26:15 -0700	[thread overview]
Message-ID: <CA+=Sn1n9gB6JvjSNRQxatUz1ANK+rTXaJ9WAqa+=YvWVjvEv3Q@mail.gmail.com> (raw)
In-Reply-To: <0193b63e-98dc-42bc-cd33-485361ea50bf@gmail.com>

On Wed, Sep 20, 2023 at 6:52 AM Robin Dapp <rdapp.gcc@gmail.com> wrote:
>
> Hi,
>
> as described in PR111401 we currently emit a COND and a PLUS expression
> for conditional reductions.  This makes it difficult to combine both
> into a masked reduction statement later.
> This patch improves that by directly emitting a COND_ADD during ifcvt and
> adjusting some vectorizer code to handle it.
>
> It also makes neutral_op_for_reduction return -0 if HONOR_SIGNED_ZEROS
> is true.
>
> Related question/change: We only allow PLUS_EXPR in fold_left_reduction_fn
> but have code to handle MINUS_EXPR in vectorize_fold_left_reduction.  I
> suppose that's intentional but it "just works" on riscv and the testsuite
> doesn't change when allowing MINUS_EXPR so I went ahead and did that.
>
> Bootstrapped and regtested on x86 and aarch64.

This caused gcc.target/i386/avx512f-reduce-op-1.c testcase to start to
fail when testing on a x86_64 that has avx512f (In my case I am using
`Intel(R) Xeon(R) D-2166NT CPU @ 2.00GHz`).  I reverted the commit to
double check it too.

The difference in optimized I see is:
  if (_40 != 3.5e+1) // working
vs
  if (_40 != 6.4e+1) // not working

It is test_epi32_ps which is failing with TEST_PS macro and the plus
operand that uses TESTOP:
    TESTOP (add, +, float, ps, 0.0f);                                   \

I have not reduced the testcase any further though.

Thanks,
Andrew Pinski


>
> Regards
>  Robin
>
> gcc/ChangeLog:
>
>         PR middle-end/111401
>         * internal-fn.cc (cond_fn_p): New function.
>         * internal-fn.h (cond_fn_p): Define.
>         * tree-if-conv.cc (convert_scalar_cond_reduction): Emit COND_ADD
>         if supported.
>         (predicate_scalar_phi): Add whitespace.
>         * tree-vect-loop.cc (fold_left_reduction_fn): Add IFN_COND_ADD.
>         (neutral_op_for_reduction): Return -0 for PLUS.
>         (vect_is_simple_reduction): Don't count else operand in
>         COND_ADD.
>         (vectorize_fold_left_reduction): Add COND_ADD handling.
>         (vectorizable_reduction): Don't count else operand in COND_ADD.
>         (vect_transform_reduction): Add COND_ADD handling.
>         * tree-vectorizer.h (neutral_op_for_reduction): Add default
>         parameter.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c: New test.
>         * gcc.target/riscv/rvv/autovec/cond/pr111401.c: New test.
> ---
>  gcc/internal-fn.cc                            |  38 +++++
>  gcc/internal-fn.h                             |   1 +
>  .../vect-cond-reduc-in-order-2-signed-zero.c  | 141 ++++++++++++++++++
>  .../riscv/rvv/autovec/cond/pr111401.c         |  61 ++++++++
>  gcc/tree-if-conv.cc                           |  63 ++++++--
>  gcc/tree-vect-loop.cc                         | 130 ++++++++++++----
>  gcc/tree-vectorizer.h                         |   2 +-
>  7 files changed, 394 insertions(+), 42 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/cond/pr111401.c
>
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 0fd34359247..77939890f5a 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4241,6 +4241,44 @@ first_commutative_argument (internal_fn fn)
>      }
>  }
>
> +/* Return true if this CODE describes a conditional (masked) internal_fn.  */
> +
> +bool
> +cond_fn_p (code_helper code)
> +{
> +  if (!code.is_fn_code ())
> +    return false;
> +
> +  if (!internal_fn_p ((combined_fn) code))
> +    return false;
> +
> +  internal_fn fn = as_internal_fn ((combined_fn) code);
> +  switch (fn)
> +    {
> +    #undef DEF_INTERNAL_COND_FN
> +    #define DEF_INTERNAL_COND_FN(NAME, F, O, T)                          \
> +    case IFN_COND_##NAME:                                        \
> +    case IFN_COND_LEN_##NAME:                                    \
> +      return true;
> +    #include "internal-fn.def"
> +    #undef DEF_INTERNAL_COND_FN
> +
> +    #undef DEF_INTERNAL_SIGNED_COND_FN
> +    #define DEF_INTERNAL_SIGNED_COND_FN(NAME, F, S, SO, UO, T)   \
> +    case IFN_COND_##NAME:                                        \
> +    case IFN_COND_LEN_##NAME:                                    \
> +      return true;
> +    #include "internal-fn.def"
> +    #undef DEF_INTERNAL_SIGNED_COND_FN
> +
> +    default:
> +      return false;
> +    }
> +
> +  return false;
> +}
> +
> +
>  /* Return true if this CODE describes an internal_fn that returns a vector with
>     elements twice as wide as the element size of the input vectors.  */
>
> diff --git a/gcc/internal-fn.h b/gcc/internal-fn.h
> index 99de13a0199..f1cc9db29c0 100644
> --- a/gcc/internal-fn.h
> +++ b/gcc/internal-fn.h
> @@ -219,6 +219,7 @@ extern bool commutative_ternary_fn_p (internal_fn);
>  extern int first_commutative_argument (internal_fn);
>  extern bool associative_binary_fn_p (internal_fn);
>  extern bool widening_fn_p (code_helper);
> +extern bool cond_fn_p (code_helper code);
>
>  extern bool set_edom_supported_p (void);
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c b/gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c
> new file mode 100644
> index 00000000000..57c600838ee
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-cond-reduc-in-order-2-signed-zero.c
> @@ -0,0 +1,141 @@
> +/* Make sure a -0 stays -0 when we perform a conditional reduction.  */
> +/* { dg-do run } */
> +/* { dg-require-effective-target vect_double } */
> +/* { dg-add-options ieee } */
> +/* { dg-additional-options "-std=c99 -fno-fast-math" } */
> +
> +#include "tree-vect.h"
> +
> +#include <math.h>
> +
> +#define N (VECTOR_BITS * 17)
> +
> +double __attribute__ ((noinline, noclone))
> +reduc_plus_double (double *restrict a, double init, int *cond, int n)
> +{
> +  double res = init;
> +  for (int i = 0; i < n; i++)
> +    if (cond[i])
> +      res += a[i];
> +  return res;
> +}
> +
> +double __attribute__ ((noinline, noclone, optimize ("0")))
> +reduc_plus_double_ref (double *restrict a, double init, int *cond, int n)
> +{
> +  double res = init;
> +  for (int i = 0; i < n; i++)
> +    if (cond[i])
> +      res += a[i];
> +  return res;
> +}
> +
> +double __attribute__ ((noinline, noclone))
> +reduc_minus_double (double *restrict a, double init, int *cond, int n)
> +{
> +  double res = init;
> +  for (int i = 0; i < n; i++)
> +    if (cond[i])
> +      res -= a[i];
> +  return res;
> +}
> +
> +double __attribute__ ((noinline, noclone, optimize ("0")))
> +reduc_minus_double_ref (double *restrict a, double init, int *cond, int n)
> +{
> +  double res = init;
> +  for (int i = 0; i < n; i++)
> +    if (cond[i])
> +      res -= a[i];
> +  return res;
> +}
> +
> +int __attribute__ ((optimize (1)))
> +main ()
> +{
> +  int n = 19;
> +  double a[N];
> +  int cond1[N], cond2[N];
> +
> +  for (int i = 0; i < N; i++)
> +    {
> +      a[i] = (i * 0.1) * (i & 1 ? 1 : -1);
> +      cond1[i] = 0;
> +      cond2[i] = i & 4 ? 1 : 0;
> +      asm volatile ("" ::: "memory");
> +    }
> +
> +  double res1 = reduc_plus_double (a, -0.0, cond1, n);
> +  double ref1 = reduc_plus_double_ref (a, -0.0, cond1, n);
> +  double res2 = reduc_minus_double (a, -0.0, cond1, n);
> +  double ref2 = reduc_minus_double_ref (a, -0.0, cond1, n);
> +  double res3 = reduc_plus_double (a, -0.0, cond1, n);
> +  double ref3 = reduc_plus_double_ref (a, -0.0, cond1, n);
> +  double res4 = reduc_minus_double (a, -0.0, cond1, n);
> +  double ref4 = reduc_minus_double_ref (a, -0.0, cond1, n);
> +
> +  if (res1 != ref1 || signbit (res1) != signbit (ref1))
> +    __builtin_abort ();
> +  if (res2 != ref2 || signbit (res2) != signbit (ref2))
> +    __builtin_abort ();
> +  if (res3 != ref3 || signbit (res3) != signbit (ref3))
> +    __builtin_abort ();
> +  if (res4 != ref4 || signbit (res4) != signbit (ref4))
> +    __builtin_abort ();
> +
> +  res1 = reduc_plus_double (a, 0.0, cond1, n);
> +  ref1 = reduc_plus_double_ref (a, 0.0, cond1, n);
> +  res2 = reduc_minus_double (a, 0.0, cond1, n);
> +  ref2 = reduc_minus_double_ref (a, 0.0, cond1, n);
> +  res3 = reduc_plus_double (a, 0.0, cond1, n);
> +  ref3 = reduc_plus_double_ref (a, 0.0, cond1, n);
> +  res4 = reduc_minus_double (a, 0.0, cond1, n);
> +  ref4 = reduc_minus_double_ref (a, 0.0, cond1, n);
> +
> +  if (res1 != ref1 || signbit (res1) != signbit (ref1))
> +    __builtin_abort ();
> +  if (res2 != ref2 || signbit (res2) != signbit (ref2))
> +    __builtin_abort ();
> +  if (res3 != ref3 || signbit (res3) != signbit (ref3))
> +    __builtin_abort ();
> +  if (res4 != ref4 || signbit (res4) != signbit (ref4))
> +    __builtin_abort ();
> +
> +  res1 = reduc_plus_double (a, -0.0, cond2, n);
> +  ref1 = reduc_plus_double_ref (a, -0.0, cond2, n);
> +  res2 = reduc_minus_double (a, -0.0, cond2, n);
> +  ref2 = reduc_minus_double_ref (a, -0.0, cond2, n);
> +  res3 = reduc_plus_double (a, -0.0, cond2, n);
> +  ref3 = reduc_plus_double_ref (a, -0.0, cond2, n);
> +  res4 = reduc_minus_double (a, -0.0, cond2, n);
> +  ref4 = reduc_minus_double_ref (a, -0.0, cond2, n);
> +
> +  if (res1 != ref1 || signbit (res1) != signbit (ref1))
> +    __builtin_abort ();
> +  if (res2 != ref2 || signbit (res2) != signbit (ref2))
> +    __builtin_abort ();
> +  if (res3 != ref3 || signbit (res3) != signbit (ref3))
> +    __builtin_abort ();
> +  if (res4 != ref4 || signbit (res4) != signbit (ref4))
> +    __builtin_abort ();
> +
> +  res1 = reduc_plus_double (a, 0.0, cond2, n);
> +  ref1 = reduc_plus_double_ref (a, 0.0, cond2, n);
> +  res2 = reduc_minus_double (a, 0.0, cond2, n);
> +  ref2 = reduc_minus_double_ref (a, 0.0, cond2, n);
> +  res3 = reduc_plus_double (a, 0.0, cond2, n);
> +  ref3 = reduc_plus_double_ref (a, 0.0, cond2, n);
> +  res4 = reduc_minus_double (a, 0.0, cond2, n);
> +  ref4 = reduc_minus_double_ref (a, 0.0, cond2, n);
> +
> +  if (res1 != ref1 || signbit (res1) != signbit (ref1))
> +    __builtin_abort ();
> +  if (res2 != ref2 || signbit (res2) != signbit (ref2))
> +    __builtin_abort ();
> +  if (res3 != ref3 || signbit (res3) != signbit (ref3))
> +    __builtin_abort ();
> +  if (res4 != ref4 || signbit (res4) != signbit (ref4))
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/cond/pr111401.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/cond/pr111401.c
> new file mode 100644
> index 00000000000..1d559ce5391
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/cond/pr111401.c
> @@ -0,0 +1,61 @@
> +/* { dg-do run { target { riscv_vector } } } */
> +/* { dg-additional-options "-march=rv64gcv -mabi=lp64d --param riscv-autovec-preference=scalable -fdump-tree-vect-details" } */
> +
> +double
> +__attribute__ ((noipa))
> +foo2 (double *__restrict a, double init, int *__restrict cond, int n)
> +{
> +  for (int i = 0; i < n; i++)
> +    if (cond[i])
> +      init += a[i];
> +  return init;
> +}
> +
> +double
> +__attribute__ ((noipa))
> +foo3 (double *__restrict a, double init, int *__restrict cond, int n)
> +{
> +  for (int i = 0; i < n; i++)
> +    if (cond[i])
> +      init -= a[i];
> +  return init;
> +}
> +
> +#define SZ 125
> +
> +__attribute__ ((optimize ("1")))
> +int
> +main ()
> +{
> +  double res1 = 0, res2 = 0;
> +  double a1[SZ], a2[SZ];
> +  int c1[SZ], c2[SZ];
> +  for (int i = 0; i < SZ; i++)
> +    {
> +      a1[i] = i * 3 + (i & 4) - (i & 7);
> +      a2[i] = i * 3 + (i & 4) - (i & 7);
> +      c1[i] = i & 1;
> +      c2[i] = i & 1;
> +    }
> +
> +  double init1 = 2.7, init2 = 8.2;
> +  double ref1 = init1, ref2 = init2;
> +  for (int i = 0; i < SZ; i++)
> +    {
> +      if (c1[i])
> +        ref1 += a1[i];
> +      if (c2[i])
> +        ref2 -= a2[i];
> +    }
> +
> +  res1 = foo2 (a1, init1, c1, SZ);
> +  res2 = foo3 (a2, init2, c2, SZ);
> +
> +  if (res1 != ref1)
> +    __builtin_abort ();
> +  if (res2 != ref2)
> +    __builtin_abort ();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loop" 2 "vect" } } */
> +/* { dg-final { scan-tree-dump-not "VCOND_MASK" "vect" } } */
> diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
> index 799f071965e..425976b0861 100644
> --- a/gcc/tree-if-conv.cc
> +++ b/gcc/tree-if-conv.cc
> @@ -1852,10 +1852,12 @@ convert_scalar_cond_reduction (gimple *reduc, gimple_stmt_iterator *gsi,
>    gimple *new_assign;
>    tree rhs;
>    tree rhs1 = gimple_assign_rhs1 (reduc);
> +  tree lhs = gimple_assign_lhs (reduc);
>    tree tmp = make_temp_ssa_name (TREE_TYPE (rhs1), NULL, "_ifc_");
>    tree c;
>    enum tree_code reduction_op  = gimple_assign_rhs_code (reduc);
> -  tree op_nochange = neutral_op_for_reduction (TREE_TYPE (rhs1), reduction_op, NULL);
> +  tree op_nochange = neutral_op_for_reduction (TREE_TYPE (rhs1), reduction_op,
> +                                              NULL, false);
>    gimple_seq stmts = NULL;
>
>    if (dump_file && (dump_flags & TDF_DETAILS))
> @@ -1864,19 +1866,52 @@ convert_scalar_cond_reduction (gimple *reduc, gimple_stmt_iterator *gsi,
>        print_gimple_stmt (dump_file, reduc, 0, TDF_SLIM);
>      }
>
> -  /* Build cond expression using COND and constant operand
> -     of reduction rhs.  */
> -  c = fold_build_cond_expr (TREE_TYPE (rhs1),
> -                           unshare_expr (cond),
> -                           swap ? op_nochange : op1,
> -                           swap ? op1 : op_nochange);
> +  /* If possible try to create an IFN_COND_ADD instead of a COND_EXPR and
> +     a PLUS_EXPR.  Don't do this if the reduction def operand itself is
> +     a vectorizable call as we can create a COND version of it directly.  */
> +  internal_fn ifn;
> +  ifn = get_conditional_internal_fn (reduction_op);
>
> -  /* Create assignment stmt and insert it at GSI.  */
> -  new_assign = gimple_build_assign (tmp, c);
> -  gsi_insert_before (gsi, new_assign, GSI_SAME_STMT);
> -  /* Build rhs for unconditional increment/decrement/logic_operation.  */
> -  rhs = gimple_build (&stmts, reduction_op,
> -                     TREE_TYPE (rhs1), op0, tmp);
> +  bool try_cond_op = true;
> +  gimple *opstmt;
> +  if (TREE_CODE (op1) == SSA_NAME
> +      && (opstmt = SSA_NAME_DEF_STMT (op1))
> +      && is_gimple_call (opstmt))
> +    {
> +      combined_fn cfn = gimple_call_combined_fn (opstmt);
> +      internal_fn ifnop;
> +      reduction_fn_for_scalar_code (cfn, &ifnop);
> +      if (vectorized_internal_fn_supported_p (ifnop, TREE_TYPE
> +                                             (gimple_call_lhs (opstmt))))
> +       try_cond_op = false;
> +    }
> +
> +  if (ifn != IFN_LAST
> +      && vectorized_internal_fn_supported_p (ifn, TREE_TYPE (lhs))
> +      && try_cond_op && !swap)
> +    {
> +      gcall *cond_call = gimple_build_call_internal (ifn, 4,
> +                                                    unshare_expr (cond),
> +                                                    op0, op1, op0);
> +      gsi_insert_before (gsi, cond_call, GSI_SAME_STMT);
> +      gimple_call_set_lhs (cond_call, tmp);
> +      rhs = tmp;
> +    }
> +  else
> +    {
> +      /* Build cond expression using COND and constant operand
> +        of reduction rhs.  */
> +      c = fold_build_cond_expr (TREE_TYPE (rhs1),
> +                               unshare_expr (cond),
> +                               swap ? op_nochange : op1,
> +                               swap ? op1 : op_nochange);
> +      /* Create assignment stmt and insert it at GSI.  */
> +      new_assign = gimple_build_assign (tmp, c);
> +      gsi_insert_before (gsi, new_assign, GSI_SAME_STMT);
> +      /* Build rhs for unconditional increment/decrement/logic_operation.  */
> +      rhs = gimple_build (&stmts, reduction_op,
> +                         TREE_TYPE (rhs1), op0, tmp);
> +    }
>
>    if (has_nop)
>      {
> @@ -2241,7 +2276,7 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
>         {
>           /* Convert reduction stmt into vectorizable form.  */
>           rhs = convert_scalar_cond_reduction (reduc, gsi, cond, op0, op1,
> -                                              swap,has_nop, nop_reduc);
> +                                              swap, has_nop, nop_reduc);
>           redundant_ssa_names.safe_push (std::make_pair (res, rhs));
>         }
>        new_stmt = gimple_build_assign (res, rhs);
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index 23c6e8259e7..94d3cead1e6 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -3672,7 +3672,7 @@ vect_analyze_loop (class loop *loop, vec_info_shared *shared)
>  static bool
>  fold_left_reduction_fn (code_helper code, internal_fn *reduc_fn)
>  {
> -  if (code == PLUS_EXPR)
> +  if (code == PLUS_EXPR || code == MINUS_EXPR)
>      {
>        *reduc_fn = IFN_FOLD_LEFT_PLUS;
>        return true;
> @@ -3751,23 +3751,29 @@ reduction_fn_for_scalar_code (code_helper code, internal_fn *reduc_fn)
>     by the introduction of additional X elements, return that X, otherwise
>     return null.  CODE is the code of the reduction and SCALAR_TYPE is type
>     of the scalar elements.  If the reduction has just a single initial value
> -   then INITIAL_VALUE is that value, otherwise it is null.  */
> +   then INITIAL_VALUE is that value, otherwise it is null.
> +   If AS_INITIAL is TRUE the value is supposed to be used as initial value.
> +   In that case no signed zero is returned.  */
>
>  tree
>  neutral_op_for_reduction (tree scalar_type, code_helper code,
> -                         tree initial_value)
> +                         tree initial_value, bool as_initial)
>  {
>    if (code.is_tree_code ())
>      switch (tree_code (code))
>        {
> -      case WIDEN_SUM_EXPR:
>        case DOT_PROD_EXPR:
>        case SAD_EXPR:
> -      case PLUS_EXPR:
>        case MINUS_EXPR:
>        case BIT_IOR_EXPR:
>        case BIT_XOR_EXPR:
>         return build_zero_cst (scalar_type);
> +      case WIDEN_SUM_EXPR:
> +      case PLUS_EXPR:
> +       if (!as_initial && HONOR_SIGNED_ZEROS (scalar_type))
> +         return build_real (scalar_type, dconstm0);
> +       else
> +         return build_zero_cst (scalar_type);
>
>        case MULT_EXPR:
>         return build_one_cst (scalar_type);
> @@ -4106,8 +4112,14 @@ vect_is_simple_reduction (loop_vec_info loop_info, stmt_vec_info phi_info,
>            return NULL;
>          }
>
> -      nphi_def_loop_uses++;
> -      phi_use_stmt = use_stmt;
> +      /* In case of a COND_OP (mask, op1, op2, op1) reduction we might have
> +        op1 twice (once as definition, once as else) in the same operation.
> +        Only count it as one. */
> +      if (use_stmt != phi_use_stmt)
> +       {
> +         nphi_def_loop_uses++;
> +         phi_use_stmt = use_stmt;
> +       }
>      }
>
>    tree latch_def = PHI_ARG_DEF_FROM_EDGE (phi, loop_latch_edge (loop));
> @@ -6378,7 +6390,7 @@ vect_create_epilog_for_reduction (loop_vec_info loop_vinfo,
>           if (REDUC_GROUP_FIRST_ELEMENT (stmt_info))
>             initial_value = reduc_info->reduc_initial_values[0];
>           neutral_op = neutral_op_for_reduction (TREE_TYPE (vectype), code,
> -                                                initial_value);
> +                                                initial_value, false);
>         }
>        if (neutral_op)
>         vector_identity = gimple_build_vector_from_val (&seq, vectype,
> @@ -6860,8 +6872,8 @@ vectorize_fold_left_reduction (loop_vec_info loop_vinfo,
>                                gimple_stmt_iterator *gsi,
>                                gimple **vec_stmt, slp_tree slp_node,
>                                gimple *reduc_def_stmt,
> -                              tree_code code, internal_fn reduc_fn,
> -                              tree ops[3], tree vectype_in,
> +                              code_helper code, internal_fn reduc_fn,
> +                              tree *ops, int num_ops, tree vectype_in,
>                                int reduc_index, vec_loop_masks *masks,
>                                vec_loop_lens *lens)
>  {
> @@ -6877,17 +6889,40 @@ vectorize_fold_left_reduction (loop_vec_info loop_vinfo,
>
>    gcc_assert (!nested_in_vect_loop_p (loop, stmt_info));
>    gcc_assert (ncopies == 1);
> -  gcc_assert (TREE_CODE_LENGTH (code) == binary_op);
> +
> +  bool is_cond_op = false;
> +  if (code.is_tree_code ())
> +    code = tree_code (code);
> +  else
> +    {
> +      gcc_assert (cond_fn_p (code));
> +      is_cond_op = true;
> +      code = conditional_internal_fn_code (internal_fn (code));
> +    }
> +
> +  gcc_assert (TREE_CODE_LENGTH (tree_code (code)) == binary_op);
>
>    if (slp_node)
>      gcc_assert (known_eq (TYPE_VECTOR_SUBPARTS (vectype_out),
>                           TYPE_VECTOR_SUBPARTS (vectype_in)));
>
> -  tree op0 = ops[1 - reduc_index];
> +  /* The operands either come from a binary operation or an IFN_COND operation.
> +     The former is a gimple assign with binary rhs and the latter is a
> +     gimple call with four arguments.  */
> +  gcc_assert (num_ops == 2 || num_ops == 4);
> +  tree op0, opmask;
> +  if (!is_cond_op)
> +    op0 = ops[1 - reduc_index];
> +  else
> +    {
> +      op0 = ops[2];
> +      opmask = ops[0];
> +      gcc_assert (!slp_node);
> +    }
>
>    int group_size = 1;
>    stmt_vec_info scalar_dest_def_info;
> -  auto_vec<tree> vec_oprnds0;
> +  auto_vec<tree> vec_oprnds0, vec_opmask;
>    if (slp_node)
>      {
>        auto_vec<vec<tree> > vec_defs (2);
> @@ -6903,9 +6938,17 @@ vectorize_fold_left_reduction (loop_vec_info loop_vinfo,
>        vect_get_vec_defs_for_operand (loop_vinfo, stmt_info, 1,
>                                      op0, &vec_oprnds0);
>        scalar_dest_def_info = stmt_info;
> +
> +      /* For an IFN_COND_OP we also need the vector mask operand.  */
> +      if (is_cond_op)
> +         vect_get_vec_defs_for_operand (loop_vinfo, stmt_info, 1,
> +                                        opmask, &vec_opmask);
>      }
>
> -  tree scalar_dest = gimple_assign_lhs (scalar_dest_def_info->stmt);
> +  gimple *sdef = scalar_dest_def_info->stmt;
> +  tree scalar_dest = is_gimple_call (sdef)
> +                      ? gimple_call_lhs (sdef)
> +                      : gimple_assign_lhs (scalar_dest_def_info->stmt);
>    tree scalar_type = TREE_TYPE (scalar_dest);
>    tree reduc_var = gimple_phi_result (reduc_def_stmt);
>
> @@ -6939,17 +6982,20 @@ vectorize_fold_left_reduction (loop_vec_info loop_vinfo,
>        tree bias = NULL_TREE;
>        if (LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
>         mask = vect_get_loop_mask (loop_vinfo, gsi, masks, vec_num, vectype_in, i);
> +      else if (is_cond_op)
> +       mask = vec_opmask[0];
>        if (LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo))
>         {
>           len = vect_get_loop_len (loop_vinfo, gsi, lens, vec_num, vectype_in,
>                                    i, 1);
>           signed char biasval = LOOP_VINFO_PARTIAL_LOAD_STORE_BIAS (loop_vinfo);
>           bias = build_int_cst (intQI_type_node, biasval);
> -         mask = build_minus_one_cst (truth_type_for (vectype_in));
> +         if (!is_cond_op)
> +           mask = build_minus_one_cst (truth_type_for (vectype_in));
>         }
>
>        /* Handle MINUS by adding the negative.  */
> -      if (reduc_fn != IFN_LAST && code == MINUS_EXPR)
> +      if (reduc_fn != IFN_LAST && tree_code (code) == MINUS_EXPR)
>         {
>           tree negated = make_ssa_name (vectype_out);
>           new_stmt = gimple_build_assign (negated, NEGATE_EXPR, def0);
> @@ -6957,7 +7003,8 @@ vectorize_fold_left_reduction (loop_vec_info loop_vinfo,
>           def0 = negated;
>         }
>
> -      if (mask && mask_reduc_fn == IFN_LAST)
> +      if (LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)
> +         && mask && mask_reduc_fn == IFN_LAST)
>         def0 = merge_with_identity (gsi, mask, vectype_out, def0,
>                                     vector_identity);
>
> @@ -6988,8 +7035,8 @@ vectorize_fold_left_reduction (loop_vec_info loop_vinfo,
>         }
>        else
>         {
> -         reduc_var = vect_expand_fold_left (gsi, scalar_dest_var, code,
> -                                            reduc_var, def0);
> +         reduc_var = vect_expand_fold_left (gsi, scalar_dest_var,
> +                                            tree_code (code), reduc_var, def0);
>           new_stmt = SSA_NAME_DEF_STMT (reduc_var);
>           /* Remove the statement, so that we can use the same code paths
>              as for statements that we've just created.  */
> @@ -7440,6 +7487,11 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
>        if (i == STMT_VINFO_REDUC_IDX (stmt_info))
>         continue;
>
> +      /* For an IFN_COND_OP we might hit the reduction definition operand
> +        twice (once as definition, once as else).  */
> +      if (op.ops[i] == op.ops[STMT_VINFO_REDUC_IDX (stmt_info)])
> +       continue;
> +
>        /* There should be only one cycle def in the stmt, the one
>           leading to reduc_def.  */
>        if (VECTORIZABLE_CYCLE_DEF (dt))
> @@ -7640,6 +7692,13 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
>            when generating the code inside the loop.  */
>
>    code_helper orig_code = STMT_VINFO_REDUC_CODE (phi_info);
> +
> +  /* If conversion might have created a conditional operation like
> +     IFN_COND_ADD already.  Use the internal code for the following checks.  */
> +  if (cond_fn_p (orig_code))
> +      orig_code = conditional_internal_fn_code
> +       (as_internal_fn(combined_fn (orig_code)));
> +
>    STMT_VINFO_REDUC_CODE (reduc_info) = orig_code;
>
>    vect_reduction_type reduction_type = STMT_VINFO_REDUC_TYPE (reduc_info);
> @@ -7678,7 +7737,7 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
>         {
>           if (dump_enabled_p ())
>             dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> -                           "reduction: not commutative/associative");
> +                           "reduction: not commutative/associative\n");
>           return false;
>         }
>      }
> @@ -8213,6 +8272,7 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
>
>    code_helper code = canonicalize_code (op.code, op.type);
>    internal_fn cond_fn = get_conditional_internal_fn (code, op.type);
> +
>    vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
>    vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
>    bool mask_by_cond_expr = use_mask_by_cond_expr_p (code, cond_fn, vectype_in);
> @@ -8231,17 +8291,21 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
>    if (code == COND_EXPR)
>      gcc_assert (ncopies == 1);
>
> +  /* A COND_OP reduction must have the same definition and else value. */
> +  if (cond_fn_p (code))
> +    gcc_assert (op.num_ops == 4 && (op.ops[1] == op.ops[3]));
> +
>    bool masked_loop_p = LOOP_VINFO_FULLY_MASKED_P (loop_vinfo);
>
>    vect_reduction_type reduction_type = STMT_VINFO_REDUC_TYPE (reduc_info);
>    if (reduction_type == FOLD_LEFT_REDUCTION)
>      {
>        internal_fn reduc_fn = STMT_VINFO_REDUC_FN (reduc_info);
> -      gcc_assert (code.is_tree_code ());
> +      gcc_assert (code.is_tree_code () || cond_fn_p (code));
>        return vectorize_fold_left_reduction
>           (loop_vinfo, stmt_info, gsi, vec_stmt, slp_node, reduc_def_phi,
> -          tree_code (code), reduc_fn, op.ops, vectype_in, reduc_index, masks,
> -          lens);
> +          code, reduc_fn, op.ops, op.num_ops, vectype_in,
> +          reduc_index, masks, lens);
>      }
>
>    bool single_defuse_cycle = STMT_VINFO_FORCE_SINGLE_CYCLE (reduc_info);
> @@ -8254,14 +8318,20 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
>    tree scalar_dest = gimple_get_lhs (stmt_info->stmt);
>    tree vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
>
> +  /* Get NCOPIES vector definitions for all operands except the reduction
> +     definition.  */
>    vect_get_vec_defs (loop_vinfo, stmt_info, slp_node, ncopies,
>                      single_defuse_cycle && reduc_index == 0
>                      ? NULL_TREE : op.ops[0], &vec_oprnds0,
>                      single_defuse_cycle && reduc_index == 1
>                      ? NULL_TREE : op.ops[1], &vec_oprnds1,
> -                    op.num_ops == 3
> -                    && !(single_defuse_cycle && reduc_index == 2)
> +                    op.num_ops == 4
> +                    || (op.num_ops == 3
> +                        && !(single_defuse_cycle && reduc_index == 2))
>                      ? op.ops[2] : NULL_TREE, &vec_oprnds2);
> +
> +  /* For single def-use cycles get one copy of the vectorized reduction
> +     definition.  */
>    if (single_defuse_cycle)
>      {
>        gcc_assert (!slp_node);
> @@ -8301,7 +8371,7 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
>         }
>        else
>         {
> -         if (op.num_ops == 3)
> +         if (op.num_ops >= 3)
>             vop[2] = vec_oprnds2[i];
>
>           if (masked_loop_p && mask_by_cond_expr)
> @@ -8314,10 +8384,16 @@ vect_transform_reduction (loop_vec_info loop_vinfo,
>           if (emulated_mixed_dot_prod)
>             new_stmt = vect_emulate_mixed_dot_prod (loop_vinfo, stmt_info, gsi,
>                                                     vec_dest, vop);
> -         else if (code.is_internal_fn ())
> +
> +         else if (code.is_internal_fn () && !cond_fn_p (code))
>             new_stmt = gimple_build_call_internal (internal_fn (code),
>                                                    op.num_ops,
>                                                    vop[0], vop[1], vop[2]);
> +         else if (cond_fn_p (code))
> +           new_stmt = gimple_build_call_internal (internal_fn (code),
> +                                                  op.num_ops,
> +                                                  vop[0], vop[1], vop[2],
> +                                                  vop[1]);
>           else
>             new_stmt = gimple_build_assign (vec_dest, tree_code (op.code),
>                                             vop[0], vop[1], vop[2]);
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index f1d0cd79961..e22067400af 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -2319,7 +2319,7 @@ extern tree vect_create_addr_base_for_vector_ref (vec_info *,
>                                                   tree);
>
>  /* In tree-vect-loop.cc.  */
> -extern tree neutral_op_for_reduction (tree, code_helper, tree);
> +extern tree neutral_op_for_reduction (tree, code_helper, tree, bool = true);
>  extern widest_int vect_iv_limit_for_partial_vectors (loop_vec_info loop_vinfo);
>  bool vect_rgroup_iv_might_wrap_p (loop_vec_info, rgroup_controls *);
>  /* Used in tree-vect-loop-manip.cc */
> --
> 2.41.0

      parent reply	other threads:[~2023-11-02 23:26 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-20 13:51 Robin Dapp
2023-09-27  0:44 ` Tamar Christina
2023-10-04  7:54   ` Robin Dapp
2023-10-04 13:15     ` Robin Dapp
2023-10-04 15:12     ` Tamar Christina
2023-10-05  8:54       ` Robin Dapp
2023-10-05  9:02       ` Robin Dapp
2023-10-05 14:05       ` Robin Dapp
2023-10-05 14:15         ` Tamar Christina
2023-10-06  9:10         ` Richard Biener
2023-10-06 12:28           ` Robin Dapp
2023-10-06 12:30             ` Robin Dapp
2023-10-06 13:43               ` Richard Biener
2023-10-06 20:54                 ` Robin Dapp
2023-10-09  8:25                   ` Richard Biener
2023-10-09 12:54                     ` Robin Dapp
2023-10-09 13:05                       ` Richard Biener
2023-10-09  5:50         ` Richard Sandiford
2023-10-09 12:02           ` Robin Dapp
2023-10-09 14:57             ` Richard Sandiford
2023-10-11 19:15               ` Robin Dapp
2023-10-12 10:47                 ` Richard Sandiford
2023-10-12 11:11                 ` Richard Biener
2023-10-19 20:07                   ` Robin Dapp
2023-10-23 10:53                     ` Richard Biener
2023-10-24 11:11                       ` Richard Sandiford
2023-10-24 19:56                         ` Robin Dapp
2023-10-31 21:04                           ` Richard Sandiford
2023-10-31 21:19                             ` Robin Dapp
2023-11-02  7:48                               ` Richard Biener
2023-09-27 11:42 ` Richard Biener
2023-11-02 23:26 ` Andrew Pinski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+=Sn1n9gB6JvjSNRQxatUz1ANK+rTXaJ9WAqa+=YvWVjvEv3Q@mail.gmail.com' \
    --to=pinskia@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rdapp.gcc@gmail.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).