On Thu, Jul 3, 2025 at 11:02 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> "H.J. Lu" <hjl.to...@gmail.com> writes:
> > Since a backend may ignore user type alignment for arguments passed on
> > stack, update alignment for arguments passed on stack when copying MEM's
> > memory attributes.
> >
> > gcc/
> >
> > PR target/120839
> > * emit-rtl.cc (set_mem_attrs): Update alignment for argument on
> > stack.
> >
> > gcc/testsuite/
> >
> > PR target/120839
> > * gcc.target/i386/pr120839-1.c: New test.
> > * gcc.target/i386/pr120839-2.c: Likewise.
> >
> >
> > --
> > H.J.
> >
> > From 3f8a9bfb4beae47bfc0da20b517a5b3b06a1cbcc Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.to...@gmail.com>
> > Date: Sat, 28 Jun 2025 06:27:25 +0800
> > Subject: [PATCH] Update alignment for argument on stack
> >
> > Since a backend may ignore user type alignment for arguments passed on
> > stack, update alignment for arguments passed on stack when copying MEM's
> > memory attributes.
> >
> > gcc/
> >
> > PR target/120839
> > * emit-rtl.cc (set_mem_attrs): Update alignment for argument on
> > stack.
> >
> > gcc/testsuite/
> >
> > PR target/120839
> > * gcc.target/i386/pr120839-1.c: New test.
> > * gcc.target/i386/pr120839-2.c: Likewise.
> >
> > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > ---
> > gcc/emit-rtl.cc | 14 ++++++++++++++
> > gcc/testsuite/gcc.target/i386/pr120839-1.c | 14 ++++++++++++++
> > gcc/testsuite/gcc.target/i386/pr120839-2.c | 19 +++++++++++++++++++
> > 3 files changed, 47 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr120839-1.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr120839-2.c
> >
> > diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
> > index f4fc92bb37a..0d1616361ca 100644
> > --- a/gcc/emit-rtl.cc
> > +++ b/gcc/emit-rtl.cc
> > @@ -389,6 +389,20 @@ set_mem_attrs (rtx mem, mem_attrs *attrs)
> > {
> > MEM_ATTRS (mem) = ggc_alloc<mem_attrs> ();
> > memcpy (MEM_ATTRS (mem), attrs, sizeof (mem_attrs));
> > + if (MEM_EXPR (mem))
> > + {
> > + tree base_address = get_base_address (MEM_EXPR (mem));
> > + if (base_address && TREE_CODE (base_address) == PARM_DECL)
> > + {
> > + /* User alignment on type may be ignored for parameter
> > + passed on stack. */
> > + tree type = TREE_TYPE (base_address);
> > + unsigned int alignment
> > + = targetm.calls.function_arg_boundary (TYPE_MODE (type),
> > + type);
> > + set_mem_align (mem, alignment);
> > + }
> > + }
> > }
> > }
>
> This doesn't feel like the right place to address this. set_mem_attrs
> is just supposed to install the attributes that it has been given,
> without second-guessing the contents.
>
> Where does the incorrect alignment ultimately come from? (As in,
> which piece of code creates the MEM and fails to give it the correct
> alignment?)
x86 has
static unsigned int
ix86_function_arg_boundary (machine_mode mode, const_tree type)
{
unsigned int align;
if (type)
{
/* Since the main variant type is used for call, we convert it to
the main variant type. */
type = TYPE_MAIN_VARIANT (type);
align = TYPE_ALIGN (type);
if (TYPE_EMPTY_P (type))
return PARM_BOUNDARY;
}
Because of
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120900
for C,
typedef struct {
long double a, b
} c __attribute__((aligned(32)));
when such a variable is passed on stack, its alignment
on stack is 16, not 32. However, when set_mem_attrs
is called to set MEM_ALIGN for such a variable on stack,
it sets MEM_ALIGN to 32, instead of 16. But 16 is the
actual alignment on stack. This means that the incorrect
alignment is set exactly where my patch tries to change.
> Richard
>
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/pr120839-1.c
> > b/gcc/testsuite/gcc.target/i386/pr120839-1.c
> > new file mode 100644
> > index 00000000000..74fbf876330
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr120839-1.c
> > @@ -0,0 +1,14 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +
> > +typedef struct
> > +{
> > + long double a;
> > + long double b;
> > +} c __attribute__((aligned(32)));
> > +extern double d;
> > +void
> > +bar (c f)
> > +{
> > + d = f.a;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/i386/pr120839-2.c
> > b/gcc/testsuite/gcc.target/i386/pr120839-2.c
> > new file mode 100644
> > index 00000000000..e5b711c966f
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr120839-2.c
> > @@ -0,0 +1,19 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +/* { dg-final { scan-assembler-not "and\[lq\]?\[\\t \]*\\$-32,\[\\t
> > \]*%\[re\]?sp" } } */
> > +
> > +typedef struct
> > +{
> > + long double a;
> > + long double b;
> > +} c __attribute__((aligned(32)));
> > +extern c x;
> > +extern double d;
> > +extern void bar (c);
> > +void
> > +foo (void)
> > +{
> > + x.a = d;
> > + x.b = d;
> > + bar (x);
> > +}
--
H.J.
