On Fri, Mar 27, 2026 at 6:49 AM Uros Bizjak <[email protected]> wrote: > > On Fri, Mar 27, 2026 at 2:16 PM Richard Biener <[email protected]> wrote: > > > > On Fri, 27 Mar 2026, Uros Bizjak wrote: > > > > > On Fri, Mar 27, 2026 at 1:25 PM Uros Bizjak <[email protected]> wrote: > > > > > > > > On Fri, Mar 27, 2026 at 12:14 PM Richard Biener <[email protected]> > > > > wrote: > > > > > > > > > > The following fixes a confusion seen in the x86 backend by > > > > > assign_parm_adjust_stack_rtl failing to trigger a local stack copy > > > > > for an incoming stack parameter that is not aligned according to > > > > > its type. The condition was introduced in r0-64961-gbfc45551d5ace4 > > > > > but there is the MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY > > > > > condition not triggering for the case in question where both > > > > > MEM_ALIGN and PREFERRED_STACK_BOUNDARY are 128. x86 supports > > > > > stack-realignment so we can honor the declared alignment and this > > > > > clears up the confusion. The following replaces the bound > > > > > by MAX_SUPPORTED_STACK_ALIGNMENT if SUPPORTS_STACK_ALIGNMENT > > > > > and the parameter has it's address taken and with BIGGEST_ALIGNMENT > > > > > if SUPPORTS_STACK_ALIGNMENT otherwise, only affecting x86 and nvptx > > > > > at this point. > > > > > > > > > > In addition to this it changes the i386 backends computation of > > > > > the maximum stack alignment used to bound itself to BIGGEST_ALIGNMENT > > > > > because the middle-end does not (and in general cannot) enforce actual > > > > > alignment of all stack objects according to their type. > > > > > > > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > > > > > > > Compared to v2 this uses BIGGEST_ALIGNMENT if the parameter does not > > > > > have its address taken and caps ix86_update_stack_alignment on > > > > > BIGGEST_ALIGNMENT as well. > > > > > > > > I don't think capping ix86_update_stack_alignment to BIGGEST_ALIGNMENT > > > > is correct. For TARGET_IAMCU, biggest alignment is 32, but it still > > > > can use FXSAVE that wants 16-byte (128-bit) alignment. > > > > set_fast_math_sse in libgcc/config/i386/crtfastmath.c uses: > > > > > > > > /* Check if DAZ is available. */ > > > > struct > > > > { > > > > unsigned short cwd; > > > > unsigned short swd; > > > > unsigned short twd; > > > > unsigned short fop; > > > > unsigned int fip; > > > > unsigned int fcs; > > > > unsigned int foo; > > > > unsigned int fos; > > > > unsigned int mxcsr; > > > > unsigned int mxcsr_mask; > > > > unsigned int st_space[32]; > > > > unsigned int xmm_space[32]; > > > > unsigned int padding[56]; > > > > } __attribute__ ((aligned (16))) fxsave; > > > > > > > > which will probably fail with your x86 change due to unsatisfied > > > > alignment requirements. > > > > > > This testcase illustrates above: > > > > > > --cut here-- > > > void foo (void) > > > { > > > struct > > > { > > > unsigned short cwd; > > > unsigned short swd; > > > unsigned short twd; > > > unsigned short fop; > > > unsigned int fip; > > > unsigned int fcs; > > > unsigned int foo; > > > unsigned int fos; > > > unsigned int mxcsr; > > > unsigned int mxcsr_mask; > > > unsigned int st_space[32]; > > > unsigned int xmm_space[32]; > > > unsigned int padding[56]; > > > } __attribute__ ((aligned (16))) fxsave; > > > > > > __builtin_ia32_fxsave(&fxsave); > > > } > > > --cut here-- > > > > > > gcc -O2 -miamcu -m32 -mfxsr: > > > > > > foo: > > > pushl %ebp > > > movl %esp, %ebp > > > andl $-16, %esp > > > subl $512, %esp > > > fxsave (%esp) > > > leave > > > ret > > > > > > The compiler realigns the stack without problems, even if > > > BIGGEST_ALIGNMENT for TARGET_IAMCU is 32 bits. > > > > With the proposed patch it still does: > > > > foo: > > .LFB0: > > .cfi_startproc > > pushl %ebp > > .cfi_def_cfa_offset 8 > > .cfi_offset 5, -8 > > movl %esp, %ebp > > .cfi_def_cfa_register 5 > > andl $-16, %esp > > subl $512, %esp > > fxsave (%esp) > > leave > > .cfi_restore 5 > > .cfi_def_cfa 4, 4 > > ret > > > > But that's likely because it takes the address of fxsave. > > Great, this was my biggest concern with the patch. If some variable is > defined with __attribute__((aligned)), then the compiler should emit > stack realignment code to grant requested alignment. > > > I'll note that I'd view BIGGEST_ALIGNMENT to be wrong, as its > > documentation says: > > > > "@defmac BIGGEST_ALIGNMENT > > Biggest alignment that any data type can require on this machine, in > > bits. Note that this is not the biggest alignment that is supported, > > just the biggest alignment that, when violated, may cause a fault. > > @end defmac" > > Under this explanation, the IA32MCU definition looks OK. > > > I'll also note that the scanning in ix86_update_stack_alignment is > > in addition(?) to what tracking of stack alignment needs is doing > > during RTL expansion. > > > > The alternative is to go back to v2 but that would cause extra > > stack copies to align variables where the extra alignment would > > not have an observable effect. > > Let's wait for HJ's opinion, but based on the definition of > BIGGEST_ALIGNEMNT, I think the x86 part should be OK. > > Uros.
The issue is that DECL_ALIGN of PARM_DECL is limited by targetm.calls.function_arg_boundary no matter whatever value of DECL_ALIGN is. I am testing this patch. Since ix86_function_arg_boundary may ignore type alignment for arguments passed on stack, call ix86_function_arg_boundary to get the actual alignment of arguments passed on stack. gcc/ PR target/120839 * config/i386/i386.cc (ix86_get_alignment_on_stack): New. (ix86_update_stack_alignment): Call ix86_get_alignment_on_stack to get alignment on stack. gcc/testsuite/ PR target/120839 * gcc.target/i386/pr120839-1a.c: New test. * gcc.target/i386/pr120839-1b.c: Likewise. * gcc.target/i386/pr120839-2.c: Likewise. -- H.J.
From 361fef5194f829f994ecb91509a8731d38a7689b Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <[email protected]> Date: Sat, 28 Jun 2025 06:27:25 +0800 Subject: [PATCH] x86: Call ix86_function_arg_boundary to get alignment on stack Since ix86_function_arg_boundary may ignore type alignment for arguments passed on stack, call ix86_function_arg_boundary to get the actual alignment of arguments passed on stack. gcc/ PR target/120839 * config/i386/i386.cc (ix86_get_alignment_on_stack): New. (ix86_update_stack_alignment): Call ix86_get_alignment_on_stack to get alignment on stack. gcc/testsuite/ PR target/120839 * gcc.target/i386/pr120839-1a.c: New test. * gcc.target/i386/pr120839-1b.c: Likewise. * gcc.target/i386/pr120839-2.c: Likewise. Signed-off-by: H.J. Lu <[email protected]> --- gcc/config/i386/i386.cc | 25 ++++++++++++++++++++- gcc/testsuite/gcc.target/i386/pr120839-1a.c | 14 ++++++++++++ gcc/testsuite/gcc.target/i386/pr120839-1b.c | 4 ++++ gcc/testsuite/gcc.target/i386/pr120839-2.c | 19 ++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr120839-1a.c create mode 100644 gcc/testsuite/gcc.target/i386/pr120839-1b.c create mode 100644 gcc/testsuite/gcc.target/i386/pr120839-2.c diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 15e0dd547a9..a6cb58843f6 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -8607,6 +8607,29 @@ struct stack_access_data unsigned int *stack_alignment; }; +/* Return OP alignment on stack. Call ix86_function_arg_boundary to + get the actual alignment of arguments passed on stack. */ + +static unsigned int +ix86_get_alignment_on_stack (const_rtx op) +{ + unsigned int alignment; + tree mem_expr = MEM_EXPR (op); + if (mem_expr) + { + tree var = get_base_address (mem_expr); + if (TREE_CODE (var) == PARM_DECL) + { + tree type = TREE_TYPE (var); + alignment = ix86_function_arg_boundary (TYPE_MODE (type), + type); + return alignment; + } + } + alignment = MEM_ALIGN (op); + return alignment; +} + /* Update the maximum stack slot alignment from memory alignment in PAT. */ static void @@ -8624,7 +8647,7 @@ ix86_update_stack_alignment (rtx, const_rtx pat, void *data) { if (reg_mentioned_p (p->reg, XEXP (op, 0))) { - unsigned int alignment = MEM_ALIGN (op); + unsigned int alignment = ix86_get_alignment_on_stack (op); if (alignment > *p->stack_alignment) *p->stack_alignment = alignment; diff --git a/gcc/testsuite/gcc.target/i386/pr120839-1a.c b/gcc/testsuite/gcc.target/i386/pr120839-1a.c new file mode 100644 index 00000000000..74fbf876330 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr120839-1a.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-1b.c b/gcc/testsuite/gcc.target/i386/pr120839-1b.c new file mode 100644 index 00000000000..7bf17ce6b37 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr120839-1b.c @@ -0,0 +1,4 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mavx" } */ + +#include "pr120839-1a.c" 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.53.0
