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;
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.
