Moore, Catherine <catherine_mo...@mentor.com> writes:
> The patch itself looks good, but the tests that you added need a little more
> work.
>
> I tested with the mips-sde-elf-lite configuration and I'm seeing failures for
> many
> options. The main failure mode seems to be that the stack is incremented by
> 24 instead of
> 32.
> I tried this change in frame-header-1.c and frame-header-3.c:
>
> /* { dg-final { scan-assembler-not "\taddiu\t\\\$sp,\\\$sp,-8" } }
>
> instead of:
>
> /* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" }
I'd quite like to be specific about the frame layout we expect as the testcase
is so simple
that I think it should be predictable over time. Did you happen to see a
pattern to the
failure? i.e. Could it be non-o32 ABIs? I'm not a fan of scan-assembler-nots in
general
as it is so easy to change exact output text and never match them anyway even
if the
offending instruction is generated.
> There are still errors in frame-header-2.c when compiling with -mips16 and
> -mabi=64 (this
> one uses a daddiu).
> Also, the tests fail for -flto variants.
Let's just add NOMIPS16 (I think that is the macro) to the functions and lock
the tests down
to -mabi=32 which is the only ABI they are valid for anyway.
Matthew
> Would you please fix this up and resubmit?
>
> Thanks,
> Catherine
>
> >
> >
> > diff --git a/gcc/testsuite/gcc.target/mips/frame-header-1.c
> > b/gcc/testsuite/gcc.target/mips/frame-header-1.c
> > new file mode 100644
> > index 0000000..8495e0f
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/mips/frame-header-1.c
> > @@ -0,0 +1,21 @@
> > +/* Verify that we do not optimize away the frame header in foo when using
> > + -mno-frame-header-opt by checking the stack pointer increment done in
> > + that function. Without the optimization foo should increment the stack
> > + by 32 bytes, with the optimization it would only be 8 bytes. */
> > +
> > +/* { dg-do compile } */
> > +/* { dg-options "-mno-frame-header-opt" } */
> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" } } */
> > +
> > +void __attribute__((noinline))
> > +bar (int* a)
> > +{
> > + *a = 1;
> > +}
> > +
> > +void
> > +foo (int a)
> > +{
> > + bar (&a);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/mips/frame-header-2.c
> > b/gcc/testsuite/gcc.target/mips/frame-header-2.c
> > new file mode 100644
> > index 0000000..37ea2d1
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/mips/frame-header-2.c
> > @@ -0,0 +1,21 @@
> > +/* Verify that we do optimize away the frame header in foo when using
> > + -mframe-header-opt by checking the stack pointer increment done in
> > + that function. Without the optimization foo should increment the
> > + stack by 32 bytes, with the optimization it would only be 8 bytes.
> > +*/
> > +
> > +/* { dg-do compile } */
> > +/* { dg-options "-mframe-header-opt" } */
> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-8" } } */
> > +
> > +void __attribute__((noinline))
> > +bar (int* a)
> > +{
> > + *a = 1;
> > +}
> > +
> > +void
> > +foo (int a)
> > +{
> > + bar (&a);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/mips/frame-header-3.c
> > b/gcc/testsuite/gcc.target/mips/frame-header-3.c
> > new file mode 100644
> > index 0000000..1cb1547
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/mips/frame-header-3.c
> > @@ -0,0 +1,22 @@
> > +/* Verify that we do not optimize away the frame header in foo when using
> > + -mframe-header-opt but are calling a weak function that may be
> > overridden
> > + by a different function that does need the frame header. Without the
> > + optimization foo should increment the stack by 32 bytes, with the
> > + optimization it would only be 8 bytes. */
> > +
> > +/* { dg-do compile } */
> > +/* { dg-options "-mframe-header-opt" } */
> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" } } */
> > +
> > +void __attribute__((noinline, weak))
> > +bar (int* a)
> > +{
> > + *a = 1;
> > +}
> > +
> > +void
> > +foo (int a)
> > +{
> > + bar (&a);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
> > b/gcc/testsuite/gcc.target/mips/mips.exp
> > index 42e7fff..0f2d6a2 100644
> > --- a/gcc/testsuite/gcc.target/mips/mips.exp
> > +++ b/gcc/testsuite/gcc.target/mips/mips.exp
> > @@ -256,6 +256,7 @@ set mips_option_groups {
> > maddps "HAS_MADDPS"
> > lsa "(|!)HAS_LSA"
> > section_start "-Wl,--section-start=.*"
> > + frame-header "-mframe-header-opt|-mno-frame-header-opt"
> > }
> >
> > for { set option 0 } { $option < 32 } { incr option } {
> >
