From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 4B1EA3857343 for ; Wed, 25 May 2022 07:30:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4B1EA3857343 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E86FB1FB; Wed, 25 May 2022 00:30:07 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2920D3F70D; Wed, 25 May 2022 00:30:06 -0700 (PDT) From: Richard Sandiford To: "H.J. Lu via Gcc-patches" Mail-Followup-To: "H.J. Lu via Gcc-patches" , Richard Biener , "H.J. Lu" , richard.sandiford@arm.com Subject: Re: [PATCH v2] DSE: Use the constant store source if possible References: <20220521030120.1977551-1-hjl.tools@gmail.com> Date: Wed, 25 May 2022 08:30:05 +0100 In-Reply-To: (H. J. Lu via Gcc-patches's message of "Mon, 23 May 2022 11:34:24 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Wed, 25 May 2022 07:30:10 -0000 "H.J. Lu via Gcc-patches" writes: > On Mon, May 23, 2022 at 12:38:06PM +0200, Richard Biener wrote: >> On Sat, May 21, 2022 at 5:02 AM H.J. Lu via Gcc-patches >> wrote: >> > >> > When recording store for RTL dead store elimination, check if the source >> > register is set only once to a constant. If yes, record the constant >> > as the store source. It eliminates unrolled zero stores after memset 0 >> > in a loop where a vector register is used as the zero store source. >> > >> > gcc/ >> > >> > PR rtl-optimization/105638 >> > * dse.cc (record_store): Use the constant source if the source >> > register is set only once. >> > >> > gcc/testsuite/ >> > >> > PR rtl-optimization/105638 >> > * g++.target/i386/pr105638.C: New test. >> > --- >> > gcc/dse.cc | 19 ++++++++++ >> > gcc/testsuite/g++.target/i386/pr105638.C | 44 ++++++++++++++++++++++++ >> > 2 files changed, 63 insertions(+) >> > create mode 100644 gcc/testsuite/g++.target/i386/pr105638.C >> > >> > diff --git a/gcc/dse.cc b/gcc/dse.cc >> > index 30c11cee034..0433dd3d846 100644 >> > --- a/gcc/dse.cc >> > +++ b/gcc/dse.cc >> > @@ -1508,6 +1508,25 @@ record_store (rtx body, bb_info_t bb_info) >> > >> > if (tem && CONSTANT_P (tem)) >> > const_rhs = tem; >> > + else >> > + { >> > + /* If RHS is set only once to a constant, set CONST_RHS >> > + to the constant. */ >> > + df_ref def = DF_REG_DEF_CHAIN (REGNO (rhs)); >> > + if (def != nullptr >> > + && !DF_REF_IS_ARTIFICIAL (def) >> > + && !DF_REF_NEXT_REG (def)) >> > + { >> > + rtx_insn *def_insn = DF_REF_INSN (def); >> > + rtx def_body = PATTERN (def_insn); >> > + if (GET_CODE (def_body) == SET) >> > + { >> > + rtx def_src = SET_SRC (def_body); >> > + if (CONSTANT_P (def_src)) >> > + const_rhs = def_src; >> >> doesn't DSE have its own tracking of stored values? Shouldn't we > > It tracks stored values only within the basic block. When RTL loop > invariant motion hoists a constant initialization out of the loop into > a separate basic block, the constant store value becomes unknown > within the original basic block. > >> improve _that_ if it is not enough? I also wonder if you need to > > My patch extends DSE stored value tracking to include the constant which > is set only once in another basic block. > >> verify the SET isn't partial? >> > > Here is the v2 patch to check that the constant is set by a non-partial > unconditional load. > > OK for master? > > Thanks. > > H.J. > --- > RTL DSE tracks redundant constant stores within a basic block. When RTL > loop invariant motion hoists a constant initialization out of the loop > into a separate basic block, the constant store value becomes unknown > within the original basic block. When recording store for RTL DSE, check > if the source register is set only once to a constant by a non-partial > unconditional load. If yes, record the constant as the constant store > source. It eliminates unrolled zero stores after memset 0 in a loop > where a vector register is used as the zero store source. > > gcc/ > > PR rtl-optimization/105638 > * dse.cc (record_store): Use the constant source if the source > register is set only once. > > gcc/testsuite/ > > PR rtl-optimization/105638 > * g++.target/i386/pr105638.C: New test. > --- > gcc/dse.cc | 22 ++++++++++++ > gcc/testsuite/g++.target/i386/pr105638.C | 44 ++++++++++++++++++++++++ > 2 files changed, 66 insertions(+) > create mode 100644 gcc/testsuite/g++.target/i386/pr105638.C > > diff --git a/gcc/dse.cc b/gcc/dse.cc > index 30c11cee034..af8e88dac32 100644 > --- a/gcc/dse.cc > +++ b/gcc/dse.cc > @@ -1508,6 +1508,28 @@ record_store (rtx body, bb_info_t bb_info) > > if (tem && CONSTANT_P (tem)) > const_rhs = tem; > + else > + { > + /* If RHS is set only once to a constant, set CONST_RHS > + to the constant. */ > + df_ref def = DF_REG_DEF_CHAIN (REGNO (rhs)); > + if (def != nullptr > + && !DF_REF_IS_ARTIFICIAL (def) > + && !(DF_REF_FLAGS (def) > + & (DF_REF_PARTIAL | DF_REF_CONDITIONAL)) > + && !DF_REF_NEXT_REG (def)) Can we introduce a helper for this? There are already similar tests in ira and loop-iv, and it seems a bit too complex to have to open-code each time. Thanks, Richard > + { > + rtx_insn *def_insn = DF_REF_INSN (def); > + rtx def_body = PATTERN (def_insn); > + if (GET_CODE (def_body) == SET) > + { > + rtx def_src = SET_SRC (def_body); > + if (CONSTANT_P (def_src) > + && GET_MODE (def_src) == GET_MODE (rhs)) > + const_rhs = def_src; > + } > + } > + } > } > } > > diff --git a/gcc/testsuite/g++.target/i386/pr105638.C b/gcc/testsuite/g++.target/i386/pr105638.C > new file mode 100644 > index 00000000000..ff40a459de1 > --- /dev/null > +++ b/gcc/testsuite/g++.target/i386/pr105638.C > @@ -0,0 +1,44 @@ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-options "-std=gnu++20 -O2 -march=skylake" } */ > +/* { dg-final { scan-assembler-not "vpxor" } } */ > + > +#include > +#include > +#include > + > +class FastBoard { > +public: > + typedef std::pair movescore_t; > + typedef std::tr1::array scoredlist_t; > + > +protected: > + std::vector m_critical; > + > + int m_boardsize; > +}; > + > +class FastState { > +public: > + FastBoard board; > + > + int movenum; > +protected: > + FastBoard::scoredlist_t scoredmoves; > +}; > + > +class KoState : public FastState { > +private: > + std::vector ko_hash_history; > + std::vector hash_history; > +}; > + > +class GameState : public KoState { > +public: > + void foo (); > +private: > + std::vector game_history; > +}; > + > +void GameState::foo() { > + game_history.resize(movenum); > +}