Re: [PATCH] avoid-store-forwarding: Reject BLKmode memory operands [PR126686]

On Thu, Aug 20, 2026 at 11:25 AM Andrea Pinski
<[email protected]> wrote:
>
> On Thu, Aug 20, 2026 at 7:07 AM Konstantinos Eleftheriou
> <[email protected]> wrote:
> >
> > Memory operands were only rejected when their size is unknown or
> > non-constant, so a BLKmode store with a known size could become a
> > forwarding candidate. Building the bit insert sequence for it asserts
> > in store_bit_field, as BLKmode has no corresponding integer mode.
> > Reject such memory when collecting candidates.
> >
> >         PR rtl-optimization/126686
> >
> > gcc/ChangeLog:
> >
> >         * avoid-store-forwarding.cc
> >         (store_forwarding_analyzer::avoid_store_forwarding): Reject
> >         BLKmode stores and loads as forwarding candidates.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/s390/pr126686.c: New test.
> > ---
> >  gcc/avoid-store-forwarding.cc            | 10 +++++++---
> >  gcc/testsuite/gcc.target/s390/pr126686.c | 16 ++++++++++++++++
> >  2 files changed, 23 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/s390/pr126686.c
> >
> > diff --git a/gcc/avoid-store-forwarding.cc b/gcc/avoid-store-forwarding.cc
> > index 141f96c3d848..99feebafe126 100644
> > --- a/gcc/avoid-store-forwarding.cc
> > +++ b/gcc/avoid-store-forwarding.cc
> > @@ -565,11 +565,15 @@ store_forwarding_analyzer::avoid_store_forwarding (basic_block bb)
> >        /* The mem RTX if INSN is a store, NULL_RTX otherwise.  */
> >        rtx store_mem = MEM_P (SET_DEST (set)) ? SET_DEST (set) : NULL_RTX;
> >
> > -      /* We cannot analyze memory RTXs that have unknown size. */
> > +      /* We cannot analyze memory RTXs that have unknown size.  BLKmode
> > +        memory is rejected as well, as there is no mode for the forwarded
> > +        value, even when its size is known.  */
> >        if ((store_mem && (!MEM_SIZE_KNOWN_P (store_mem)
> > -                        || !MEM_SIZE (store_mem).is_constant ()))
> > +                        || !MEM_SIZE (store_mem).is_constant ()
> > +                        || GET_MODE (store_mem) == BLKmode))
> >           || (load_mem && (!MEM_SIZE_KNOWN_P (load_mem)
> > -                          || !MEM_SIZE (load_mem).is_constant ())))
> > +                          || !MEM_SIZE (load_mem).is_constant ()
> > +                          || GET_MODE (load_mem) == BLKmode)))
>
> Can you place the GET_MODE check first in the list since it should be
> the cheapest test?
> Otherwise ok.
>
>
> >         {
> >           store_exprs.truncate (0);
> >           continue;
> > diff --git a/gcc/testsuite/gcc.target/s390/pr126686.c b/gcc/testsuite/gcc.target/s390/pr126686.c
> > new file mode 100644
> > index 000000000000..ab25d109ec35
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/s390/pr126686.c
> > @@ -0,0 +1,16 @@
> > +/* PR rtl-optimization/126686 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O3 -favoid-store-forwarding" } */

Actually since the teststcase has no s390 specific thing in it should
just be added to gcc.dg instead.
Even gcc.dg/torture using `{ dg-additional-options
"-favoid-store-forwarding" }`.

> > +
> > +int tmp;
> > +short d_e;
> > +
> > +int
> > +foo ()
> > +{
> > +  long f = 0;
> > +  __builtin_memset ((char *) &f + sizeof f - 2, d_e, 2);
> > +  tmp = f;
> > +
> > +  return f;
> > +}
> > --
> > 2.55.0
> >