On Fri, Jul 4, 2025 at 9:46 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Fri, Jul 4, 2025 at 3:42 PM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > 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?
>
> Where should I look for such codes in the middle-end?
It must be in the maze of assign_parms, assign_parm_adjust_stack_rtl
looks like what I remember:
/* If we can't trust the parm stack slot to be aligned enough for its
ultimate type, don't use that slot after entry. We'll make another
stack slot, if we need one. */
if (stack_parm
>
> > > 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.
>
>
>
> --
> H.J.
