Re: [PATCH v2] DSE: Use the constant store source if possible
"H.J. Lu" writes:
> On Wed, May 25, 2022 at 12:30 AM Richard Sandiford
> wrote:
>>
>> "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
Re: [PATCH v2] DSE: Use the constant store source if possible
On Wed, May 25, 2022 at 2:30 AM Richard Sandiford
wrote:
>
> Richard Biener via Gcc-patches writes:
> > On Tue, May 24, 2022 at 10:11 PM H.J. Lu wrote:
> >>
> >> On Mon, May 23, 2022 at 11:42 PM Richard Biener
> >> wrote:
> >> >
> >> > On Mon, May 23, 2022 at 8:34 PM H.J. Lu wrote:
> >> > >
> >> > > 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
Re: [PATCH v2] DSE: Use the constant store source if possible
On Wed, May 25, 2022 at 12:30 AM Richard Sandiford
wrote:
>
> "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.
I can use
Re: [PATCH v2] DSE: Use the constant store source if possible
Richard Biener via Gcc-patches writes:
> On Tue, May 24, 2022 at 10:11 PM H.J. Lu wrote:
>>
>> On Mon, May 23, 2022 at 11:42 PM Richard Biener
>> wrote:
>> >
>> > On Mon, May 23, 2022 at 8:34 PM H.J. Lu wrote:
>> > >
>> > > 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
>> > > + {
>> > > + /*
Re: [PATCH v2] DSE: Use the constant store source if possible
On Tue, May 24, 2022 at 10:11 PM H.J. Lu wrote:
>
> On Mon, May 23, 2022 at 11:42 PM Richard Biener
> wrote:
> >
> > On Mon, May 23, 2022 at 8:34 PM H.J. Lu wrote:
> > >
> > > 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));
> > > +
Re: [PATCH v2] DSE: Use the constant store source if possible
"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
Re: [PATCH v2] DSE: Use the constant store source if possible
On Mon, May 23, 2022 at 11:42 PM Richard Biener
wrote:
>
> On Mon, May 23, 2022 at 8:34 PM H.J. Lu wrote:
> >
> > 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 really use df-chain here and rely that a single definition is
>
Re: [PATCH v2] DSE: Use the constant store source if possible
On Mon, May 23, 2022 at 8:34 PM H.J. Lu wrote:
>
> 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 really use df-chain here and rely that a single definition is
the only one? If rhs is a hardreg does df-chain include implicit
sets of function argument registers for example? Don't we need RD
here or at least verify the single df-chain definition dominates the
use here (if we can rely on the reg otherwise be uninitialized and thus
the use invoking undefined
[PATCH v2] DSE: Use the constant store source if possible
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))
+ {
+ 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 000..ff40a459de1
