From: Jakub Jelinek <jakub@redhat.com>
To: Uros Bizjak <ubizjak@gmail.com>,
Kirill Yukhin <kirill.yukhin@gmail.com>,
Marek Polacek <polacek@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] FALLTHRU markers for sse.md and some cleanups
Date: Fri, 30 Sep 2016 21:31:00 -0000 [thread overview]
Message-ID: <20160930201750.GI7282@tucnak.redhat.com> (raw)
In-Reply-To: <20160929162113.GU3223@redhat.com>
On Thu, Sep 29, 2016 at 06:21:13PM +0200, Marek Polacek wrote:
> On Tue, Sep 27, 2016 at 09:58:20PM +0200, Jakub Jelinek wrote:
> > On Tue, Sep 27, 2016 at 09:29:10PM +0200, Florian Weimer wrote:
> > > Not sure if I read this code correctly, but if we fall through from
> > > V32HImode, and we have TARGET_SSE2 set, we execute this code:
> > >
> > > tmp = "p<logic>";
> > > ssesuffix = TARGET_AVX512VL ? "q" : "";
> > >
> > > And not gcc_unreachable (), as is probably intended.
> >
> > It really doesn't matter.
> > The instruction uses
> > (define_mode_iterator VI12_AVX_AVX512F
> > [ (V64QI "TARGET_AVX512F") (V32QI "TARGET_AVX") V16QI
> > (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX") V8HI])
> > iterator (and, after all, ix86_hard_regno_mode_ok should ensure the same),
> > which means V64QI/V32HI will only show up for TARGET_AVX512F, V32QI/V16HI
> > only for TARGET_AVX (which implies TARGET_SSE2), and the slightly
> > nonsensical
> > gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
> > before the switch (the || TARGET_AVX512VL is pointless, because
> > TARGET_AVX512VL implies TARGET_SSE2 as well as TARGET_AVX2).
> > So, I'd go perhaps for (untested) following patch, first diff -up, followed
> > by diff -upb:
>
> Looks good, are you going to test/commit it? Or should I?
Here it is, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2016-09-30 Jakub Jelinek <jakub@redhat.com>
* config/i386/sse.md (<mask_codefor><code><mode>): Add FALLTHRU
comments. Simplify asserts, remove unnecessary conditions.
Formatting fixes.
(*<code><mode>3): Likewise.
--- gcc/config/i386/sse.md.jj 2016-08-30 08:42:09.169067639 +0200
+++ gcc/config/i386/sse.md 2016-09-30 14:40:45.382959729 +0200
@@ -11393,41 +11393,40 @@ (define_insn "<mask_codefor><code><mode>
{
case MODE_XI:
gcc_assert (TARGET_AVX512F);
+ /* FALLTHRU */
case MODE_OI:
- gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+ gcc_assert (TARGET_AVX2);
+ /* FALLTHRU */
case MODE_TI:
- gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
+ gcc_assert (TARGET_SSE2);
switch (<MODE>mode)
- {
- case V16SImode:
- case V8DImode:
- if (TARGET_AVX512F)
- {
- tmp = "p<logic><ssemodesuffix>";
- break;
- }
- case V8SImode:
- case V4DImode:
- case V4SImode:
- case V2DImode:
- tmp = TARGET_AVX512VL ? "p<logic><ssemodesuffix>" : "p<logic>";
- break;
- default:
- gcc_unreachable ();
- }
+ {
+ case V16SImode:
+ case V8DImode:
+ tmp = "p<logic><ssemodesuffix>";
+ break;
+ case V8SImode:
+ case V4DImode:
+ case V4SImode:
+ case V2DImode:
+ tmp = TARGET_AVX512VL ? "p<logic><ssemodesuffix>" : "p<logic>";
+ break;
+ default:
+ gcc_unreachable ();
+ }
break;
- case MODE_V8SF:
+ case MODE_V8SF:
gcc_assert (TARGET_AVX);
- case MODE_V4SF:
+ case MODE_V4SF:
gcc_assert (TARGET_SSE);
gcc_assert (!<mask_applied>);
tmp = "<logic>ps";
break;
- default:
+ default:
gcc_unreachable ();
- }
+ }
switch (which_alternative)
{
@@ -11489,46 +11488,42 @@ (define_insn "*<code><mode>3"
{
case MODE_XI:
gcc_assert (TARGET_AVX512F);
+ /* FALLTHRU */
case MODE_OI:
- gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+ gcc_assert (TARGET_AVX2);
+ /* FALLTHRU */
case MODE_TI:
- gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
+ gcc_assert (TARGET_SSE2);
switch (<MODE>mode)
- {
- case V64QImode:
- case V32HImode:
- if (TARGET_AVX512F)
- {
- tmp = "p<logic>";
- ssesuffix = "q";
- break;
- }
- case V32QImode:
- case V16HImode:
- case V16QImode:
- case V8HImode:
- if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
- {
- tmp = "p<logic>";
- ssesuffix = TARGET_AVX512VL ? "q" : "";
- break;
- }
- default:
- gcc_unreachable ();
- }
+ {
+ case V64QImode:
+ case V32HImode:
+ tmp = "p<logic>";
+ ssesuffix = "q";
+ break;
+ case V32QImode:
+ case V16HImode:
+ case V16QImode:
+ case V8HImode:
+ tmp = "p<logic>";
+ ssesuffix = TARGET_AVX512VL ? "q" : "";
+ break;
+ default:
+ gcc_unreachable ();
+ }
break;
- case MODE_V8SF:
+ case MODE_V8SF:
gcc_assert (TARGET_AVX);
- case MODE_V4SF:
+ case MODE_V4SF:
gcc_assert (TARGET_SSE);
tmp = "<logic>ps";
ssesuffix = "";
break;
- default:
+ default:
gcc_unreachable ();
- }
+ }
switch (which_alternative)
{
Jakub
next prev parent reply other threads:[~2016-09-30 20:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-27 16:55 PATCH to add more FALLTHRU markers Marek Polacek
2016-09-27 19:00 ` Richard Sandiford
2016-09-29 17:22 ` Marek Polacek
2016-09-27 19:08 ` Florian Weimer
2016-09-27 19:22 ` Marek Polacek
2016-09-27 19:33 ` Florian Weimer
2016-09-27 19:39 ` Marek Polacek
2016-09-27 20:13 ` Jakub Jelinek
2016-09-29 17:17 ` Marek Polacek
2016-09-29 20:21 ` Jakub Jelinek
2016-09-30 21:31 ` Jakub Jelinek [this message]
2016-10-01 8:31 ` [PATCH] FALLTHRU markers for sse.md and some cleanups Uros Bizjak
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=20160930201750.GI7282@tucnak.redhat.com \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=kirill.yukhin@gmail.com \
--cc=polacek@redhat.com \
--cc=ubizjak@gmail.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).