From: Richard Biener <rguenther@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Jan Hubicka <jh@suse.cz>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] cgraph: Handle simd clones in cgraph_node::set_{const,pure}_flag [PR106433]
Date: Tue, 7 Feb 2023 10:28:16 +0100 [thread overview]
Message-ID: <86AB6E8B-6B1A-4A1A-96E0-9A4AB533ACD3@suse.de> (raw)
In-Reply-To: <Y+INds/30aydJlJj@tucnak>
> Am 07.02.2023 um 09:37 schrieb Jakub Jelinek <jakub@redhat.com>:
>
> Hi!
>
> The following testcase ICEs, because we determine only in late pure const
> pass that bar is const (the content of the function loses a store to a
> global var during dse3 and read from it during cddce2) and local-pure-const2
> makes it const. The cgraph ordering is that post IPA (in late IPA simd
> clones are created) bar is processed first, then foo as its caller, then
> foo.simdclone* and finally bar.simdclone*. Conceptually I think that is the
> right ordering which allows for static simd clones to be removed.
>
> The reason for the ICE is that because bar was marked const, the call to
> it lost vops before vectorization, and when we in foo.simdclone* try to
> vectorize the call to bar, we replace it with bar.simdclone* which hasn't
> been marked const and so needs vops, which we don't add.
>
> Now, because the simd clones are created from the same IL, just in a loop
> with different argument/return value passing, I think generally if the base
> function is determined to be const or pure, the simd clones should be too,
> unless e.g. the vectorization causes different optimization decisions, but
> then still the global memory reads if any shouldn't affect what the function
> does and global memory stores shouldn't be reachable at runtime.
>
> So, the following patch changes set_{const,pure}_flag to mark also simd
> clones.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok,
Thanks,
Richard
> 2023-02-07 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/106433
> * cgraph.cc (set_const_flag_1): Recurse on simd clones too.
> (cgraph_node::set_pure_flag): Call set_pure_flag_1 on simd clones too.
>
> * gcc.c-torture/compile/pr106433.c: New test.
>
> --- gcc/cgraph.cc.jj 2023-02-02 10:54:44.327473492 +0100
> +++ gcc/cgraph.cc 2023-02-06 12:28:22.040593063 +0100
> @@ -2764,6 +2764,9 @@ set_const_flag_1 (cgraph_node *node, boo
> if (!set_const || alias->get_availability () > AVAIL_INTERPOSABLE)
> set_const_flag_1 (alias, set_const, looping, changed);
> }
> + for (struct cgraph_node *n = node->simd_clones; n != NULL;
> + n = n->simdclone->next_clone)
> + set_const_flag_1 (n, set_const, looping, changed);
> for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> if (e->caller->thunk
> && (!set_const || e->caller->get_availability () > AVAIL_INTERPOSABLE))
> @@ -2876,6 +2879,9 @@ cgraph_node::set_pure_flag (bool pure, b
> {
> struct set_pure_flag_info info = {pure, looping, false};
> call_for_symbol_thunks_and_aliases (set_pure_flag_1, &info, !pure, true);
> + for (struct cgraph_node *n = simd_clones; n != NULL;
> + n = n->simdclone->next_clone)
> + set_pure_flag_1 (n, &info);
> return info.changed;
> }
>
> --- gcc/testsuite/gcc.c-torture/compile/pr106433.c.jj 2023-02-06 12:37:26.963748811 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr106433.c 2023-02-06 12:37:06.631041918 +0100
> @@ -0,0 +1,24 @@
> +/* PR tree-optimization/106433 */
> +
> +int m, *p;
> +
> +__attribute__ ((simd)) int
> +bar (int x)
> +{
> + if (x)
> + {
> + if (m < 1)
> + for (m = 0; m < 1; ++m)
> + ++x;
> + p = &x;
> + for (;;)
> + ++m;
> + }
> + return 0;
> +}
> +
> +__attribute__ ((simd)) int
> +foo (int x)
> +{
> + return bar (x);
> +}
>
> Jakub
>
next prev parent reply other threads:[~2023-02-07 9:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-07 8:36 Jakub Jelinek
2023-02-07 9:28 ` Richard Biener [this message]
2023-02-08 17:10 ` Jan Hubicka
2023-02-08 17:14 ` Jakub Jelinek
2023-02-08 17:27 ` Jan Hubicka
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=86AB6E8B-6B1A-4A1A-96E0-9A4AB533ACD3@suse.de \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jh@suse.cz \
/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).