> -----Original Message-----
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Monday, October 05, 2015 1:16 PM
> To: Moore, Catherine
> Cc: Matthew Fortune; GCC Patches
> Subject: RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI
>
> On Mon, 2015-09-28 at 22:10 +0000, Moore, Catherine wrote:
>
> > Hi Steve, I'm sorry for the delay in reviewing this patch.
> > Some changes have been committed upstream (see revision #227941) that
> > will require updates to this patch.
> > Please post the update for review. Other comments are embedded.
>
> OK, I have updated the comments based on your input and changed the
> code to compile with the ToT GCC after revision @227941. Here is the new
> patch.
>
HI Sreve,
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" }
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.
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 } {
>
