From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix mixed input kind permute optimization
Date: Tue, 21 May 2024 19:42:09 +0100 [thread overview]
Message-ID: <mptwmnnm2gu.fsf@arm.com> (raw)
In-Reply-To: <20240521172620.284CB13685@imap1.dmz-prg2.suse.org> (Richard Biener's message of "Tue, 21 May 2024 19:26:19 +0200 (CEST)")
Richard Biener <rguenther@suse.de> writes:
> When change_vec_perm_layout runs into a permute combining two
> nodes where one is invariant and one internal the partition of
> one input can be -1 but the other might not be. The following
> supports this case by simply ignoring inputs with input partiton -1.
>
> I'm not sure this is correct but it avoids ICEing when accessing
> that partitions layout for gcc.target/i386/pr98928.c with the
> change to avoid splitting store dataref groups during SLP discovery.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu (ontop of
> the SLP series). The change can't break anything that's already
> broken but I'm not sure this does the right thing - the testcase
> has an uniform constant. I'll try to come up with a better runtime
> testcase tomorrow. Hints as to where to correctly fix such case
> appreciated.
Famous last words, but yeah, it looks correct to me. I think the
routine in principle should have a free choice of which layout to
choose for invariants (as long as it's consistent for all queries
about the same node). So it should just be a question of whether
keeping the original layout is more likely to give a valid
permutation, or whether going with out_layout_i would be better.
I don't have a strong intuition either way.
Thanks,
Richard
>
> * tree-vect-slp.cc (change_vec_perm_layout): Ignore an
> input partition of -1.
> ---
> gcc/tree-vect-slp.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 873748b0a72..f6ec1a81c96 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -4828,6 +4828,8 @@ change_vec_perm_layout (slp_tree node, lane_permutation_t &perm,
> {
> slp_tree in_node = SLP_TREE_CHILDREN (node)[entry.first];
> unsigned int in_partition_i = m_vertices[in_node->vertex].partition;
> + if (in_partition_i == -1u)
> + continue;
> this_in_layout_i = m_partitions[in_partition_i].layout;
> }
> if (this_in_layout_i > 0)
next prev parent reply other threads:[~2024-05-21 18:42 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-21 17:26 Richard Biener
2024-05-21 18:42 ` Richard Sandiford [this message]
2024-05-22 9:28 ` Richard Sandiford
2024-05-22 9:38 ` 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=mptwmnnm2gu.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rguenther@suse.de \
/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).