Pinging this for a review and/or further feedback.

Thanks,
Manolis
On Wed, Jun 26, 2024 at 3:06 PM Manolis Tsamis <manolis.tsa...@vrull.eu> wrote:
>
> This change checks when a two_operators SLP node has multiple occurrences of
> the same statement (e.g. {A, B, A, B, ...}) and tries to rearrange the 
> operands
> so that there are no duplicates. Two vec_perm expressions are then introduced
> to recreate the original ordering. These duplicates can appear due to how
> two_operators nodes are handled, and they prevent vectorization in some cases.
>
> This targets the vectorization of the SPEC2017 x264 pixel_satd functions.
> In some processors a larger than 10% improvement on x264 has been observed.
>
> See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98138
>
> gcc/ChangeLog:
>
>         * tree-vect-slp.cc: Avoid duplicates in two_operators nodes.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/aarch64/vect-slp-two-operator.c: New test.
>
> Signed-off-by: Manolis Tsamis <manolis.tsa...@vrull.eu>
> ---
>
> Changes in v2:
>         - Do not use predefined patterns; support rearrangement of arbitrary
>         node orderings.
>         - Only apply for two_operators nodes.
>         - Recurse with single SLP operand instead of two duplicated ones.
>         - Refactoring of code.
>
>  .../aarch64/vect-slp-two-operator.c           |  36 ++++++
>  gcc/tree-vect-slp.cc                          | 114 ++++++++++++++++++
>  2 files changed, 150 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/vect-slp-two-operator.c
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/vect-slp-two-operator.c 
> b/gcc/testsuite/gcc.target/aarch64/vect-slp-two-operator.c
> new file mode 100644
> index 00000000000..b6b093ffc34
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/vect-slp-two-operator.c
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect 
> -fdump-tree-vect-details" } */
> +
> +typedef unsigned char uint8_t;
> +typedef unsigned int uint32_t;
> +
> +#define HADAMARD4(d0, d1, d2, d3, s0, s1, s2, s3) {\
> +    int t0 = s0 + s1;\
> +    int t1 = s0 - s1;\
> +    int t2 = s2 + s3;\
> +    int t3 = s2 - s3;\
> +    d0 = t0 + t2;\
> +    d1 = t1 + t3;\
> +    d2 = t0 - t2;\
> +    d3 = t1 - t3;\
> +}
> +
> +void sink(uint32_t tmp[4][4]);
> +
> +int x264_pixel_satd_8x4( uint8_t *pix1, int i_pix1, uint8_t *pix2, int 
> i_pix2 )
> +{
> +    uint32_t tmp[4][4];
> +    int sum = 0;
> +    for( int i = 0; i < 4; i++, pix1 += i_pix1, pix2 += i_pix2 )
> +    {
> +        uint32_t a0 = (pix1[0] - pix2[0]) + ((pix1[4] - pix2[4]) << 16);
> +        uint32_t a1 = (pix1[1] - pix2[1]) + ((pix1[5] - pix2[5]) << 16);
> +        uint32_t a2 = (pix1[2] - pix2[2]) + ((pix1[6] - pix2[6]) << 16);
> +        uint32_t a3 = (pix1[3] - pix2[3]) + ((pix1[7] - pix2[7]) << 16);
> +        HADAMARD4( tmp[i][0], tmp[i][1], tmp[i][2], tmp[i][3], a0,a1,a2,a3 );
> +    }
> +    sink(tmp);
> +}
> +
> +/* { dg-final { scan-tree-dump "vectorizing stmts using SLP" "vect" } } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index b47b7e8c979..60d0d388dff 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -2420,6 +2420,95 @@ out:
>        }
>    swap = NULL;
>
> +  bool has_two_operators_perm = false;
> +  auto_vec<unsigned> two_op_perm_indices[2];
> +  vec<stmt_vec_info> two_op_scalar_stmts[2] = {vNULL, vNULL};
> +
> +  if (two_operators && oprnds_info.length () == 2 && group_size > 2)
> +    {
> +      unsigned idx = 0;
> +      hash_map<gimple *, unsigned> seen;
> +      vec<slp_oprnd_info> new_oprnds_info
> +       = vect_create_oprnd_info (1, group_size);
> +      bool success = true;
> +
> +      enum tree_code code = ERROR_MARK;
> +      if (oprnds_info[0]->def_stmts[0]
> +         && is_a<gassign *> (oprnds_info[0]->def_stmts[0]->stmt))
> +       code = gimple_assign_rhs_code (oprnds_info[0]->def_stmts[0]->stmt);
> +
> +      for (unsigned j = 0; j < group_size; ++j)
> +       {
> +         FOR_EACH_VEC_ELT (oprnds_info, i, oprnd_info)
> +           {
> +             stmt_vec_info stmt_info = oprnd_info->def_stmts[j];
> +             if (!stmt_info || !stmt_info->stmt
> +                 || !is_a<gassign *> (stmt_info->stmt)
> +                 || gimple_assign_rhs_code (stmt_info->stmt) != code
> +                 || skip_args[i])
> +               {
> +                 success = false;
> +                 break;
> +               }
> +
> +             bool exists;
> +             unsigned &stmt_idx
> +               = seen.get_or_insert (stmt_info->stmt, &exists);
> +
> +             if (!exists)
> +               {
> +                 new_oprnds_info[0]->def_stmts.safe_push (stmt_info);
> +                 new_oprnds_info[0]->ops.safe_push (oprnd_info->ops[j]);
> +                 stmt_idx = idx;
> +                 idx++;
> +               }
> +
> +             two_op_perm_indices[i].safe_push (stmt_idx);
> +           }
> +
> +         if (!success)
> +           break;
> +       }
> +
> +      if (success && idx == group_size)
> +       {
> +         if (dump_enabled_p ())
> +           {
> +             dump_printf_loc (MSG_NOTE, vect_location,
> +                              "Replace two_operators operands:\n");
> +
> +             FOR_EACH_VEC_ELT (oprnds_info, i, oprnd_info)
> +               {
> +                 dump_printf_loc (MSG_NOTE, vect_location,
> +                                  "Operand %u:\n", i);
> +                 for (unsigned j = 0; j < group_size; j++)
> +                   dump_printf_loc (MSG_NOTE, vect_location, "\tstmt %u %G",
> +                                    j, oprnd_info->def_stmts[j]->stmt);
> +               }
> +
> +             dump_printf_loc (MSG_NOTE, vect_location,
> +                              "With a single operand:\n");
> +             for (unsigned j = 0; j < group_size; j++)
> +               dump_printf_loc (MSG_NOTE, vect_location, "\tstmt %u %G",
> +                                j, new_oprnds_info[0]->def_stmts[j]->stmt);
> +           }
> +
> +         two_op_scalar_stmts[0].safe_splice (oprnds_info[0]->def_stmts);
> +         two_op_scalar_stmts[1].safe_splice (oprnds_info[1]->def_stmts);
> +
> +         new_oprnds_info[0]->first_op_type = oprnds_info[0]->first_op_type;
> +         new_oprnds_info[0]->first_dt = oprnds_info[0]->first_dt;
> +         new_oprnds_info[0]->any_pattern = oprnds_info[0]->any_pattern;
> +         new_oprnds_info[0]->first_gs_p = oprnds_info[0]->first_gs_p;
> +         new_oprnds_info[0]->first_gs_info = oprnds_info[0]->first_gs_info;
> +
> +         vect_free_oprnd_info (oprnds_info);
> +         oprnds_info = new_oprnds_info;
> +         nops = 1;
> +         has_two_operators_perm = true;
> +       }
> +    }
> +
>    auto_vec<slp_tree, 4> children;
>
>    stmt_info = stmts[0];
> @@ -2691,6 +2780,29 @@ fail:
>          the true { a+b, a+b, a+b, a+b } ... but there we don't have
>          explicit stmts to put in so the keying on 'stmts' doesn't
>          work (but we have the same issue with nodes that use 'ops').  */
> +
> +      if (has_two_operators_perm)
> +       {
> +         slp_tree child = children[0];
> +         children.truncate (0);
> +         for (i = 0; i < 2; i++)
> +           {
> +             slp_tree pnode
> +               = vect_create_new_slp_node (two_op_scalar_stmts[i], 2);
> +             SLP_TREE_CODE (pnode) = VEC_PERM_EXPR;
> +             SLP_TREE_VECTYPE (pnode) = vectype;
> +             SLP_TREE_CHILDREN (pnode).quick_push (child);
> +             SLP_TREE_CHILDREN (pnode).quick_push (child);
> +             lane_permutation_t& perm = SLP_TREE_LANE_PERMUTATION (pnode);
> +             children.safe_push (pnode);
> +
> +             for (unsigned j = 0; j < stmts.length (); j++)
> +               perm.safe_push (std::make_pair (0, 
> two_op_perm_indices[i][j]));
> +           }
> +
> +         SLP_TREE_REF_COUNT (child) += 4;
> +       }
> +
>        slp_tree one = new _slp_tree;
>        slp_tree two = new _slp_tree;
>        SLP_TREE_DEF_TYPE (one) = vect_internal_def;
> @@ -2727,12 +2839,14 @@ fail:
>           else
>             SLP_TREE_LANE_PERMUTATION (node).safe_push (std::make_pair (0, 
> i));
>         }
> +
>        SLP_TREE_CODE (one) = code0;
>        SLP_TREE_CODE (two) = ocode;
>        SLP_TREE_LANES (one) = stmts.length ();
>        SLP_TREE_LANES (two) = stmts.length ();
>        SLP_TREE_REPRESENTATIVE (one) = stmts[0];
>        SLP_TREE_REPRESENTATIVE (two) = stmts[j];
> +
>        return node;
>      }
>
> --
> 2.44.0
>

Reply via email to