public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	 Richard Sandiford <richard.sandiford@arm.com>
Subject: Re: [PATCH 1/3][vect] Add main vectorized loop unrolling
Date: Tue, 21 Sep 2021 14:30:52 +0200 (CEST)	[thread overview]
Message-ID: <q2n8qs0-p9s-nq31-6sp-79n7o35730ss@fhfr.qr> (raw)
In-Reply-To: <27777876-4201-5e86-bf9a-063143d38641@arm.com>

On Fri, 17 Sep 2021, Andre Vieira (lists) wrote:

> Hi all,
> 
> This patch adds the ability to define a target hook to unroll the main
> vectorized loop. It also introduces --param's vect-unroll and
> vect-unroll-reductions to control this through a command-line. I found this
> useful to experiment and believe can help when tuning, so I decided to leave
> it in.
> We only unroll the main loop and have disabled unrolling epilogues for now. We
> also do not support unrolling of any loop that has a negative step and we do
> not support unrolling a loop with any reduction other than a
> TREE_CODE_REDUCTION.
> 
> Bootstrapped and regression tested on aarch64-linux-gnu as part of the series.

I wonder why we want to change the vector modes used for the epilogue,
we're either making it more likely to need to fall through to the
scalar epilogue or require another vectorized epilogue.

That said, for simplicity I'd only change the VF of the main loop.

There I wonder why you need to change vect_update_vf_for_slp or
vect_determine_partial_vectors_and_peeling and why it's not enough
to adjust the VF in a single place, I'd do that here:

  /* This is the point where we can re-start analysis with SLP forced off.  
*/
start_over:

  /* Now the vectorization factor is final.  */
  poly_uint64 vectorization_factor = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
  gcc_assert (known_ne (vectorization_factor, 0U));

---->  call vect_update_vf_for_unroll ()

note there's also loop->unroll (from #pragma GCC unroll) which we
could include in what you look at in vect_unroll_value.

I don't like add_stmt_cost_for_unroll - how should a target go
and decide based on what it is fed?  You could as well feed it
the scalar body or the vinfo so it can get a shot at all
the vectorizers meta data - but feeding it individual stmt_infos
does not add any meaningful abstraction and thus what's the
point?

I _think_ what would make some sense is when we actually cost
the vector body (with the not unrolled VF) ask the target
"well, how about unrolling this?" because there it has the
chance to look at the actual vector stmts produced (in "cost form").
And if the target answers "yeah - go ahead and try x4" we signal
that to the iteration and have "mode N with x4 unroll" validated and
costed.

So instead of a new target hook amend the finish_cost hook to
produce a suggested unroll value and cost both the unrolled and
not unrolled body.

Sorry for steering in a different direction ;)

Thanks,
Richard.



> gcc/ChangeLog:
> 
>         * doc/tm.texi: Document TARGET_VECTORIZE_UNROLL_FACTOR
>         and TARGET_VECTORIZE_ADD_STMT_COST_FOR_UNROLL.
>         * doc/tm.texi.in: Add entries for target hooks above.
>         * params.opt: Add vect-unroll and vect-unroll-reductions 
> parameters.
>         * target.def: Define hooks TARGET_VECTORIZE_UNROLL_FACTOR
>         and TARGET_VECTORIZE_ADD_STMT_COST_FOR_UNROLL.
>         * targhooks.c (default_add_stmt_cost_for_unroll): New.
>         (default_unroll_factor): Likewise.
>         * targhooks.h (default_add_stmt_cost_for_unroll): Likewise.
>         (default_unroll_factor): Likewise.
>         * tree-vect-loop.c (_loop_vec_info::_loop_vec_info): Initialize
>         par_unrolling_factor.
>         (vect_update_vf_for_slp): Use unrolling factor to update 
> vectorization
>         factor.
>         (vect_determine_partial_vectors_and_peeling): Account for 
> unrolling.
>         (vect_determine_unroll_factor): Determine how much to unroll
> vectorized
>         main loop.
>         (vect_analyze_loop_2): Call vect_determine_unroll_factor.
>         (vect_analyze_loop): Allow for epilogue vectorization when 
> unrolling
>         and rewalk vector_mode array for the epilogues.
>         (vectorizable_reduction): Disable single_defuse_cycle when 
> unrolling.
>         * tree-vectorizer.h (vect_unroll_value): Declare par_unrolling_factor
>         as a member of loop_vec_info.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

  reply	other threads:[~2021-09-21 12:30 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-17 15:27 [PATCH 0/3][vect] Enable vector unrolling of main loop Andre Vieira (lists)
2021-09-17 15:31 ` [PATCH 1/3][vect] Add main vectorized loop unrolling Andre Vieira (lists)
2021-09-21 12:30   ` Richard Biener [this message]
2021-09-21 16:34     ` Andre Vieira (lists)
2021-09-22  6:14       ` Richard Biener
2021-09-30  8:52         ` [PATCH 1v2/3][vect] " Andre Vieira (lists)
2021-10-01  8:19           ` Richard Biener
2021-10-04 16:30             ` Richard Sandiford
2021-10-12 10:35             ` Andre Vieira (lists)
2021-10-15  8:48               ` Richard Biener
2021-10-20 13:29                 ` Andre Vieira (lists)
2021-10-21 12:14                   ` Richard Biener
2021-10-22 10:18                     ` Richard Sandiford
2021-11-11 16:02                       ` Andre Vieira (lists)
2021-11-12 13:12                         ` Richard Biener
2021-11-22 11:41                           ` Andre Vieira (lists)
2021-11-22 12:39                             ` Richard Biener
2021-11-24  9:46                               ` Andre Vieira (lists)
2021-11-24 11:00                                 ` Richard Biener
2021-11-25 10:40                                   ` Andre Vieira (lists)
2021-11-25 12:46                                     ` Richard Biener
2021-11-30 11:36                                       ` Andre Vieira (lists)
2021-11-30 13:56                                         ` Richard Biener
2021-12-07 11:27                                           ` [vect] Re-analyze all modes for epilogues Andre Vieira (lists)
2021-12-07 11:31                                             ` Andre Vieira (lists)
2021-12-07 11:48                                               ` Richard Biener
2021-12-07 13:31                                                 ` Richard Sandiford
2021-12-07 13:33                                                   ` Richard Biener
2021-12-07 11:45                                             ` Richard Biener
2021-12-07 15:17                                               ` Andre Vieira (lists)
2021-12-13 16:41                                                 ` Andre Vieira (lists)
2021-12-14 11:39                                                   ` Richard Sandiford
2021-12-17 16:33                                                     ` Andre Vieira (lists)
2022-01-07 12:39                                                       ` Richard Sandiford
2022-01-10 18:31                                           ` [PATCH 1v2/3][vect] Add main vectorized loop unrolling Andre Vieira (lists)
2022-01-11  7:14                                             ` Richard Biener
2021-10-22 10:12                   ` Richard Sandiford
2021-09-17 15:32 ` [PATCH 2/3][vect] Consider outside costs earlier for epilogue loops Andre Vieira (lists)
2021-10-14 13:44   ` Andre Vieira (lists)
2021-10-22 15:33   ` Richard Sandiford

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=q2n8qs0-p9s-nq31-6sp-79n7o35730ss@fhfr.qr \
    --to=rguenther@suse.de \
    --cc=andre.simoesdiasvieira@arm.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).