On Fri, Jul 4, 2025 at 9:33 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Fri, Jul 4, 2025 at 2:37 PM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
> >
> > "H.J. Lu" <hjl.to...@gmail.com> writes:
> > > 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 what I meant was: which piece of code calls set_mem_attrs
> > to set MEM_ALIGN in this case? The bug seems further up the
> > call stack to me.
> >
>
> Breakpoint 1, set_mem_attrs (mem=0x7fffe9605510, attrs=0x7fffffffccd0)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/emit-rtl.cc:381
> 381 if (mem_attrs_eq_p (attrs, mode_mem_attrs[(int) GET_MODE (mem)]))
> (gdb) bt
> #0 set_mem_attrs (mem=0x7fffe9605510, attrs=0x7fffffffccd0)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/emit-rtl.cc:381
> #1 0x00000000008096d3 in set_mem_attributes_minus_bitpos (ref=0x7fffe9605510,
> t=0x7fffe9810bb0, objectp=1, bitpos=...)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/emit-rtl.cc:2191
> #2 0x0000000000809729 in set_mem_attributes (ref=0x7fffe9605510,
> t=0x7fffe9810bb0, objectp=1)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/emit-rtl.cc:2197
> #3 0x000000000090d1a6 in assign_parm_find_stack_rtl (parm=0x7fffe9810bb0,
> data=0x7fffffffcee0)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/function.cc:2681
> #4 0x0000000000911278 in assign_parms (fndecl=0x7fffe99d2900)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/function.cc:3691
> #5 0x0000000000915be8 in expand_function_start (subr=0x7fffe99d2900)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/function.cc:5201
> #6 0x00000000006c5b6b in (anonymous namespace)::pass_expand::execute (
> this=0x486c0e0, fun=0x7ffff7fbc190)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/cfgexpand.cc:7168
> #7 0x0000000000d02703 in execute_one_pass (pass=0x486c0e0)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/passes.cc:2648
> #8 0x0000000000d02afb in execute_pass_list_1 (pass=0x486c0e0)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/passes.cc:2757
> ...
> (gdb) call debug (mem)
> (mem/c:BLK (reg/f:DI 92 virtual-incoming-args) [0 A8])
> (gdb) f 1
> #1 0x00000000008096d3 in set_mem_attributes_minus_bitpos (ref=0x7fffe9605510,
> t=0x7fffe9810bb0, objectp=1, bitpos=...)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/emit-rtl.cc:2191
> 2191 set_mem_attrs (ref, &attrs);
> (gdb) p attrs
> $2 = {expr = 0x7fffe9810bb0, offset = {coeffs = {0}}, size = {coeffs = {32}},
> alias = 2, align = 256, addrspace = 0 '\000', offset_known_p = true,
> size_known_p = true}
> (gdb)
> Breakpoint 2, set_mem_attributes_minus_bitpos (ref=0x7fffe9605510,
> t=0x7fffe9810bb0, objectp=1, bitpos=...)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/emit-rtl.cc:2163
> 2163 get_object_alignment_1 (t, &obj_align, &obj_bitpos);
> (gdb) next
> 2164 unsigned int diff_align = known_alignment (obj_bitpos - bitpos);
> (gdb)
> 2165 if (diff_align != 0)
> (gdb) p obj_align
> $3 = 256
> (gdb)
> (gdb) next
> 2167 attrs.align = MAX (attrs.align, obj_align);
> (gdb) p attrs.align
> $4 = 256
> (gdb)
>
> 256 is wrong only because
>
> typedef struct {
> long double a;
> long double b;
> } c __attribute__((aligned(32)));
>
> is passed on stack.
>
> The problems are in get_object_alignment_2 and
> set_mem_attributes_minus_bitpos.
I think the problem is that the IL doesn't reflect reality. When the PARM_DECL
(or its type) has the specified alignment that has to reflect reality. Consider
void foo (c x)
{
__typeof (x) *tem = &x;
x->a;
}
when the target doesn't honor the alignment of 'x' then the accesses
based off 'tem'
will be wrong.
The middle-end has, in similar cases, the "fix" for such bad backends
to generate a
callee copy for the incoming argument (with insufficient alignment) to
a local stack
variable with (sufficient alignment). Why is that not triggering here?
> Here is the v2 patch:
>
> Since a backend may ignore user type alignment for arguments passed on
> stack, call targetm.calls.function_arg_boundary to get alignment of
> parameter on stack and don't increase it.
>
> gcc/
>
> PR target/120839
> * builtins.cc (get_object_alignment_2): Call
> targetm.calls.function_arg_boundary to get alignment of
> parameter on stack.
> * emit-rtl.cc (set_mem_attributes_minus_bitpos): Don't
> increase alignment of parameter passed on stack.
>
> gcc/testsuite/
>
> PR target/120839
> * gcc.target/i386/pr120839-1.c: New test.
> * gcc.target/i386/pr120839-2.c: Likewise.
>
> --
> H.J.
