On Tue, Mar 31, 2026 at 2:43 PM H.J. Lu <[email protected]> wrote: > > On Tue, Mar 31, 2026 at 12:21 AM Richard Biener > <[email protected]> wrote: > > > > 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. > > In order to generate vmovaps, instead of vmovdqu, we align the > stack and generate vmovdqu first. It isn't an optimization.
If you chose to elide the copy we'll still emit a vmovaps. I'm not saying it's particularly great code, but the IL can contain aligned accesses to storage that is declared as being aligned. If you suddenly elide that alignment you break valid code. Richard. > > > 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. > > > > -- > H.J.
