> 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
> BTW, I opened:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124671
>
> with a run-time testcase.
>
> /* If parm was passed in memory, and we need to convert it on entry,
>
>> stack_parm = NULL;
>>
>> /* If parm was passed in memory, and we need to convert it on entry,
>> @@ -3714,7 +3722,7 @@ assign_parms (tree fndecl)
>> else
>> set_decl_incoming_rtl (parm, data.entry_parm, false);
>>
>> - assign_parm_adjust_stack_rtl (&data);
>> + assign_parm_adjust_stack_rtl (parm, &data);
>>
>> if (assign_parm_setup_block_p (&data))
>> assign_parm_setup_block (&all, parm, &data);
>> diff --git a/gcc/testsuite/gcc.dg/torture/pr120839.c
>> b/gcc/testsuite/gcc.dg/torture/pr120839.c
>> new file mode 100644
>> index 00000000000..158e800649f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/torture/pr120839.c
>> @@ -0,0 +1,7 @@
>> +/* { dg-do compile } */
>> +
>> +typedef struct {
>> + long double a, b;
>> +} c __attribute__((aligned(32)));
>> +double d;
>> +void e(c f) { d = f.a; }
>> diff --git a/gcc/testsuite/gcc.target/i386/pr120839-avx.c
>> b/gcc/testsuite/gcc.target/i386/pr120839-avx.c
>> new file mode 100644
>> index 00000000000..1f5406e065c
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/pr120839-avx.c
>> @@ -0,0 +1,8 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -mavx" } */
>> +
>> +typedef struct {
>> + long double a, b;
>> +} c __attribute__((aligned(32)));
>> +double d;
>> +void e(c f) { d = f.a; }
>> --
>> 2.51.0
>
>
>
> --
> H.J.
