This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu <hongjiu.lu@intel.com> wrote:
> >
> > get_frame_size () returns used stack slots during compilation, which
> > may be optimized out later. Since ix86_find_max_used_stack_alignment
> > is called by ix86_finalize_stack_frame_flags to check if stack frame
> > is required, there is no need to call get_frame_size () which may give
> > inaccurate final stack frame size.
> >
> > Tested on AVX512 machine configured with
> >
> > --with-arch=native --with-cpu=native
> >
> > OK for trunk?
> >
> >
> > H.J.
> > ---
> > gcc/
> >
> > PR target/88483
> > * config/i386/i386.c (ix86_finalize_stack_frame_flags): Don't
> > use get_frame_size ().
> >
> > gcc/testsuite/
> >
> > PR target/88483
> > * gcc.target/i386/stackalign/pr88483.c: New test.
>
> LGTM, but you know this part of the compiler better than I.
>
> Thanks,
> Uros.
>
> > ---
> > gcc/config/i386/i386.c | 1 -
> > .../gcc.target/i386/stackalign/pr88483.c | 17 +++++++++++++++++
> > 2 files changed, 17 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
> >
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index caa701fe242..edc8f4f092e 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -12876,7 +12876,6 @@ ix86_finalize_stack_frame_flags (void)
> > && flag_exceptions
> > && cfun->can_throw_non_call_exceptions)
> > && !ix86_frame_pointer_required ()
> > - && get_frame_size () == 0
> > && ix86_nsaved_sseregs () == 0
> > && ix86_varargs_gpr_size + ix86_varargs_fpr_size == 0)
> > {
> > diff --git a/gcc/testsuite/gcc.target/i386/stackalign/pr88483.c b/gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
> > new file mode 100644
> > index 00000000000..5aec8fd4cf6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/stackalign/pr88483.c
> > @@ -0,0 +1,17 @@
> > +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
> > +/* { dg-options "-O2 -mavx2" } */
> > +
> > +struct B
> > +{
> > + char a[12];
> > + int b;
> > +};
> > +
> > +struct B
> > +f2 (void)
> > +{
> > + struct B x = {};
> > + return x;
> > +}
> > +
> > +/* { dg-final { scan-assembler-not "and\[lq\]?\[^\\n\]*-\[0-9\]+,\[^\\n\]*sp" } } */
> > --
> > 2.19.2
> >
My fix triggered a latent bug in ix86_find_max_used_stack_alignment.
Here is the fix. OK for trunk?
Thanks.
--
H.J.
From 83f0b37f287ed198a3b50e2be6b0f7f5c154020e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 14 Dec 2018 12:21:02 -0800
Subject: [PATCH] x86: Properly check stack reference
A latent bug in ix86_find_max_used_stack_alignment was uncovered by the
fix for PR target/88483, which caused:
FAIL: gcc.target/i386/incoming-8.c scan-assembler andl[\\t ]*\\$-16,[\\t ]*%esp
on i386. ix86_find_max_used_stack_alignment failed to notice stack
reference via non-stack/frame registers and missed stack alignment
requirement. We should track all registers which may reference stack
by checking registers set from stack referencing registers.
Tested on i686 and x86-64 with
--with-arch=native --with-cpu=native
on AVX512 machine. Tested on i686 and x86-64 without
--with-arch=native --with-cpu=native
on x86-64 machine.
PR target/88483
* config/i386/i386.c (ix86_stack_referenced_p): New function.
(ix86_find_max_used_stack_alignment): Call ix86_stack_referenced_p
to check if stack is referenced.
---
gcc/config/i386/i386.c | 43 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 4599ca2a7d5..bf93ec3722f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12777,6 +12777,18 @@ output_probe_stack_range (rtx reg, rtx end)
return "";
}
+/* Return true if OP references stack frame though one of registers
+ in STACK_REF_REGS. */
+
+static bool
+ix86_stack_referenced_p (const_rtx op, rtx *stack_ref_regs)
+{
+ for (int i = 0; i < LAST_REX_INT_REG; i++)
+ if (stack_ref_regs[i] && reg_mentioned_p (stack_ref_regs[i], op))
+ return true;
+ return false;
+}
+
/* Return true if stack frame is required. Update STACK_ALIGNMENT
to the largest alignment, in bits, of stack slot used if stack
frame is required and CHECK_STACK_SLOT is true. */
@@ -12801,6 +12813,12 @@ ix86_find_max_used_stack_alignment (unsigned int &stack_alignment,
bool require_stack_frame = false;
+ /* Array of hard registers which reference stack frame. */
+ rtx stack_ref_regs[LAST_REX_INT_REG];
+ memset (stack_ref_regs, 0, sizeof (stack_ref_regs));
+ stack_ref_regs[STACK_POINTER_REGNUM] = stack_pointer_rtx;
+ stack_ref_regs[FRAME_POINTER_REGNUM] = frame_pointer_rtx;
+
FOR_EACH_BB_FN (bb, cfun)
{
rtx_insn *insn;
@@ -12811,16 +12829,33 @@ ix86_find_max_used_stack_alignment (unsigned int &stack_alignment,
{
require_stack_frame = true;
+ rtx body = PATTERN (insn);
+ if (GET_CODE (body) == SET)
+ {
+ /* If a register is set from a stack referencing
+ register, it is a stack referencing register. */
+ rtx dest = SET_DEST (body);
+ if (REG_P (dest) && !MEM_P (SET_SRC (body)))
+ {
+ int regno = REGNO (dest);
+ gcc_assert (regno < FIRST_PSEUDO_REGISTER);
+ if (GENERAL_REGNO_P (regno))
+ {
+ add_to_hard_reg_set (&set_up_by_prologue, Pmode,
+ regno);
+ stack_ref_regs[regno] = dest;
+ }
+ }
+ }
+
if (check_stack_slot)
{
/* Find the maximum stack alignment. */
subrtx_iterator::array_type array;
FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
if (MEM_P (*iter)
- && (reg_mentioned_p (stack_pointer_rtx,
- *iter)
- || reg_mentioned_p (frame_pointer_rtx,
- *iter)))
+ && ix86_stack_referenced_p (*iter,
+ stack_ref_regs))
{
unsigned int alignment = MEM_ALIGN (*iter);
if (alignment > stack_alignment)
--
2.19.2