public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>
To: Richard Biener <rguenther@suse.de>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	Richard Sandiford <richard.sandiford@arm.com>
Subject: Re: [vect] PR103997: Fix epilogue mode skipping
Date: Thu, 13 Jan 2022 13:51:03 +0000	[thread overview]
Message-ID: <57df4982-3c4d-d170-5b28-cfa3b375586e@arm.com> (raw)
In-Reply-To: <8419o610-p15n-r889-r411-37616134p348@fhfr.qr>


On 13/01/2022 12:36, Richard Biener wrote:
> On Thu, 13 Jan 2022, Andre Vieira (lists) wrote:
>
>> This time to the list too (sorry for double email)
>>
>> Hi,
>>
>> The original patch '[vect] Re-analyze all modes for epilogues', skipped modes
>> that should not be skipped since it used the vector mode provided by
>> autovectorize_vector_modes to derive the minimum VF required for it. However,
>> those modes should only really be used to dictate vector size, so instead this
>> patch looks for the mode in 'used_vector_modes' with the largest element size,
>> and constructs a vector mode with the smae size as the current
>> vector_modes[mode_i]. Since we are using the largest element size the NUNITs
>> for this mode is the smallest possible VF required for an epilogue with this
>> mode and should thus skip only the modes we are certain can not be used.
>>
>> Passes bootstrap and regression on x86_64 and aarch64.
> Clearly
>
> +         /* To make sure we are conservative as to what modes we skip, we
> +            should use check the smallest possible NUNITS which would be
> +            derived from the mode in USED_VECTOR_MODES with the largest
> +            element size.  */
> +         scalar_mode max_elsize_mode = GET_MODE_INNER
> (vector_modes[mode_i]);
> +         for (vec_info::mode_set::iterator i =
> +               first_loop_vinfo->used_vector_modes.begin ();
> +             i != first_loop_vinfo->used_vector_modes.end (); ++i)
> +           {
> +             if (VECTOR_MODE_P (*i)
> +                 && GET_MODE_SIZE (GET_MODE_INNER (*i))
> +                 > GET_MODE_SIZE (max_elsize_mode))
> +               max_elsize_mode = GET_MODE_INNER (*i);
> +           }
>
> can be done once before iterating over the modes for the epilogue.
True, I'll start with QImode instead of the inner of 
vector_modes[mode_i] too since we can't guarantee the mode is a 
VECTOR_MODE_P and it is actually better too since we can't possible 
guarantee the element size of the USED_VECTOR_MODES is smaller than that 
of the first vector mode...

> Richard maybe knows whether we should take care to look at the
> size of the vector mode as well since related_vector_mode when
> passed 0 as nunits produces a vector mode with the same size
> as vector_modes[mode_i] but not all used_vector_modes may be
> of the same size
I suspect that should be fine though, since if we use the largest 
element size of all used_vector_modes then that should gives us the 
least possible number of NUNITS and thus only conservatively skip. That 
said, that does assume that no vector mode used may be larger than the 
size of the loop's vector_mode. Can I assume that?
>
> (and you probably also want to exclude
> VECTOR_BOOLEAN_TYPE_P from the search?)
Yeah I think so too, thanks!

I keep going back to thinking (as I brought up in the bugzilla ticket), 
maybe we ought to only skip if the NUNITS of the vector mode with the 
same vector size as vector_modes[mode_i] is larger than first_info_vf, 
or just don't skip at all...


  reply	other threads:[~2022-01-13 13:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13  9:48 Andre Vieira (lists)
2022-01-13 12:36 ` Richard Biener
2022-01-13 13:51   ` Andre Vieira (lists) [this message]
2022-01-13 14:25     ` Richard Biener
2022-01-13 15:38       ` Andre Vieira (lists)
2022-01-14  7:08         ` Richard Biener
2022-01-14  9:35           ` Andre Vieira (lists)
2022-01-14  9:57             ` Richard Biener
2022-01-18 18:14               ` Andre Vieira (lists)
2022-01-19 11:04                 ` Richard Biener
2022-01-19 14:15                   ` Andre Vieira (lists)

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=57df4982-3c4d-d170-5b28-cfa3b375586e@arm.com \
    --to=andre.simoesdiasvieira@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    --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).