From: Richard Biener <richard.guenther@gmail.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>, richard.sandiford@arm.com
Subject: Re: [9b/n] PR85694: Make vect_is_simple_use look through pattern statements
Date: Thu, 28 Jun 2018 09:57:00 -0000 [thread overview]
Message-ID: <CAFiYyc0SpYOb6Wsk_mNzVNRVgjYWHPGL4WjVSthzdOE2q3GBPQ@mail.gmail.com> (raw)
In-Reply-To: <87r2ksbmix.fsf_-_@arm.com>
On Wed, Jun 27, 2018 at 11:34 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Sandiford <richard.sandiford@arm.com> writes:
> > Richard Biener <richard.guenther@gmail.com> writes:
> >> On Mon, Jun 18, 2018 at 5:04 PM Richard Sandiford
> >> <richard.sandiford@arm.com> wrote:
> >>>
> >>> When following the definitions of SSA names, some recognisers
> >>> already cope with statements that have been replaced by patterns.
> >>> This patch makes that happen automatically for users of
> >>> type_conversion_p and vect_get_internal_def. It also adds
> >>> a vect_look_through_pattern helper that can be used directly.
> >>>
> >>> The reason for doing this is that the main patch for PR85694
> >>> makes over_widening handle more general cases. These over-widened
> >>> patterns can still be useful when matching later statements;
> >>> e.g. an overwidened MULT_EXPR could be the input to a DOT_PROD_EXPR.
> >>>
> >>> The patch doesn't do anything with the STMT_VINFO_IN_PATTERN_P checks
> >>> in vect_recog_over_widening_pattern or vect_recog_widen_shift_pattern
> >>> since later patches rewrite them anyway.
> >>>
> >>> Doing this fixed an XFAIL in vect-reduc-dot-u16b.c.
> >>>
> >>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
> >>
> >> Hmm. It seems to me that *def_stmt for vect_is_simple_use should
> >> eventually be the pattern def given the vectype overload takes the
> >> vectype from the pattern def already but oddly enough the
> >> DEF_TYPE is taken from the non-pattern stmt.
> >>
> >> I wonder which callers look at def_stmt at all (and how...)
> >>
> >> I guess swapping the def_stmt and dt arguments and adding yet another
> >> overload to remove all unused &def_stmt args might this easier to review...
> >>
> >> So - I'm suggesting to change vect_is_simple_use.
> >
> > OK, I'll try that. Might end up being its own mini-series. :-)
>
> And here's the second and final part: make vect_is_simple_use check
> whether a defining statement has been replaced by a pattern statement,
> and if so return the pattern statement instead.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
OK.
Thanks,
Richard.
> Richard
>
>
> 2018-06-14 Richard Sandiford <richard.sandiford@arm.com>
>
> gcc/
> * tree-vect-loop.c (vectorizable_reduction): Assert that the
> phi is not a pattern statement and has not been replaced by
> a pattern statement.
> * tree-vect-patterns.c (type_conversion_p): Don't check
> STMT_VINFO_IN_PATTERN_P.
> (vect_recog_vector_vector_shift_pattern): Likewise.
> (vect_recog_dot_prod_pattern): Expect vect_is_simple_use to return
> the pattern statement rather than the original statement; check
> directly for a WIDEN_MULT_EXPR here.
> * tree-vect-slp.c (vect_get_and_check_slp_defs): Expect
> vect_is_simple_use to return the pattern statement rather
> than the original statement; use is_pattern_stmt_p to check
> for such a pattern statement.
> * tree-vect-stmts.c (process_use): Expect vect_is_simple_use
> to return the pattern statement rather than the original statement;
> don't do the same transformation here.
> (vect_is_simple_use): If the defining statement has been replaced
> by a pattern statement, return the pattern statement instead.
> Remove the corresponding (local) transformation from the vectype
> overload.
>
> gcc/testsuite/
> * gcc.dg/vect/vect-reduc-dot-u16b.c: Remove xfail and update the
> test for vectorization along the lines described in the comment.
>
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c 2018-06-27 10:27:31.782458413 +0100
> +++ gcc/tree-vect-loop.c 2018-06-27 10:27:34.490434751 +0100
> @@ -6482,6 +6482,8 @@ vectorizable_reduction (gimple *stmt, gi
> }
>
> stmt_vec_info reduc_def_info = vinfo_for_stmt (reduc_def_stmt);
> + /* PHIs should not participate in patterns. */
> + gcc_assert (!STMT_VINFO_RELATED_STMT (reduc_def_info));
> enum vect_reduction_type v_reduc_type
> = STMT_VINFO_REDUC_TYPE (reduc_def_info);
> gimple *tmp = STMT_VINFO_REDUC_DEF (reduc_def_info);
> Index: gcc/tree-vect-patterns.c
> ===================================================================
> --- gcc/tree-vect-patterns.c 2018-06-27 10:27:31.782458413 +0100
> +++ gcc/tree-vect-patterns.c 2018-06-27 10:27:34.490434751 +0100
> @@ -193,13 +193,6 @@ type_conversion_p (tree name, gimple *us
> if (!*def_stmt)
> return false;
>
> - if (dt == vect_internal_def)
> - {
> - stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
> - if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
> - return false;
> - }
> -
> if (!is_gimple_assign (*def_stmt))
> return false;
>
> @@ -383,20 +376,11 @@ vect_recog_dot_prod_pattern (vec<gimple
> /* FORNOW. Can continue analyzing the def-use chain when this stmt in a phi
> inside the loop (in case we are analyzing an outer-loop). */
> gassign *mult = dyn_cast <gassign *> (mult_vinfo->stmt);
> - if (!mult || gimple_assign_rhs_code (mult) != MULT_EXPR)
> + if (!mult)
> return NULL;
> - if (STMT_VINFO_IN_PATTERN_P (mult_vinfo))
> + if (gimple_assign_rhs_code (mult) == WIDEN_MULT_EXPR)
> {
> /* Has been detected as a widening multiplication? */
> -
> - mult = dyn_cast <gassign *> (STMT_VINFO_RELATED_STMT (mult_vinfo));
> - if (!mult || gimple_assign_rhs_code (mult) != WIDEN_MULT_EXPR)
> - return NULL;
> - STMT_VINFO_PATTERN_DEF_SEQ (stmt_vinfo)
> - = STMT_VINFO_PATTERN_DEF_SEQ (mult_vinfo);
> - mult_vinfo = vinfo_for_stmt (mult);
> - gcc_assert (mult_vinfo);
> - gcc_assert (STMT_VINFO_DEF_TYPE (mult_vinfo) == vect_internal_def);
> oprnd00 = gimple_assign_rhs1 (mult);
> oprnd01 = gimple_assign_rhs2 (mult);
> }
> @@ -406,6 +390,9 @@ vect_recog_dot_prod_pattern (vec<gimple
> gimple *def_stmt;
> tree oprnd0, oprnd1;
>
> + if (gimple_assign_rhs_code (mult) != MULT_EXPR)
> + return NULL;
> +
> oprnd0 = gimple_assign_rhs1 (mult);
> oprnd1 = gimple_assign_rhs2 (mult);
> if (!type_conversion_p (oprnd0, mult, true, &half_type0, &def_stmt,
> @@ -2050,9 +2037,7 @@ vect_recog_vector_vector_shift_pattern (
>
> tree def = NULL_TREE;
> gassign *def_stmt = dyn_cast <gassign *> (def_vinfo->stmt);
> - if (!STMT_VINFO_IN_PATTERN_P (def_vinfo)
> - && def_stmt
> - && gimple_assign_cast_p (def_stmt))
> + if (def_stmt && gimple_assign_cast_p (def_stmt))
> {
> tree rhs1 = gimple_assign_rhs1 (def_stmt);
> if (TYPE_MODE (TREE_TYPE (rhs1)) == TYPE_MODE (TREE_TYPE (oprnd0))
> Index: gcc/tree-vect-slp.c
> ===================================================================
> --- gcc/tree-vect-slp.c 2018-06-27 10:27:31.782458413 +0100
> +++ gcc/tree-vect-slp.c 2018-06-27 10:27:34.490434751 +0100
> @@ -365,11 +365,9 @@ vect_get_and_check_slp_defs (vec_info *v
> from the pattern. Check that all the stmts of the node are in the
> pattern. */
> if (def_stmt && gimple_bb (def_stmt)
> - && vect_stmt_in_region_p (vinfo, def_stmt)
> - && vinfo_for_stmt (def_stmt)
> - && STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (def_stmt))
> - && !STMT_VINFO_RELEVANT (vinfo_for_stmt (def_stmt))
> - && !STMT_VINFO_LIVE_P (vinfo_for_stmt (def_stmt)))
> + && vect_stmt_in_region_p (vinfo, def_stmt)
> + && vinfo_for_stmt (def_stmt)
> + && is_pattern_stmt_p (vinfo_for_stmt (def_stmt)))
> {
> pattern = true;
> if (!first && !oprnd_info->first_pattern
> @@ -398,7 +396,6 @@ vect_get_and_check_slp_defs (vec_info *v
> return 1;
> }
>
> - def_stmt = STMT_VINFO_RELATED_STMT (vinfo_for_stmt (def_stmt));
> dt = STMT_VINFO_DEF_TYPE (vinfo_for_stmt (def_stmt));
>
> if (dt == vect_unknown_def_type)
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c 2018-06-27 10:27:31.782458413 +0100
> +++ gcc/tree-vect-stmts.c 2018-06-27 10:27:34.490434751 +0100
> @@ -506,8 +506,6 @@ process_use (gimple *stmt, tree use, loo
> if (dump_enabled_p ())
> dump_printf_loc (MSG_NOTE, vect_location,
> "reduc-stmt defining reduc-phi in the same nest.\n");
> - if (STMT_VINFO_IN_PATTERN_P (dstmt_vinfo))
> - dstmt_vinfo = vinfo_for_stmt (STMT_VINFO_RELATED_STMT (dstmt_vinfo));
> gcc_assert (STMT_VINFO_RELEVANT (dstmt_vinfo) < vect_used_by_reduction);
> gcc_assert (STMT_VINFO_LIVE_P (dstmt_vinfo)
> || STMT_VINFO_RELEVANT (dstmt_vinfo) > vect_unused_in_scope);
> @@ -10069,8 +10067,6 @@ vect_is_simple_use (tree operand, vec_in
> }
>
> gimple *def_stmt = SSA_NAME_DEF_STMT (operand);
> - if (def_stmt_out)
> - *def_stmt_out = def_stmt;
> if (dump_enabled_p ())
> {
> dump_printf_loc (MSG_NOTE, vect_location, "def_stmt: ");
> @@ -10082,8 +10078,15 @@ vect_is_simple_use (tree operand, vec_in
> else
> {
> stmt_vec_info stmt_vinfo = vinfo_for_stmt (def_stmt);
> + if (STMT_VINFO_IN_PATTERN_P (stmt_vinfo))
> + {
> + def_stmt = STMT_VINFO_RELATED_STMT (stmt_vinfo);
> + stmt_vinfo = vinfo_for_stmt (def_stmt);
> + }
> *dt = STMT_VINFO_DEF_TYPE (stmt_vinfo);
> }
> + if (def_stmt_out)
> + *def_stmt_out = def_stmt;
>
> if (dump_enabled_p ())
> {
> @@ -10174,12 +10177,6 @@ vect_is_simple_use (tree operand, vec_in
> || *dt == vect_nested_cycle)
> {
> stmt_vec_info stmt_info = vinfo_for_stmt (def_stmt);
> -
> - if (STMT_VINFO_IN_PATTERN_P (stmt_info)
> - && !STMT_VINFO_RELEVANT (stmt_info)
> - && !STMT_VINFO_LIVE_P (stmt_info))
> - stmt_info = vinfo_for_stmt (STMT_VINFO_RELATED_STMT (stmt_info));
> -
> *vectype = STMT_VINFO_VECTYPE (stmt_info);
> gcc_assert (*vectype != NULL_TREE);
> }
> Index: gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u16b.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u16b.c 2018-06-27 10:27:12.366628072 +0100
> +++ gcc/testsuite/gcc.dg/vect/vect-reduc-dot-u16b.c 2018-06-27 10:27:34.490434751 +0100
> @@ -10,11 +10,8 @@ #define DOT2 43680
> unsigned short X[N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
> unsigned short Y[N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
>
> -/* short->int->int dot product.
> - Currently not detected as a dot-product pattern: the multiplication
> - promotes the ushorts to int, and then the product is promoted to unsigned
> - int for the addition. Which results in an int->unsigned int cast, which
> - since no bits are modified in the cast should be trivially vectorizable. */
> +/* ushort->int->uint dot product: the multiplication promotes the ushorts
> + to int, and then the product is converted to uint for the addition. */
> __attribute__ ((noinline)) unsigned int
> foo2(int len) {
> int i;
> @@ -47,12 +44,6 @@ int main (void)
> return 0;
> }
>
> -/* { dg-final { scan-tree-dump-times "vect_recog_dot_prod_pattern: detected" 1 "vect" { xfail *-*-* } } } */
> -
> -/* Once the dot-product pattern is detected, we expect
> - that loop to be vectorized on vect_udot_hi targets (targets that support
> - dot-product of unsigned shorts) and targets that support widening multiplication. */
> -/* The induction loop in main is vectorized. */
> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" { xfail *-*-* } } } */
> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target vect_pack_trunc } } } */
> +/* { dg-final { scan-tree-dump-times "vect_recog_dot_prod_pattern: detected" 1 "vect" } } */
>
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_pack_trunc || vect_udot_hi } } } } */
prev parent reply other threads:[~2018-06-28 9:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-18 15:04 [9/n] PR85694: Add a vect_look_through_pattern helper Richard Sandiford
2018-06-19 10:12 ` Richard Biener
2018-06-20 10:24 ` Richard Sandiford
2018-06-27 9:31 ` [9a/n] PR85694: Reorder vect_is_simple_use arguments Richard Sandiford
2018-06-28 9:55 ` Richard Biener
2018-06-27 9:34 ` [9b/n] PR85694: Make vect_is_simple_use look through pattern statements Richard Sandiford
2018-06-28 9:57 ` Richard Biener [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=CAFiYyc0SpYOb6Wsk_mNzVNRVgjYWHPGL4WjVSthzdOE2q3GBPQ@mail.gmail.com \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.sandiford@arm.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).