From: Hongtao Liu <crazylht@gmail.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix incorrect handle in vectorizable_induction for mixed induction type.
Date: Tue, 20 Sep 2022 10:28:54 +0800 [thread overview]
Message-ID: <CAMZc-byB7D8icOuCm2RVsRsXjLJyQkbiTEHW==u+Qs9G0uXyfA@mail.gmail.com> (raw)
In-Reply-To: <20220920022133.64778-1-hongtao.liu@intel.com>
On Tue, Sep 20, 2022 at 10:23 AM liuhongt <hongtao.liu@intel.com> wrote:
>
> The codes in vectorizable_induction for slp_node assume all phi_info
> have same induction type(vect_step_op_add), but since we support
> nonlinear induction, it could be wrong handled.
> So the patch return false when slp_node has mixed induction type.
>
> Note codes in other place will still vectorize the induction with
> separate iv update and vec_perm. But slp_node handle in
> vectorizable_induction will be more optimal when all induction type
> are the same, it will update ivs with one operation instead of
> separate iv updates and permutation.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR tree-optimization/103144
> * tree-vect-loop.cc (vectorizable_induction): Return false for
> slp_node with mixed induction type.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr103144-mix-1.c: New test.
> * gcc.target/i386/pr103144-mix-2.c: New test.
> ---
> .../gcc.target/i386/pr103144-mix-1.c | 17 +++++++++
> .../gcc.target/i386/pr103144-mix-2.c | 35 +++++++++++++++++++
> gcc/tree-vect-loop.cc | 34 ++++++++++++++----
> 3 files changed, 79 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr103144-mix-1.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr103144-mix-2.c
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr103144-mix-1.c b/gcc/testsuite/gcc.target/i386/pr103144-mix-1.c
> new file mode 100644
> index 00000000000..b292d66ef71
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr103144-mix-1.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "optimized" } } */
> +/* For induction variable with differernt induction type(vect_step_op_add, vect_step_op_neg),
> + It should't be handled in vectorizable_induction with just 1 single iv update(addition.),
> + separate iv update and vec_perm are needed. */
> +int
> +__attribute__((noipa))
> +foo (int* p, int c, int n)
> +{
> + for (int i = 0; i != n; i++)
> + {
> + p[2* i]= i;
> + p[2 * i+1] = c;
> + c = -c;
> + }
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr103144-mix-2.c b/gcc/testsuite/gcc.target/i386/pr103144-mix-2.c
> new file mode 100644
> index 00000000000..b7043d59aec
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr103144-mix-2.c
> @@ -0,0 +1,35 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -mavx2 -ftree-vectorize -fvect-cost-model=unlimited -mprefer-vector-width=256" } */
> +/* { dg-require-effective-target avx2 } */
> +
> +#include "avx2-check.h"
> +#include <string.h>
> +#include "pr103144-mix-1.c"
> +
> +typedef int v8si __attribute__((vector_size(32)));
> +
> +#define N 34
> +void
> +avx2_test (void)
> +{
> + int* epi32_exp = (int*) malloc (N * sizeof (int));
> + int* epi32_dst = (int*) malloc (N * sizeof (int));
> +
> + __builtin_memset (epi32_exp, 0, N * sizeof (int));
> + int b = 8;
> + v8si init1 = __extension__(v8si) { 0, b, 1, -b, 2, b, 3, -b };
> + v8si init2 = __extension__(v8si) { 4, b, 5, -b, 6, b, 7, -b };
> + v8si init3 = __extension__(v8si) { 8, b, 9, -b, 10, b, 11, -b };
> + v8si init4 = __extension__(v8si) { 12, b, 13, -b, 14, b, 15, -b };
> + memcpy (epi32_exp, &init1, 32);
> + memcpy (epi32_exp + 8, &init2, 32);
> + memcpy (epi32_exp + 16, &init3, 32);
> + memcpy (epi32_exp + 24, &init4, 32);
> + epi32_exp[32] = 16;
> + epi32_exp[33] = b;
> + foo (epi32_dst, b, N / 2);
> + if (__builtin_memcmp (epi32_dst, epi32_exp, N * sizeof (int)) != 0)
> + __builtin_abort ();
> +
> + return;
> +}
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index 9c434b66c5b..c7050a47c1c 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -9007,14 +9007,34 @@ vectorizable_induction (loop_vec_info loop_vinfo,
> iv_loop = loop;
> gcc_assert (iv_loop == (gimple_bb (phi))->loop_father);
>
> - if (slp_node && !nunits.is_constant ())
> + if (slp_node)
> {
> - /* The current SLP code creates the step value element-by-element. */
> - if (dump_enabled_p ())
> - dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> - "SLP induction not supported for variable-length"
> - " vectors.\n");
> - return false;
> + if (!nunits.is_constant ())
> + {
> + /* The current SLP code creates the step value element-by-element. */
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> + "SLP induction not supported for variable-length"
> + " vectors.\n");
> + return false;
> + }
> +
> + stmt_vec_info phi_info;
> + FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (slp_node), i, phi_info)
> + {
> + if (STMT_VINFO_LOOP_PHI_EVOLUTION_TYPE (phi_info) != vect_step_op_add)
> + {
> + /* The below SLP code assume all induction type to be the same.
> + But slp in other place will still vectorize the loop via updating
> + iv update separately + vec_perm, but not from below codes. */
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> + "SLP induction not supported for mixed induction type"
> + " vectors.\n");
> + return false;
> + }
> + }
> +
> }
>
> if (FLOAT_TYPE_P (vectype) && !param_vect_induction_float)
> --
> 2.18.1
>
--
BR,
Hongtao
next prev parent reply other threads:[~2022-09-20 2:26 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-20 2:21 liuhongt
2022-09-20 2:28 ` Hongtao Liu [this message]
2022-09-21 7:31 ` Richard Biener
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='CAMZc-byB7D8icOuCm2RVsRsXjLJyQkbiTEHW==u+Qs9G0uXyfA@mail.gmail.com' \
--to=crazylht@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.guenther@gmail.com \
/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).