On Sat, Jun 7, 2025 at 9:32 PM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Fri, Jun 6, 2025 at 12:02 PM Andrew Pinski <pins...@gmail.com> wrote:
> >
> > On Thu, Jun 5, 2025 at 11:39 PM Richard Biener
> > <richard.guent...@gmail.com> wrote:
> > >
> > > On Fri, Jun 6, 2025 at 12:14 AM Andrew Pinski <quic_apin...@quicinc.com>
> > > wrote:
> > > >
> > > > Currently we expand `{}` and store zeros to the stack and then do a full
> > > > mode load back. This is a waste, instead we should just use the zero cst
> > > > in the mode that fits the bitsize.
> > > >
> > > > Boostrapped and tested on x86_64-linux-gnu.
> > > > Build and tested for aarch64-linux-gnu.
> > >
> > > Huh, why can't we do this from expand_normal ({})?
> >
> > So the problem is the mode in this case is BLKmode as the type is
> > `char[3]`. (maybe size of 5/6/7 will cause issues too).
> > Let me see if I could move this to expand_normal, I am not 100% sure
> > if the mode mismatch will cause issues elsewhere though; here it is
> > safe though.
>
> I tried that https://gcc.gnu.org/pipermail/gcc-patches/2025-June/686055.html
> and it worked for x86_64-linux-gnu just fine but for arm-linux-gnu it
> seems like there are regressions:
> https://ci.linaro.org/job/tcwg_gcc_check--master-arm-precommit/12299/artifact/artifacts/artifacts.precommit/notify/mail-body.txt
>
> Looks like there is an issue with const zero in this case due to
> constants having VOIDmode and trying to do a subreg of that does not
> work.
> So currently only the above patch is the one which seems to work always.
Hmm. So how about catching this a bit earlier and use clear_storage (like
expand_constructor would if given a target)? That seems to be better
targeted than eventually block-moving from zero?
Richard.
> Thanks,
> Andrew
>
> >
> > Thanks,
> > Andrew Pinski
> >
> > >
> > > > PR middle-end/110459
> > > > gcc/ChangeLog:
> > > >
> > > > * expr.cc (store_field): For `{}` exp where bitsize is known
> > > > to be less than BITS_PER_WORD, use zero cst.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * g++.target/aarch64/array-return-1.C: New test.
> > > > * g++.target/i386/array-return-1.C: New test.
> > > >
> > > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > > > ---
> > > > gcc/expr.cc | 10 ++++++++-
> > > > .../g++.target/aarch64/array-return-1.C | 22 +++++++++++++++++++
> > > > .../g++.target/i386/array-return-1.C | 20 +++++++++++++++++
> > > > 3 files changed, 51 insertions(+), 1 deletion(-)
> > > > create mode 100644 gcc/testsuite/g++.target/aarch64/array-return-1.C
> > > > create mode 100644 gcc/testsuite/g++.target/i386/array-return-1.C
> > > >
> > > > diff --git a/gcc/expr.cc b/gcc/expr.cc
> > > > index 1eeefa1cadc..af03809cad8 100644
> > > > --- a/gcc/expr.cc
> > > > +++ b/gcc/expr.cc
> > > > @@ -8243,7 +8243,15 @@ store_field (rtx target, poly_int64 bitsize,
> > > > poly_int64 bitpos,
> > > > }
> > > > }
> > > >
> > > > - temp = expand_normal (exp);
> > > > + if (TREE_CODE (exp) == CONSTRUCTOR
> > > > + && CONSTRUCTOR_NELTS (exp) == 0
> > > > + && known_le (bitsize, BITS_PER_WORD))
> > > > + {
> > > > + machine_mode temp_mode = smallest_int_mode_for_size
> > > > (bitsize).require ();
> > > > + temp = CONST0_RTX (temp_mode);
> > > > + }
> > > > + else
> > > > + temp = expand_normal (exp);
> > > >
> > > > /* We don't support variable-sized BLKmode bitfields, since our
> > > > handling of BLKmode is bound up with the ability to break
> > > > diff --git a/gcc/testsuite/g++.target/aarch64/array-return-1.C
> > > > b/gcc/testsuite/g++.target/aarch64/array-return-1.C
> > > > new file mode 100644
> > > > index 00000000000..7c0aa480775
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.target/aarch64/array-return-1.C
> > > > @@ -0,0 +1,22 @@
> > > > +// { dg-do compile }
> > > > +// { dg-options "-O2" }
> > > > +// { dg-final { check-function-bodies "**" "" "" { target { le } } } }
> > > > +
> > > > +// PR middle-end/110459
> > > > +
> > > > +/*
> > > > +**_Z7sample2c:
> > > > +** and w0, w0, 255
> > > > +** ret
> > > > +*/
> > > > +
> > > > +struct array {
> > > > + char data[4];
> > > > +};
> > > > +
> > > > +// There should be no adjustment to the stack
> > > > +// { dg-final { scan-assembler-not "sp, sp," } }
> > > > +auto sample2(char c) {
> > > > + array buffer = {c, 0, 0, 0};
> > > > + return buffer;
> > > > +}
> > > > diff --git a/gcc/testsuite/g++.target/i386/array-return-1.C
> > > > b/gcc/testsuite/g++.target/i386/array-return-1.C
> > > > new file mode 100644
> > > > index 00000000000..f11905d01e5
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.target/i386/array-return-1.C
> > > > @@ -0,0 +1,20 @@
> > > > +// { dg-do compile }
> > > > +// { dg-options "-O2" }
> > > > +// { dg-final { check-function-bodies "**" "" "" { target { ! ia32 } }
> > > > } }
> > > > +
> > > > +// PR middle-end/110459
> > > > +
> > > > +/*
> > > > +**_Z7sample2c:
> > > > +** movzbl %dil, %eax
> > > > +** ret
> > > > +*/
> > > > +
> > > > +struct array {
> > > > + char data[4];
> > > > +};
> > > > +
> > > > +array sample2(char c) {
> > > > + array buffer = {c, 0, 0, 0};
> > > > + return buffer;
> > > > +}
> > > > --
> > > > 2.43.0
> > > >
