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.
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.
From 2846d8dfcf3f03955f6a5bca700dcb89cbcea16e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 28 Jun 2025 06:27:25 +0800
Subject: [PATCH v2] Don't increase alignment of parameter on stack
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.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
gcc/builtins.cc | 11 ++++++++++-
gcc/emit-rtl.cc | 8 +++++++-
gcc/testsuite/gcc.target/i386/pr120839-1.c | 14 ++++++++++++++
gcc/testsuite/gcc.target/i386/pr120839-2.c | 19 +++++++++++++++++++
4 files changed, 50 insertions(+), 2 deletions(-)
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/builtins.cc b/gcc/builtins.cc
index a2ce3726810..9cecdd8a464 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -277,7 +277,16 @@ get_object_alignment_2 (tree exp, unsigned int *alignp,
}
else if (DECL_P (exp))
{
- align = DECL_ALIGN (exp);
+ if (TREE_CODE (exp) == PARM_DECL)
+ {
+ /* Alignment of parameter passed on stack may be different
+ from DECL_ALIGN. */
+ tree type = TREE_TYPE (exp);
+ align = targetm.calls.function_arg_boundary
+ (TYPE_MODE (type), type);
+ }
+ else
+ align = DECL_ALIGN (exp);
known_alignment = true;
}
else if (TREE_CODE (exp) == INDIRECT_REF
diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
index f4fc92bb37a..234b05c7ef3 100644
--- a/gcc/emit-rtl.cc
+++ b/gcc/emit-rtl.cc
@@ -2092,6 +2092,7 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
MEM_KEEP_ALIAS_SET_P (ref) = 1;
/* If this is a decl, set the attributes of the MEM from it. */
+ bool parm_p = false;
if (DECL_P (t))
{
attrs.expr = t;
@@ -2099,6 +2100,7 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
attrs.offset = 0;
apply_bitpos = bitpos;
new_size = DECL_SIZE_UNIT (t);
+ parm_p = TREE_CODE (t) == PARM_DECL;
}
/* ??? If we end up with a constant or a descriptor do not
@@ -2164,7 +2166,11 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
unsigned int diff_align = known_alignment (obj_bitpos - bitpos);
if (diff_align != 0)
obj_align = MIN (obj_align, diff_align);
- attrs.align = MAX (attrs.align, obj_align);
+ /* Don't increase alignment of parameter passed on stack. */
+ if (parm_p)
+ attrs.align = obj_align;
+ else
+ attrs.align = MAX (attrs.align, obj_align);
}
poly_uint64 const_size;
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);
+}
--
2.50.0
