On Thu, Feb 17, 2022 at 9:56 PM H.J. Lu <hjl.to...@gmail.com> wrote: > > On Thu, Feb 17, 2022 at 08:51:31AM +0100, Uros Bizjak wrote: > > On Thu, Feb 17, 2022 at 6:25 AM Hongtao Liu via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > On Thu, Feb 17, 2022 at 12:26 PM H.J. Lu via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > Reading YMM registers with all zero bits needs VZEROUPPER on Sandy > > > > Bride, > > > > Ivy Bridge, Haswell, Broadwell and Alder Lake to avoid SSE <-> AVX > > > > transition penalty. Add TARGET_READ_ZERO_YMM_ZMM_NEED_VZEROUPPER to > > > > generate vzeroupper instruction after loading all-zero YMM/YMM registers > > > > and enable it by default. > > > Shouldn't TARGET_READ_ZERO_YMM_ZMM_NONEED_VZEROUPPER sounds a bit > > > smoother? > > > Because originally we needed to add vzeroupper to all avx<->sse cases, > > > now it's a tune to indicate that we don't need to add it in some > > > > Perhaps we should go from the other side and use > > X86_TUNE_OPTIMIZE_AVX_READ for new processors? > > > > Here is the v2 patch to add TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO. > The patch LGTM in general, but please rebase against https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590541.html and resend the patch, also wait a couple days in case Uros(and others) have any comments. > > H.J. > --- > Reading YMM registers with all zero bits needs VZEROUPPER on Sandy Bride, > Ivy Bridge, Haswell, Broadwell and Alder Lake to avoid SSE <-> AVX > transition penalty. Add TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO to > omit vzeroupper instruction after loading all-zero YMM/ZMM registers. > > gcc/ > > PR target/101456 > * config/i386/i386.cc (ix86_avx_u128_mode_needed): Omit > vzeroupper after reading all-zero YMM/ZMM registers for > TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO. > * config/i386/i386.h (TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO): > New. > * config/i386/x86-tune.def > (X86_TUNE_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO): New. > > gcc/testsuite/ > > PR target/101456 > * gcc.target/i386/pr101456-1.c (dg-options): Add > -mtune-ctrl=-mtune-ctrl=omit_vzeroupper_after_avx_read_zero. > * gcc.target/i386/pr101456-2.c: Likewise. > * gcc.target/i386/pr101456-3.c: New test. > * gcc.target/i386/pr101456-4.c: Likewise. > --- > gcc/config/i386/i386.cc | 51 ++++++++++++---------- > gcc/config/i386/i386.h | 2 + > gcc/config/i386/x86-tune.def | 5 +++ > gcc/testsuite/gcc.target/i386/pr101456-1.c | 2 +- > gcc/testsuite/gcc.target/i386/pr101456-2.c | 2 +- > gcc/testsuite/gcc.target/i386/pr101456-3.c | 33 ++++++++++++++ > gcc/testsuite/gcc.target/i386/pr101456-4.c | 33 ++++++++++++++ > 7 files changed, 103 insertions(+), 25 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr101456-3.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr101456-4.c > > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > index cf246e74e57..60c72ceb72d 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -14502,33 +14502,38 @@ ix86_avx_u128_mode_needed (rtx_insn *insn) > > subrtx_iterator::array_type array; > > - rtx set = single_set (insn); > - if (set) > + if (TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO) > { > - rtx dest = SET_DEST (set); > - rtx src = SET_SRC (set); > - if (ix86_check_avx_upper_register (dest)) > + /* Perform this vzeroupper optimization if target doesn't need > + vzeroupper after reading all-zero YMM/YMM registers. */ > + rtx set = single_set (insn); > + if (set) > { > - /* This is an YMM/ZMM load. Return AVX_U128_DIRTY if the > - source isn't zero. */ > - if (standard_sse_constant_p (src, GET_MODE (dest)) != 1) > - return AVX_U128_DIRTY; > + rtx dest = SET_DEST (set); > + rtx src = SET_SRC (set); > + if (ix86_check_avx_upper_register (dest)) > + { > + /* This is an YMM/ZMM load. Return AVX_U128_DIRTY if the > + source isn't zero. */ > + if (standard_sse_constant_p (src, GET_MODE (dest)) != 1) > + return AVX_U128_DIRTY; > + else > + return AVX_U128_ANY; > + } > else > - return AVX_U128_ANY; > - } > - else > - { > - FOR_EACH_SUBRTX (iter, array, src, NONCONST) > - if (ix86_check_avx_upper_register (*iter)) > - { > - int status = ix86_avx_u128_mode_source (insn, *iter); > - if (status == AVX_U128_DIRTY) > - return status; > - } > - } > + { > + FOR_EACH_SUBRTX (iter, array, src, NONCONST) > + if (ix86_check_avx_upper_register (*iter)) > + { > + int status = ix86_avx_u128_mode_source (insn, *iter); > + if (status == AVX_U128_DIRTY) > + return status; > + } > + } > > - /* This isn't YMM/ZMM load/store. */ > - return AVX_U128_ANY; > + /* This isn't YMM/ZMM load/store. */ > + return AVX_U128_ANY; > + } > } > > /* Require DIRTY mode if a 256bit or 512bit AVX register is referenced. > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h > index f41e0908250..46379d2231b 100644 > --- a/gcc/config/i386/i386.h > +++ b/gcc/config/i386/i386.h > @@ -425,6 +425,8 @@ extern unsigned char ix86_tune_features[X86_TUNE_LAST]; > #define TARGET_AVOID_MFENCE ix86_tune_features[X86_TUNE_AVOID_MFENCE] > #define TARGET_EMIT_VZEROUPPER \ > ix86_tune_features[X86_TUNE_EMIT_VZEROUPPER] > +#define TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO \ > + ix86_tune_features[X86_TUNE_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO] > #define TARGET_EXPAND_ABS \ > ix86_tune_features[X86_TUNE_EXPAND_ABS] > #define TARGET_V2DF_REDUCTION_PREFER_HADDPD \ > diff --git a/gcc/config/i386/x86-tune.def b/gcc/config/i386/x86-tune.def > index 82ca0ae63ac..132de2db2eb 100644 > --- a/gcc/config/i386/x86-tune.def > +++ b/gcc/config/i386/x86-tune.def > @@ -649,3 +649,8 @@ DEF_TUNE (X86_TUNE_PROMOTE_QI_REGS, "promote_qi_regs", > m_NONE) > /* X86_TUNE_EMIT_VZEROUPPER: This enables vzeroupper instruction insertion > before a transfer of control flow out of the function. */ > DEF_TUNE (X86_TUNE_EMIT_VZEROUPPER, "emit_vzeroupper", ~m_KNL) > + > +/* X86_TUNE_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO: This omits vzeroupper > + instruction after reading all-zero YMM/ZMM registers. */ > +DEF_TUNE (X86_TUNE_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO, > + "omit_vzeroupper_after_avx_read_zero", 0) > diff --git a/gcc/testsuite/gcc.target/i386/pr101456-1.c > b/gcc/testsuite/gcc.target/i386/pr101456-1.c > index 803fc6e0207..f653197da7c 100644 > --- a/gcc/testsuite/gcc.target/i386/pr101456-1.c > +++ b/gcc/testsuite/gcc.target/i386/pr101456-1.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -march=skylake" } */ > +/* { dg-options "-O2 -march=skylake > -mtune-ctrl=omit_vzeroupper_after_avx_read_zero" } */ > > #include <x86intrin.h> > > diff --git a/gcc/testsuite/gcc.target/i386/pr101456-2.c > b/gcc/testsuite/gcc.target/i386/pr101456-2.c > index 554a0f1702c..9aac3ece14d 100644 > --- a/gcc/testsuite/gcc.target/i386/pr101456-2.c > +++ b/gcc/testsuite/gcc.target/i386/pr101456-2.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -march=skylake" } */ > +/* { dg-options "-O2 -march=skylake > -mtune-ctrl=omit_vzeroupper_after_avx_read_zero" } */ > > #include <x86intrin.h> > > diff --git a/gcc/testsuite/gcc.target/i386/pr101456-3.c > b/gcc/testsuite/gcc.target/i386/pr101456-3.c > new file mode 100644 > index 00000000000..8389d18ed6c > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr101456-3.c > @@ -0,0 +1,33 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -march=skylake -mtune=alderlake" } */ > + > +#include <x86intrin.h> > + > +extern __m256 x1; > +extern __m256d x2; > +extern __m256i x3; > + > +extern void bar (void); > + > +void > +foo1 (void) > +{ > + x1 = _mm256_setzero_ps (); > + bar (); > +} > + > +void > +foo2 (void) > +{ > + x2 = _mm256_setzero_pd (); > + bar (); > +} > + > +void > +foo3 (void) > +{ > + x3 = _mm256_setzero_si256 (); > + bar (); > +} > + > +/* { dg-final { scan-assembler-times "vzeroupper" 3 } } */ > diff --git a/gcc/testsuite/gcc.target/i386/pr101456-4.c > b/gcc/testsuite/gcc.target/i386/pr101456-4.c > new file mode 100644 > index 00000000000..3e4cdcc4d28 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr101456-4.c > @@ -0,0 +1,33 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -march=haswell" } */ > + > +#include <x86intrin.h> > + > +extern __m256 x1; > +extern __m256d x2; > +extern __m256i x3; > + > +extern void bar (void); > + > +void > +foo1 (void) > +{ > + x1 = _mm256_setzero_ps (); > + bar (); > +} > + > +void > +foo2 (void) > +{ > + x2 = _mm256_setzero_pd (); > + bar (); > +} > + > +void > +foo3 (void) > +{ > + x3 = _mm256_setzero_si256 (); > + bar (); > +} > + > +/* { dg-final { scan-assembler-times "vzeroupper" 3 } } */ > -- > 2.35.1 >
-- BR, Hongtao
