From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by sourceware.org (Postfix) with ESMTPS id E4C103858C2C for ; Mon, 21 Feb 2022 18:35:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E4C103858C2C Received: by mail-pj1-x102a.google.com with SMTP id f19-20020a17090ac29300b001bc68ecce4aso504198pjt.4 for ; Mon, 21 Feb 2022 10:35:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nJVo4AEZfm6re9+3hqYWe7o72vYC3H1eHNhoxHtwM4Q=; b=gPoJwF/fpMVt4FBzo+Fecm9g+rxm8hpSHg6PC5FBvIizhtem6MFRU/sTjniF34/y43 ndQ8EWNkXRKMNnYAgJMEBmtElWmGkfbMIpNtP4Bx9mQfq3ojqdR7yHtkb84ISmUBMVzT f7ke9YStYPh+j6fhqvRS4ZrtcW+rctViN9luXT57up6v7ZrWwpYi0rfmUauhB4PFG+Gj j0OYPZQaf/Hap2JmIzSwFZBp5LbJ8rodz7TvOU49v1+6wuEQWVmD5ugvhnRoribDn70+ VCJcgtucatGRksM+NtVAymMOmUGuQ3Vgiw09xGzpe4l3ZxeN7QVwOE72vO1KkIUrZ29Z KRVg== X-Gm-Message-State: AOAM533Dku0CoQTIxP+bY5GSjMbLb8Bgvby4Wl4mCcfmls1Mt42uiEgl NQeeDY1rC/4hmpJX0mYikMpd20225e1cbBPTySM= X-Google-Smtp-Source: ABdhPJzichKeVRGR1fHfgbz9/HcLm/L+Wm1XPhyGN3QFnv5DwoGTaKU1fNaTP3tQUrYz4Z2jqnlFFvuDKlnYCgCcgQk= X-Received: by 2002:a17:90a:4043:b0:1bc:450:df68 with SMTP id k3-20020a17090a404300b001bc0450df68mr284158pjg.120.1645468540768; Mon, 21 Feb 2022 10:35:40 -0800 (PST) MIME-Version: 1.0 References: <20220217042628.133306-1-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Mon, 21 Feb 2022 10:35:05 -0800 Message-ID: Subject: Re: [PATCH v2] x86: Add TARGET_OMIT_VZEROUPPER_AFTER_AVX_READ_ZERO To: Hongtao Liu Cc: Uros Bizjak , liuhongt , GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3027.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Feb 2022 18:35:44 -0000 On Sun, Feb 20, 2022 at 6:01 PM Hongtao Liu wrote: > > On Thu, Feb 17, 2022 at 9:56 PM H.J. Lu 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 > > > wrote: > > > > > > > > On Thu, Feb 17, 2022 at 12:26 PM H.J. Lu via Gcc-patches > > > > 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. I am dropping my patch since it causes the compile-time regression. > > > > 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 > > > > 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 > > > > 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 > > + > > +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 > > + > > +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 -- H.J.