On Mon, Mar 30, 2026 at 6:34 PM H.J. Lu <[email protected]> wrote:
>
> On Mon, Mar 30, 2026 at 8:28 AM Richard Biener <[email protected]> wrote:
> >
> >
> >
> > > Am 30.03.2026 um 16:44 schrieb H.J. Lu <[email protected]>:
> > >
> > > On Mon, Mar 30, 2026 at 12:28 AM Richard Biener <[email protected]>
> > > wrote:
> > >>
> > >>> On Sat, 28 Mar 2026, H.J. Lu wrote:
> > >>>
> > >>> On Sat, Mar 28, 2026 at 1:16 AM Richard Biener <[email protected]>
> > >>> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>>> Am 28.03.2026 um 03:36 schrieb H.J. Lu <[email protected]>:
> > >>>>>
> > >>>>> On Fri, Mar 27, 2026 at 4:14 AM 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.
> > >>>>>>
> > >>>>>> OK for the x86 parts? I assume the refined function.cc hunk is still
> > >>>>>> LGTM from Richard S. side.
> > >>>>>>
> > >>>>>> Note I still fail to see why we need to scan all insns in the backend
> > >>>>>> again after RTL expansion should have ensured to keep track of the
> > >>>>>> maximum needed stack alignment. But I'm trying to avoid touching
> > >>>>>> code I do not understand as much as possible.
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Richard.
> > >>>>>>
> > >>>>>> PR middle-end/120839
> > >>>>>> * function.cc (assign_parm_adjust_stack_rtl): Get the parameter
> > >>>>>> as arugment. Adjust alignment check forcing a local copy for
> > >>>>>> SUPPORTS_STACK_ALIGNMENT targets if the argument is not aligned
> > >>>>>> to its type and the current alignment is less than
> > >>>>>> MAX_SUPPORTED_STACK_ALIGNMENT if the parameter has its address
> > >>>>>> taken or BIGGEST_ALIGNMENT otherwise.
> > >>>>>> (assign_parms): Adjust.
> > >>>>>> * config/i386/i386.cc (ix86_update_stack_alignment): Bound
> > >>>>>> recorded stack alignment requirement by BIGGEST_ALIGNMENT.
> > >>>>>>
> > >>>>>> * gcc.dg/torture/pr120839.c: New testcase.
> > >>>>>> * gcc.target/i386/pr120839-avx.c: Likewise.
> > >>>>>> ---
> > >>>>>> gcc/config/i386/i386.cc | 2 +-
> > >>>>>> gcc/function.cc | 14 +++++++++++---
> > >>>>>> gcc/testsuite/gcc.dg/torture/pr120839.c | 7 +++++++
> > >>>>>> gcc/testsuite/gcc.target/i386/pr120839-avx.c | 8 ++++++++
> > >>>>>> 4 files changed, 27 insertions(+), 4 deletions(-)
> > >>>>>> create mode 100644 gcc/testsuite/gcc.dg/torture/pr120839.c
> > >>>>>> create mode 100644 gcc/testsuite/gcc.target/i386/pr120839-avx.c
> > >>>>>>
> > >>>>>> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> > >>>>>> index 15e0dd547a9..f2a49bbaf46 100644
> > >>>>>> --- a/gcc/config/i386/i386.cc
> > >>>>>> +++ b/gcc/config/i386/i386.cc
> > >>>>>> @@ -8627,7 +8627,7 @@ ix86_update_stack_alignment (rtx, const_rtx
> > >>>>>> pat, void *data)
> > >>>>>> unsigned int alignment = MEM_ALIGN (op);
> > >>>>>>
> > >>>>>> if (alignment > *p->stack_alignment)
> > >>>>>> - *p->stack_alignment = alignment;
> > >>>>>> + *p->stack_alignment = MIN (alignment,
> > >>>>>> BIGGEST_ALIGNMENT);
> > >>>>>
> > >>>>> This is wrong. X86 backend supports MAX_OFILE_ALIGNMENT
> > >>>>> stack alignment.
> > >>>>>
> > >>>>>> break;
> > >>>>>> }
> > >>>>>> else
> > >>>>>> diff --git a/gcc/function.cc b/gcc/function.cc
> > >>>>>> index bba05f3380d..41987b4f7a4 100644
> > >>>>>> --- a/gcc/function.cc
> > >>>>>> +++ b/gcc/function.cc
> > >>>>>> @@ -2825,7 +2825,7 @@ assign_parm_remove_parallels (struct
> > >>>>>> assign_parm_data_one *data)
> > >>>>>> always valid and properly aligned. */
> > >>>>>>
> > >>>>>> static void
> > >>>>>> -assign_parm_adjust_stack_rtl (struct assign_parm_data_one *data)
> > >>>>>> +assign_parm_adjust_stack_rtl (tree parm, struct
> > >>>>>> assign_parm_data_one *data)
> > >>>>>> {
> > >>>>>> rtx stack_parm = data->stack_parm;
> > >>>>>>
> > >>>>>> @@ -2840,7 +2840,15 @@ assign_parm_adjust_stack_rtl (struct
> > >>>>>> assign_parm_data_one *data)
> > >>>>>> MEM_ALIGN
> > >>>>>> (stack_parm))))
> > >>>>>> || (data->nominal_type
> > >>>>>> && TYPE_ALIGN (data->nominal_type) > MEM_ALIGN
> > >>>>>> (stack_parm)
> > >>>>>> - && MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY)))
> > >>>>>> + /* When we can re-align the stack ensure appropriate
> > >>>>>> alignment
> > >>>>>> + of the function local object up to
> > >>>>>> BIGGEST_ALIGNMENT if
> > >>>>>> + it is only accessed directly or up to the maximum
> > >>>>>> supported
> > >>>>>> + alignment if the address is exposed. */
> > >>>>>> + && MEM_ALIGN (stack_parm) < (SUPPORTS_STACK_ALIGNMENT
> > >>>>>> + ? (TREE_ADDRESSABLE (parm)
> > >>>>>> + ?
> > >>>>>> MAX_SUPPORTED_STACK_ALIGNMENT
> > >>>>>> + : BIGGEST_ALIGNMENT)
> > >>>>>> + :
> > >>>>>> PREFERRED_STACK_BOUNDARY))))
> > >>>>>
> > >>>>> I think it should be
> > >>>>>
> > >>>>> @@ -2840,7 +2840,11 @@ assign_parm_adjust_stack_rtl (struct
> > >>>>> assign_parm_data_one *data)
> > >>>>> MEM_ALIGN (stack_parm))))
> > >>>>> || (data->nominal_type
> > >>>>> && TYPE_ALIGN (data->nominal_type) > MEM_ALIGN (stack_parm)
> > >>>>> - && MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY)))
> > >>>>> + /* The parm stack slot works if its address isn't taken.
> > >>>>> When
> > >>>>> + making a local copy, MAX_SUPPORTED_STACK_ALIGNMENT is
> > >>>>> + its maximum alignment. */
> > >>>>> + && TREE_ADDRESSABLE (parm)
> > >>>>> + && MEM_ALIGN (stack_parm) < MAX_SUPPORTED_STACK_ALIGNMENT)))
> > >>>>> stack_parm = NULL;
> > >>>>
> > >>>> Even when not address taken the ABI guaranteed alignment of the stack
> > >>>> slot might be less than the declared alignment. If the alignment
> > >>>> itself is not exposed via the address we have still to avoid faulting,
> > >>>> thus BIGGEST_ALIGNMENT
> > >>>
> > >>> There should be no fault. Otherwise, caller will fault first when
> > >>> such an argument
> > >>> is pushed onto stack.
> > >>
> > >> I don't think this necessarily follows, the caller can end up using
> > >> memcpy
> > >> while portions accessed in the callee can end up using aligned moves.
> > >> Also when the ABI effectively requires less alignment the caller has to
> > >> avoid using aligned accesses even when the formal alignment of the
> > >> argument is bigger.
> > >>
> > >> All this is about ABI mandated/guaranteed alignment being lower than
> > >> the types alignment.
> > >
> > > Shouldn't it be decided by backend?
> >
> > I’m not sure what there is to decide? The
> > Alignment as specified by the ABI is insufficient to guarantee the
> > assignment required for the type. That’s a fact.
> >
> > IMO we should diagnose such cases to the user, suggesting pass by reference
> > rather than by value and then honor the types alignment, doing
> > ‚optimization Where it does not matter‘ possibly in a target specific way -
> > my approach would have been to rely on BIGGEST_ALIGNMENT as that should
> > cover all means of access to an object - possibly further constrained by
> > the objects natural size (hoping alignment padding isn’t accessed).
> >
>
> There are no functional issues when a misaligned argument
> is placed on stack. What benefits does it have to make an
> aligned copy when its address isn't taken?
You can end up using instructions requiring the declared alignment.
A struct { float a[8]; } __attribute__((aligned (32))); can be accessed
via a vmovaps by doing *(v8sf *)s.a. You'd need to track actually
used alignment to elide the copy.
IMO teaching users to avoid over-aligned by-value arguments via
diagnostics is the best thing to do here. They might have expected
register passing, much less stack passing + an extra copy due to
the alignment requirement.
Richard.
>
> --
> H.J.
