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

Reply via email to