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.
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"
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.
Richard.
