On Wed, 2 Mar 2016, Jakub Jelinek wrote:
> Hi!
>
> This patch fixes two issues:
> 1) reverts part of https://gcc.gnu.org/ml/gcc-patches/2011-06/msg02183.html
> changes, my understanding is that we don't emit or support what has been
> mentioned as rationale for that, now that we can stick multiple pattern
> stmts into a sequence; without this reversion, we mark both the pattern
> and normal stmt relevant and then when vectorizing use in one spot
> the pattern stmt and in another one the original stmt, while we clearly
> want to use the pattern stmt always; also, without the reversion we
> consider the original stmt as needed for VF computation and thus think
> the loop needs QImode elements in addition to SImode and DImode.
> 2) if the shift count is coming from stmt with corresponding pattern stmt,
> we shouldn't look through it, because we might again refer to the middle
> of a pattern stmt (this is similar to the recently committed patch); we
> don't need to punt completely in that case though, the code can just
> add a cast into the pattern sequence as it does in many other cases.
>
> Bootstrapped/regtested on {x86_64,i686,powerpc64,powerpc64le}-linux,
> bootstrap/regtest is still pending on {s390,s390x,aarch64,armv7hl}-linux,
> ok for trunk if it passes even there?
Ok. I've been trying to understand this code multiple times myself
and I'm happy to see it go ;) Even though in all cases I remember
the issue was elsewhere...
Thanks,
Richard.
> 2016-03-02 Jakub Jelinek <ja...@redhat.com>
>
> PR target/70021
> * tree-vect-stmts.c (vect_mark_relevant): Remove USED_IN_PATTERN
> argument, if STMT_VINFO_IN_PATTERN_P (stmt_info), always mark
> the pattern no matter if it is used just by non-pattern, pattern
> or mix thereof.
> (process_use, vect_mark_stmts_to_be_vectorized): Adjust callers.
> * tree-vect-patterns.c (vect_recog_vector_vector_shift_pattern): If
> oprnd1 def_stmt is in pattern, don't look through it.
>
> * gcc.dg/vect/pr70021.c: New test.
> * gcc.target/i386/pr70021.c: New test.
>
> --- gcc/tree-vect-stmts.c.jj 2016-03-01 19:23:51.000000000 +0100
> +++ gcc/tree-vect-stmts.c 2016-03-02 15:40:53.777231771 +0100
> @@ -181,8 +181,7 @@ create_array_ref (tree type, tree ptr, s
>
> static void
> vect_mark_relevant (vec<gimple *> *worklist, gimple *stmt,
> - enum vect_relevant relevant, bool live_p,
> - bool used_in_pattern)
> + enum vect_relevant relevant, bool live_p)
> {
> stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> enum vect_relevant save_relevant = STMT_VINFO_RELEVANT (stmt_info);
> @@ -202,62 +201,22 @@ vect_mark_relevant (vec<gimple *> *workl
> stmt itself should be marked. */
> if (STMT_VINFO_IN_PATTERN_P (stmt_info))
> {
> - bool found = false;
> - if (!used_in_pattern)
> - {
> - imm_use_iterator imm_iter;
> - use_operand_p use_p;
> - gimple *use_stmt;
> - tree lhs;
> - loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
> - struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
> -
> - if (is_gimple_assign (stmt))
> - lhs = gimple_assign_lhs (stmt);
> - else
> - lhs = gimple_call_lhs (stmt);
> -
> - /* This use is out of pattern use, if LHS has other uses that are
> - pattern uses, we should mark the stmt itself, and not the
> pattern
> - stmt. */
> - if (lhs && TREE_CODE (lhs) == SSA_NAME)
> - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
> - {
> - if (is_gimple_debug (USE_STMT (use_p)))
> - continue;
> - use_stmt = USE_STMT (use_p);
> -
> - if (!flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
> - continue;
> -
> - if (vinfo_for_stmt (use_stmt)
> - && STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (use_stmt)))
> - {
> - found = true;
> - break;
> - }
> - }
> - }
> + /* This is the last stmt in a sequence that was detected as a
> + pattern that can potentially be vectorized. Don't mark the stmt
> + as relevant/live because it's not going to be vectorized.
> + Instead mark the pattern-stmt that replaces it. */
>
> - if (!found)
> - {
> - /* This is the last stmt in a sequence that was detected as a
> - pattern that can potentially be vectorized. Don't mark the stmt
> - as relevant/live because it's not going to be vectorized.
> - Instead mark the pattern-stmt that replaces it. */
> -
> - pattern_stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> -
> - if (dump_enabled_p ())
> - dump_printf_loc (MSG_NOTE, vect_location,
> - "last stmt in pattern. don't mark"
> - " relevant/live.\n");
> - stmt_info = vinfo_for_stmt (pattern_stmt);
> - gcc_assert (STMT_VINFO_RELATED_STMT (stmt_info) == stmt);
> - save_relevant = STMT_VINFO_RELEVANT (stmt_info);
> - save_live_p = STMT_VINFO_LIVE_P (stmt_info);
> - stmt = pattern_stmt;
> - }
> + pattern_stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> +
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE, vect_location,
> + "last stmt in pattern. don't mark"
> + " relevant/live.\n");
> + stmt_info = vinfo_for_stmt (pattern_stmt);
> + gcc_assert (STMT_VINFO_RELATED_STMT (stmt_info) == stmt);
> + save_relevant = STMT_VINFO_RELEVANT (stmt_info);
> + save_live_p = STMT_VINFO_LIVE_P (stmt_info);
> + stmt = pattern_stmt;
> }
>
> STMT_VINFO_LIVE_P (stmt_info) |= live_p;
> @@ -572,8 +531,7 @@ process_use (gimple *stmt, tree use, loo
> }
> }
>
> - vect_mark_relevant (worklist, def_stmt, relevant, live_p,
> - is_pattern_stmt_p (stmt_vinfo));
> + vect_mark_relevant (worklist, def_stmt, relevant, live_p);
> return true;
> }
>
> @@ -630,7 +588,7 @@ vect_mark_stmts_to_be_vectorized (loop_v
> }
>
> if (vect_stmt_relevant_p (phi, loop_vinfo, &relevant, &live_p))
> - vect_mark_relevant (&worklist, phi, relevant, live_p, false);
> + vect_mark_relevant (&worklist, phi, relevant, live_p);
> }
> for (si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
> {
> @@ -642,7 +600,7 @@ vect_mark_stmts_to_be_vectorized (loop_v
> }
>
> if (vect_stmt_relevant_p (stmt, loop_vinfo, &relevant, &live_p))
> - vect_mark_relevant (&worklist, stmt, relevant, live_p, false);
> + vect_mark_relevant (&worklist, stmt, relevant, live_p);
> }
> }
>
> --- gcc/tree-vect-patterns.c.jj 2016-03-02 14:07:55.242244657 +0100
> +++ gcc/tree-vect-patterns.c 2016-03-02 15:41:33.307698533 +0100
> @@ -2090,7 +2090,8 @@ vect_recog_vector_vector_shift_pattern (
> return NULL;
>
> tree def = NULL_TREE;
> - if (gimple_assign_cast_p (def_stmt))
> + stmt_vec_info def_vinfo = vinfo_for_stmt (def_stmt);
> + if (!STMT_VINFO_IN_PATTERN_P (def_vinfo) && gimple_assign_cast_p
> (def_stmt))
> {
> tree rhs1 = gimple_assign_rhs1 (def_stmt);
> if (TYPE_MODE (TREE_TYPE (rhs1)) == TYPE_MODE (TREE_TYPE (oprnd0))
> --- gcc/testsuite/gcc.dg/vect/pr70021.c.jj 2016-03-02 15:41:33.308698520
> +0100
> +++ gcc/testsuite/gcc.dg/vect/pr70021.c 2016-03-02 15:41:33.308698520
> +0100
> @@ -0,0 +1,40 @@
> +/* PR target/70021 */
> +/* { dg-do run } */
> +
> +#include "tree-vect.h"
> +
> +#define N 160
> +int a[N];
> +unsigned long long int b[N], c[N], d[N], e[N];
> +
> +__attribute__((noinline, noclone)) void
> +foo (void)
> +{
> + int i;
> + for (i = 0; i < N; i += 4)
> + {
> + unsigned long long int f = (_Bool) b[i];
> + unsigned long long int g = c[i] != d[i];
> + e[i] = g ^ (a[i] & (g << f));
> + }
> +}
> +
> +int
> +main ()
> +{
> + int i;
> + check_vect ();
> + for (i = 0; i < N; ++i)
> + {
> + a[i] = 1618000128;
> + b[i] = 10919594786573202791ULL;
> + c[i] = 2593730175074624973ULL;
> + d[i] = 7447894520878803661ULL;
> + e[i] = 14234165565810642243ULL;
> + }
> + foo ();
> + for (i = 0; i < N; ++i)
> + if (e[i] != ((i & 3) ? 14234165565810642243ULL : 1ULL))
> + __builtin_abort ();
> + return 0;
> +}
> --- gcc/testsuite/gcc.target/i386/pr70021.c.jj 2016-03-02
> 15:41:33.308698520 +0100
> +++ gcc/testsuite/gcc.target/i386/pr70021.c 2016-03-02 15:43:17.798289031
> +0100
> @@ -0,0 +1,42 @@
> +/* PR target/70021 */
> +/* { dg-do run } */
> +/* { dg-require-effective-target avx2 } */
> +/* { dg-options "-O2 -ftree-vectorize -mavx2 -fdump-tree-vect-details" } */
> +
> +#include "avx2-check.h"
> +
> +#define N 160
> +int a[N];
> +unsigned long long int b[N], c[N], d[N], e[N];
> +
> +__attribute__((noinline, noclone)) void
> +foo (void)
> +{
> + int i;
> + for (i = 0; i < N; i += 4)
> + {
> + unsigned long long int f = (_Bool) b[i];
> + unsigned long long int g = c[i] != d[i];
> + e[i] = g ^ (a[i] & (g << f));
> + }
> +}
> +
> +void
> +avx2_test ()
> +{
> + int i;
> + for (i = 0; i < N; ++i)
> + {
> + a[i] = 1618000128;
> + b[i] = 10919594786573202791ULL;
> + c[i] = 2593730175074624973ULL;
> + d[i] = 7447894520878803661ULL;
> + e[i] = 14234165565810642243ULL;
> + }
> + foo ();
> + for (i = 0; i < N; ++i)
> + if (e[i] != ((i & 3) ? 14234165565810642243ULL : 1ULL))
> + __builtin_abort ();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
