Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On 12/15/18, H.J. Lu wrote:
> On Fri, Dec 14, 2018 at 04:04:02PM -0800, H.J. Lu wrote:
>> On Fri, Dec 14, 2018 at 3:24 PM Jeff Law wrote:
>> >
>> > On 12/14/18 4:01 PM, H.J. Lu wrote:
>> > > On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak
>> > > wrote:
>> > >> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu
>> > >> 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 000..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.
>> > >
>> > >
>> > > 0001-x86-Properly-check-stack-reference.patch
>> > >
>> > > From 83f0b37f287ed198a3b50e2be6b0f7f5c154020e Mon Sep 17 00:00:00
>> > > 2001
>> > > From: "H.J. Lu"
>> > > 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 "";
>> > > }
>>
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On Fri, Dec 14, 2018 at 04:04:02PM -0800, H.J. Lu wrote:
> On Fri, Dec 14, 2018 at 3:24 PM Jeff Law wrote:
> >
> > On 12/14/18 4:01 PM, H.J. Lu wrote:
> > > On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak wrote:
> > >> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu 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 000..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.
> > >
> > >
> > > 0001-x86-Properly-check-stack-reference.patch
> > >
> > > From 83f0b37f287ed198a3b50e2be6b0f7f5c154020e Mon Sep 17 00:00:00 2001
> > > From: "H.J. Lu"
> > > 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++)
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On Fri, Dec 14, 2018 at 3:24 PM Jeff Law wrote:
>
> On 12/14/18 4:01 PM, H.J. Lu wrote:
> > On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak wrote:
> >> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu 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 000..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.
> >
> >
> > 0001-x86-Properly-check-stack-reference.patch
> >
> > From 83f0b37f287ed198a3b50e2be6b0f7f5c154020e Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu"
> > 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. */
>
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On 12/14/18 4:01 PM, H.J. Lu wrote:
> On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak wrote:
>> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu 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 000..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.
>
>
> 0001-x86-Properly-check-stack-reference.patch
>
> From 83f0b37f287ed198a3b50e2be6b0f7f5c154020e Mon Sep 17 00:00:00 2001
> From: "H.J. Lu"
> 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
> _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));
> +
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On Thu, Dec 13, 2018 at 11:11 PM Uros Bizjak wrote:
>
> On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu 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 000..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"
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 _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 _alignment,
{
Re: [PATCH] x86: Don't use get_frame_size to finalize stack frame
On Thu, Dec 13, 2018 at 6:36 PM H.J. Lu 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 000..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
>
[PATCH] x86: Don't use get_frame_size to finalize stack frame
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.
---
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 000..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
