> Am 30.03.2026 um 16:23 schrieb H.J. Lu <[email protected]>:
>
> On Mon, Mar 30, 2026 at 7:03 AM H.J. Lu <[email protected]> wrote:
>>
>>> On Mon, Mar 30, 2026 at 6:18 AM Richard Biener <[email protected]> wrote:
>>>
>>>> On Mon, 30 Mar 2026, H.J. Lu wrote:
>>>
>>>> On Mon, Mar 30, 2026 at 1:58 AM Richard Biener <[email protected]> wrote:
>>>>>
>>>>> On Sun, 29 Mar 2026, H.J. Lu wrote:
>>>>>
>>>>>> On Sat, Mar 28, 2026 at 5:18 AM H.J. Lu <[email protected]> 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.
>>>>>>>
>>>>>>
>>>>>> Here is the combined patch with tests.
>>>>>>
>>>>>> Adjust alignment check forcing a local copy if the argument on stack is
>>>>>> not aligned to its type and the current alignment, which is determined
>>>>>> by the backend, is less than MAX_SUPPORTED_STACK_ALIGNMENT when the
>>>>>> parameter has its address taken.
>>>>>>
>>>>>> For x86, ignore argument passed on stack when updating stack alignment
>>>>>> since caller is responsible to align the outgoing stack for arguments
>>>>>> passed on stack.
>>>>>
>>>>> @@ -2840,7 +2840,12 @@ 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)))
>>>>> + /* Use the parm stack slot, whose alignment is determined
>>>>> + by targetm.calls.function_arg_boundary, if its address
>>>>> + isn't taken. When making a local copy, its maximum
>>>>> + alignment is MAX_SUPPORTED_STACK_ALIGNMENT. */
>>>>> + && TREE_ADDRESSABLE (parm)
>>>>> + && MEM_ALIGN (stack_parm) < MAX_SUPPORTED_STACK_ALIGNMENT)))
>>>>>
>>>>> This changes the !TREE_ADDRESSABLE (parm) behavior which I think we
>>>>> do not want to do at this point. I think we want
>>>>>
>>>>> && (MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY
>>>>> || (TREE_ADDRESSABLE (parm)
>>>>> && MEM_ALIGN (stack_parm) <
>>>>> MAX_SUPPORTED_STACK_ALIGNMENT))
>>>>
>>>> This will cause unnecessary stack realignment for
>>>>
>>>> typedef struct {
>>>> double a;
>>>> double b;
>>>> double c;
>>>> double d;
>>>> } c __attribute__((aligned(32)));
>>>> extern double d;
>>>> void
>>>> e (float a1, float a2, float a3, float a4, float a5, float a6, c f)
>>>> {
>>>> d = f.a;
>>>> }
>>>>
>>>> It is
>>>>
>>>> pushq %rbp
>>>> movq %rsp, %rbp
>>>> movsd 16(%rbp), %xmm0
>>>> popq %rbp
>>>> movsd %xmm0, d(%rip)
>>>> ret
>>>>
>>>> vs
>>>>
>>>> pushq %rbp
>>>> movq %rsp, %rbp
>>>> andq $-32, %rsp
>>>> movdqu 16(%rbp), %xmm0
>>>> movaps %xmm0, -32(%rsp)
>>>> movsd -32(%rsp), %xmm0
>>>> movsd %xmm0, d(%rip)
>>>> leave
>>>> ret
>>>>
>>>> MEM_ALIGN (stack_parm) is properly aligned according to ISA
>>>> and ABI as set by target. There is no need to realign stack
>>>> if address isn't taken.
>>>
>>> But that's a separate enhancement, not suitable for stage4. It also
>>> depends on the ABI, you may remove required re-alignment for targets
>>> where the ABI isn't as forgiving. It also seems that any such
>>> change should involve BIGGEST_ALIGNMENT as in my patch.
>>
>> BIGGEST_ALIGNMENT is the biggest alignment for all types,
>> not for each type.
>>
>>> Please submit separately for stage1.
>>
>> Will do that.
>
> I opened:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124697
>
> Here is the v4 patch.
Ok
Richard
> Adjust alignment check forcing a local copy if the argument on stack is
> not aligned to its type and the current alignment, which is determined
> by the backend, is less than MAX_SUPPORTED_STACK_ALIGNMENT when the
> parameter has its address taken.
>
> For x86, ignore argument passed on stack when updating stack alignment
> since caller is responsible to align the outgoing stack for arguments
> passed on stack.
>
> gcc/
>
> PR target/120839
> PR middle-end/124671
> * function.cc (assign_parm_adjust_stack_rtl): Get the parameter
> as argument. Adjust alignment check forcing a local copy.
> (assign_parms): Adjust.
> * config/i386/i386.cc (ix86_argument_passed_on_stack_p): New.
> (ix86_update_stack_alignment): Ignore argument passed on stack.
>
> gcc/testsuite/
>
> PR target/120839
> PR middle-end/124671
> * gcc.dg/pr124671.c: New test.
> * gcc.target/i386/pr120839-1a.c: Likewise.
> * gcc.target/i386/pr120839-1b.c: Likewise.
> * gcc.target/i386/pr120839-2.c: Likewise.
>
> Signed-off-by: H.J. Lu <[email protected]>
> Co-Authored-By: Richard Biener <[email protected]>
>
>
> --
> H.J.
> <v4-0001-Adjust-check-for-addressable-misaligned-stack-arg.patch>
