* [PATCH] i386: Fix ix86_add_reg_usage_to_vzeroupper [PR94308]
@ 2020-03-25 8:04 Jakub Jelinek
2020-03-25 10:30 ` Uros Bizjak
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2020-03-25 8:04 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches
Hi!
The following patch ICEs due to my recent change r10-6451-gb7b3378f91c.
Since that patch, for explicit vzeroupper in the sources (when an intrinsic
is used), we start with the *avx_vzeroupper_1 pattern which contains just the
UNSPECV_VZEROUPPER and no sets/clobbers. The vzeroupper pass then adds some
sets to those, but doesn't add clobbers and finally there is an
&& epilogue_completed splitter that splits this into the *avx_vzeroupper
pattern which has the right number of sets/clobbers (16 on 64-bit, 8 on
32-bit) + the UNSPECV_VZEROUPPER first.
The problem with this testcase on !TARGET_64BIT is that the vzeroupper pass
adds 8 sets to the pattern, i.e. the maximum number, but INSN_CODE stays
to be the one of the *avx_vzeroupper_1 pattern. The splitter doesn't do
anything here, because it sees the number of rtxes in the PARALLEL already
the right count, but during final we see that the *avx_vzeroupper_1 pattern
has "#" output template and ICE that we forgot to split it.
The following patch fixes it by forcing re-recognition of the insn after we
make the changes to it in ix86_add_reg_usage_to_vzeroupper. Anything that
will call recog_memoized later on will recog it and find out it is in this
case already *avx_vzeroupper rather than *avx_vzeroupper_1.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2020-03-25 Jakub Jelinek <jakub@redhat.com>
PR target/94308
* config/i386/i386-features.c (ix86_add_reg_usage_to_vzeroupper): Set
INSN_CODE (insn) to -1 when changing the pattern.
* gcc.target/i386/pr94308.c: New test.
--- gcc/config/i386/i386-features.c.jj 2020-03-17 13:50:52.955933209 +0100
+++ gcc/config/i386/i386-features.c 2020-03-24 19:19:17.801609289 +0100
@@ -1792,6 +1792,7 @@ ix86_add_reg_usage_to_vzeroupper (rtx_in
RTVEC_ELT (vec, j) = gen_rtx_SET (reg, reg);
}
XVEC (pattern, 0) = vec;
+ INSN_CODE (insn) = -1;
df_insn_rescan (insn);
}
--- gcc/testsuite/gcc.target/i386/pr94308.c.jj 2020-03-24 19:32:51.964436310 +0100
+++ gcc/testsuite/gcc.target/i386/pr94308.c 2020-03-24 19:32:39.848617482 +0100
@@ -0,0 +1,31 @@
+/* PR target/94308 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mfpmath=sse -mavx2 -mfma" } */
+
+#include <x86intrin.h>
+
+void
+foo (float *x, const float *y, const float *z, unsigned int w)
+{
+ unsigned int a;
+ const unsigned int b = w / 8;
+ const float *c = y;
+ const float *d = z;
+ __m256 e = _mm256_setzero_ps ();
+ __m256 f, g;
+ for (a = 0; a < b; a++)
+ {
+ f = _mm256_loadu_ps (c);
+ g = _mm256_loadu_ps (d);
+ c += 8;
+ d += 8;
+ e = _mm256_fmadd_ps (f, g, e);
+ }
+ __attribute__ ((aligned (32))) float h[8];
+ _mm256_storeu_ps (h, e);
+ _mm256_zeroupper ();
+ float i = h[0] + h[1] + h[2] + h[3] + h[4] + h[5] + h[6] + h[7];
+ for (a = b * 8; a < w; a++)
+ i += (*c++) * (*d++);
+ *x = i;
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] i386: Fix ix86_add_reg_usage_to_vzeroupper [PR94308]
2020-03-25 8:04 [PATCH] i386: Fix ix86_add_reg_usage_to_vzeroupper [PR94308] Jakub Jelinek
@ 2020-03-25 10:30 ` Uros Bizjak
0 siblings, 0 replies; 2+ messages in thread
From: Uros Bizjak @ 2020-03-25 10:30 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Wed, Mar 25, 2020 at 9:05 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following patch ICEs due to my recent change r10-6451-gb7b3378f91c.
> Since that patch, for explicit vzeroupper in the sources (when an intrinsic
> is used), we start with the *avx_vzeroupper_1 pattern which contains just the
> UNSPECV_VZEROUPPER and no sets/clobbers. The vzeroupper pass then adds some
> sets to those, but doesn't add clobbers and finally there is an
> && epilogue_completed splitter that splits this into the *avx_vzeroupper
> pattern which has the right number of sets/clobbers (16 on 64-bit, 8 on
> 32-bit) + the UNSPECV_VZEROUPPER first.
> The problem with this testcase on !TARGET_64BIT is that the vzeroupper pass
> adds 8 sets to the pattern, i.e. the maximum number, but INSN_CODE stays
> to be the one of the *avx_vzeroupper_1 pattern. The splitter doesn't do
> anything here, because it sees the number of rtxes in the PARALLEL already
> the right count, but during final we see that the *avx_vzeroupper_1 pattern
> has "#" output template and ICE that we forgot to split it.
>
> The following patch fixes it by forcing re-recognition of the insn after we
> make the changes to it in ix86_add_reg_usage_to_vzeroupper. Anything that
> will call recog_memoized later on will recog it and find out it is in this
> case already *avx_vzeroupper rather than *avx_vzeroupper_1.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-03-25 Jakub Jelinek <jakub@redhat.com>
>
> PR target/94308
> * config/i386/i386-features.c (ix86_add_reg_usage_to_vzeroupper): Set
> INSN_CODE (insn) to -1 when changing the pattern.
>
> * gcc.target/i386/pr94308.c: New test.
OK.
Thanks,
Uros.
>
> --- gcc/config/i386/i386-features.c.jj 2020-03-17 13:50:52.955933209 +0100
> +++ gcc/config/i386/i386-features.c 2020-03-24 19:19:17.801609289 +0100
> @@ -1792,6 +1792,7 @@ ix86_add_reg_usage_to_vzeroupper (rtx_in
> RTVEC_ELT (vec, j) = gen_rtx_SET (reg, reg);
> }
> XVEC (pattern, 0) = vec;
> + INSN_CODE (insn) = -1;
> df_insn_rescan (insn);
> }
>
> --- gcc/testsuite/gcc.target/i386/pr94308.c.jj 2020-03-24 19:32:51.964436310 +0100
> +++ gcc/testsuite/gcc.target/i386/pr94308.c 2020-03-24 19:32:39.848617482 +0100
> @@ -0,0 +1,31 @@
> +/* PR target/94308 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mfpmath=sse -mavx2 -mfma" } */
> +
> +#include <x86intrin.h>
> +
> +void
> +foo (float *x, const float *y, const float *z, unsigned int w)
> +{
> + unsigned int a;
> + const unsigned int b = w / 8;
> + const float *c = y;
> + const float *d = z;
> + __m256 e = _mm256_setzero_ps ();
> + __m256 f, g;
> + for (a = 0; a < b; a++)
> + {
> + f = _mm256_loadu_ps (c);
> + g = _mm256_loadu_ps (d);
> + c += 8;
> + d += 8;
> + e = _mm256_fmadd_ps (f, g, e);
> + }
> + __attribute__ ((aligned (32))) float h[8];
> + _mm256_storeu_ps (h, e);
> + _mm256_zeroupper ();
> + float i = h[0] + h[1] + h[2] + h[3] + h[4] + h[5] + h[6] + h[7];
> + for (a = b * 8; a < w; a++)
> + i += (*c++) * (*d++);
> + *x = i;
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-03-25 10:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 8:04 [PATCH] i386: Fix ix86_add_reg_usage_to_vzeroupper [PR94308] Jakub Jelinek
2020-03-25 10:30 ` Uros Bizjak
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).