* Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
2009-10-24 9:12 ` Richard Sandiford
@ 2009-10-26 19:08 ` David Daney
2009-10-27 1:04 ` Wu Zhangjin
2009-10-27 21:20 ` Richard Sandiford
0 siblings, 2 replies; 9+ messages in thread
From: David Daney @ 2009-10-26 19:08 UTC (permalink / raw)
To: GCC Patches, rdsandiford
Cc: linux-mips, wuzhangjin, Adam Nemet, rostedt, Thomas Gleixner,
Ralf Baechle, Nicholas Mc Guire
[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]
Richard Sandiford wrote:
> Thanks for the patch.
>
> David Daney <ddaney@caviumnetworks.com> writes:
>> Richard Sandiford wrote:
[...]
>>>
>>> FWIW, I'd certainly be happy to make GCC pass an additional parameter
>>> to _mcount. The parameter could give the address of the return slot,
>>> or null for leaf functions. In almost all cases[*], there would be
>>> no overhead, since the move would go in the delay slot of the call.
>>>
>>> [*] Meaning when the frame is <=32k. ;) I'm guessing you never
>>> get anywhere near that, and if you did, the scan thing wouldn't
>>> work anyway.
>>>
>>> The new behaviour could be controlled by a command-line option,
>>> which would also give linux a cheap way of checking whether the
>>> feature is available.
>>>
>> How about this patch, I think it does what you suggest.
>>
[...]
>
> Hmm, well, the suggestion was to pass a pointer rather than an offset,
> but both you and Wu Zhangjin seem to prefer the offset. Is there a
> reason for that? I suggested a pointer because
>
> (a) they're more C-like
> (b) they're just as quick and easy to compute
> (c) MIPS doesn't have indexed addresses (well, except for a few
> special cases) so the callee is going to have to compute the
> pointer at some stage anyway
Passing a pointer is better. The original patch was based on a
misunderstanding of your original suggestion, this new patch passes the
pointer.
[...]
>
> Looks good otherwise, but I'd be interested in other suggestions for
> the option name. I kept misreading "raloc" as a typo for "reloc".
Well how about raaddr? (Return-Address Address)
Still not tested, but how about this version instead?
gcc/
2009-10-26 David Daney <ddaney@caviumnetworks.com>
* doc/invoke.texi (mmcount-raaddr): Document new command line
option.
* config/mips/mips.opt (mmcount-raaddr): New option.
* config/mips/mips-protos.h (mips_function_profiler): Declare new
function.
* config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset
member.
(mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
(mips_function_profiler): Moved from FUNCTION_PROFILER, and
rewritten.
* config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
mips_function_profiler.
gcc/testsuite/
2009-10-26 David Daney <ddaney@caviumnetworks.com>
* gcc.target/mips/mips.exp (mips_option_groups): Add
mcount-raaddr.
* gcc.target/mips/mmcount-raaddr-1.c: New test.
* gcc.target/mips/mmcount-raaddr-2.c: New test.
* gcc.target/mips/mmcount-raaddr-3.c: New test.
[-- Attachment #2: mcount.patch --]
[-- Type: text/plain, Size: 10163 bytes --]
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 153502)
+++ gcc/doc/invoke.texi (working copy)
@@ -709,7 +709,7 @@ Objective-C and Objective-C++ Dialects}.
-mbranch-cost=@var{num} -mbranch-likely -mno-branch-likely @gol
-mfp-exceptions -mno-fp-exceptions @gol
-mvr4130-align -mno-vr4130-align -msynci -mno-synci @gol
--mrelax-pic-calls -mno-relax-pic-calls}
+-mrelax-pic-calls -mno-relax-pic-calls -mmcount-raaddr}
@emph{MMIX Options}
@gccoptlist{-mlibfuncs -mno-libfuncs -mepsilon -mno-epsilon -mabi=gnu @gol
@@ -14192,6 +14192,19 @@ an assembler and a linker that supports
directive and @code{-mexplicit-relocs} is in effect. With
@code{-mno-explicit-relocs}, this optimization can be performed by the
assembler and the linker alone without help from the compiler.
+
+@item -mmcount-raaddr
+@itemx -mno-mcount-raaddr
+@opindex mmcount-raaddr
+@opindex mno-mcount-raaddr
+Emit (do not emit) code to pass the address of the return address save
+location to @code{_mcount}. The default is @option{-mno-mcount-raaddr}.
+
+@option{-mmcount-raaddr} can be used in conjunction with @option{-pg}
+and a specially coded @code{_mcount} function to record function exit
+by replacing the saved return address with a pointer to the function
+exit profiling function.
+
@end table
@node MMIX Options
Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp (revision 153502)
+++ gcc/testsuite/gcc.target/mips/mips.exp (working copy)
@@ -263,6 +263,7 @@ foreach option {
sym32
synci
relax-pic-calls
+ mcount-raaddr
} {
lappend mips_option_groups $option "-m(no-|)$option"
}
Index: gcc/testsuite/gcc.target/mips/mmcount-raaddr-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mmcount-raaddr-1.c (revision 0)
+++ gcc/testsuite/gcc.target/mips/mmcount-raaddr-1.c (revision 0)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tmove\t\\\$12,\\\$0" } } */
+int bazl(int i)
+{
+ return i + 2;
+}
Index: gcc/testsuite/gcc.target/mips/mmcount-raaddr-2.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mmcount-raaddr-2.c (revision 0)
+++ gcc/testsuite/gcc.target/mips/mmcount-raaddr-2.c (revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tdaddiu\t\\\$12,\\\$sp,8" } } */
+int foo (int);
+int bar (int i)
+{
+ return foo (i) + 2;
+}
Index: gcc/testsuite/gcc.target/mips/mmcount-raaddr-3.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mmcount-raaddr-3.c (revision 0)
+++ gcc/testsuite/gcc.target/mips/mmcount-raaddr-3.c (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tdli\t\\\$12,200008" } } */
+/* { dg-final { scan-assembler "\tdaddu\t\\\$12,\\\$12,\\\$sp" } } */
+int foo (int *);
+int bar(int i)
+{
+ int big[50000];
+ return foo (big) + 2;
+}
Index: gcc/config/mips/mips.opt
===================================================================
--- gcc/config/mips/mips.opt (revision 153502)
+++ gcc/config/mips/mips.opt (working copy)
@@ -208,6 +208,10 @@ mlong64
Target Report RejectNegative Mask(LONG64)
Use a 64-bit long type
+mmcount-raaddr
+Target Report Var(TARGET_MCOUNT_RAADDR)
+Pass the address of the ra save location to _mcount in $12
+
mmemcpy
Target Report Mask(MEMCPY)
Don't optimize block moves
Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h (revision 153502)
+++ gcc/config/mips/mips-protos.h (working copy)
@@ -344,5 +344,6 @@ extern bool mips_eh_uses (unsigned int);
extern bool mips_epilogue_uses (unsigned int);
extern void mips_final_prescan_insn (rtx, rtx *, int);
extern int mips_trampoline_code_size (void);
+extern void mips_function_profiler (FILE *);
#endif /* ! GCC_MIPS_PROTOS_H */
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c (revision 153502)
+++ gcc/config/mips/mips.c (working copy)
@@ -319,6 +319,9 @@ struct GTY(()) mips_frame_info {
HOST_WIDE_INT acc_sp_offset;
HOST_WIDE_INT cop0_sp_offset;
+ /* Similar, but the value passed to _mcount. */
+ HOST_WIDE_INT ra_fp_offset;
+
/* The offset of arg_pointer_rtx from the bottom of the frame. */
HOST_WIDE_INT arg_pointer_offset;
@@ -9616,6 +9619,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WI
for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
{
+ /* Record the ra offset for use by mips_function_profiler. */
+ if (regno == RETURN_ADDR_REGNUM)
+ cfun->machine->frame.ra_fp_offset = offset + sp_offset;
mips_save_restore_reg (word_mode, regno, offset, fn);
offset -= UNITS_PER_WORD;
}
@@ -16088,6 +16094,74 @@ mips_trampoline_init (rtx m_tramp, tree
emit_insn (gen_add3_insn (end_addr, addr, GEN_INT (TRAMPOLINE_SIZE)));
emit_insn (gen_clear_cache (addr, end_addr));
}
+
+/* Implement FUNCTION_PROFILER. */
+
+void mips_function_profiler (FILE *file)
+{
+ if (TARGET_MIPS16)
+ sorry ("mips16 function profiling");
+ if (TARGET_LONG_CALLS)
+ {
+ /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
+ if (Pmode == DImode)
+ fprintf (file, "\tdla\t%s,_mcount\n", reg_names[3]);
+ else
+ fprintf (file, "\tla\t%s,_mcount\n", reg_names[3]);
+ }
+ mips_push_asm_switch (&mips_noat);
+ fprintf (file, "\tmove\t%s,%s\t\t# save current return address\n",
+ reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[2],
+ reg_names[STATIC_CHAIN_REGNUM]);
+ if (TARGET_MCOUNT_RAADDR)
+ {
+ /* If TARGET_MCOUNT_RAADDR load $12 with the address of the ra save
+ location. */
+ if (cfun->machine->frame.ra_fp_offset == 0)
+ /* ra not saved, pass zero. */
+ fprintf (file, "\tmove\t%s,%s\t\t# address of ra\n",
+ reg_names[12], reg_names[0]);
+ else if (SMALL_OPERAND (cfun->machine->frame.ra_fp_offset))
+ fprintf (file,
+ "\t%s\t%s,%s," HOST_WIDE_INT_PRINT_DEC "\t\t# address of ra\n",
+ Pmode == DImode ? "daddiu" : "addiu",
+ reg_names[12], reg_names[STACK_POINTER_REGNUM],
+ cfun->machine->frame.ra_fp_offset);
+ else
+ {
+ fprintf (file,
+ "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "\n",
+ Pmode == DImode ? "dli" : "li",
+ reg_names[12],
+ cfun->machine->frame.ra_fp_offset);
+ fprintf (file, "\t%s\t%s,%s,%s\t\t# address of ra\n",
+ Pmode == DImode ? "daddu" : "addu",
+ reg_names[12], reg_names[12],
+ reg_names[STACK_POINTER_REGNUM]);
+ }
+ }
+ if (!TARGET_NEWABI)
+ {
+ fprintf (file,
+ "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
+ TARGET_64BIT ? "dsubu" : "subu",
+ reg_names[STACK_POINTER_REGNUM],
+ reg_names[STACK_POINTER_REGNUM],
+ Pmode == DImode ? 16 : 8);
+ }
+ if (TARGET_LONG_CALLS)
+ fprintf (file, "\tjalr\t%s\n", reg_names[3]);
+ else
+ fprintf (file, "\tjal\t_mcount\n");
+ mips_pop_asm_switch (&mips_noat);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
+ reg_names[2]);
+}
\f
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h (revision 153502)
+++ gcc/config/mips/mips.h (working copy)
@@ -2372,44 +2372,7 @@ typedef struct mips_args {
/* Output assembler code to FILE to increment profiler label # LABELNO
for profiling a function entry. */
-#define FUNCTION_PROFILER(FILE, LABELNO) \
-{ \
- if (TARGET_MIPS16) \
- sorry ("mips16 function profiling"); \
- if (TARGET_LONG_CALLS) \
- { \
- /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */ \
- if (Pmode == DImode) \
- fprintf (FILE, "\tdla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- } \
- mips_push_asm_switch (&mips_noat); \
- fprintf (FILE, "\tmove\t%s,%s\t\t# save current return address\n", \
- reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[2], \
- reg_names[STATIC_CHAIN_REGNUM]); \
- if (!TARGET_NEWABI) \
- { \
- fprintf (FILE, \
- "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n", \
- TARGET_64BIT ? "dsubu" : "subu", \
- reg_names[STACK_POINTER_REGNUM], \
- reg_names[STACK_POINTER_REGNUM], \
- Pmode == DImode ? 16 : 8); \
- } \
- if (TARGET_LONG_CALLS) \
- fprintf (FILE, "\tjalr\t%s\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tjal\t_mcount\n"); \
- mips_pop_asm_switch (&mips_noat); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM], \
- reg_names[2]); \
-}
+#define FUNCTION_PROFILER(FILE, LABELNO) mips_function_profiler ((FILE))
/* The profiler preserves all interesting registers, including $31. */
#define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) false
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
2009-10-26 19:08 ` [PATCH] MIPS: Add option to pass return address location to _mcount David Daney
@ 2009-10-27 1:04 ` Wu Zhangjin
2009-10-27 21:20 ` Richard Sandiford
1 sibling, 0 replies; 9+ messages in thread
From: Wu Zhangjin @ 2009-10-27 1:04 UTC (permalink / raw)
To: David Daney
Cc: GCC Patches, rdsandiford, linux-mips, Adam Nemet, rostedt,
Thomas Gleixner, Ralf Baechle, Nicholas Mc Guire
Hi,
[...]
> >
> > Looks good otherwise, but I'd be interested in other suggestions for
> > the option name. I kept misreading "raloc" as a typo for "reloc".
>
> Well how about raaddr? (Return-Address Address)
>
> Still not tested, but how about this version instead?
>
I will help to test it later(but maybe tomorrow to later), I think this
will can not pass the compiling, have tried it with the 4.4-200903...,
some of the macros are not defined, such as RETURN_ADDR_REGNUM(not sure
in the latest version of gcc).
Regards,
Wu Zhangjin
> gcc/
> 2009-10-26 David Daney <ddaney@caviumnetworks.com>
>
> * doc/invoke.texi (mmcount-raaddr): Document new command line
> option.
> * config/mips/mips.opt (mmcount-raaddr): New option.
> * config/mips/mips-protos.h (mips_function_profiler): Declare new
> function.
> * config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset
> member.
> (mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
> (mips_function_profiler): Moved from FUNCTION_PROFILER, and
> rewritten.
> * config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
> mips_function_profiler.
>
> gcc/testsuite/
> 2009-10-26 David Daney <ddaney@caviumnetworks.com>
>
> * gcc.target/mips/mips.exp (mips_option_groups): Add
> mcount-raaddr.
> * gcc.target/mips/mmcount-raaddr-1.c: New test.
> * gcc.target/mips/mmcount-raaddr-2.c: New test.
> * gcc.target/mips/mmcount-raaddr-3.c: New test.
>
> plain text document attachment (mcount.patch)
> Index: gcc/doc/invoke.texi
> ===================================================================
> --- gcc/doc/invoke.texi (revision 153502)
> +++ gcc/doc/invoke.texi (working copy)
> @@ -709,7 +709,7 @@ Objective-C and Objective-C++ Dialects}.
> -mbranch-cost=@var{num} -mbranch-likely -mno-branch-likely @gol
> -mfp-exceptions -mno-fp-exceptions @gol
> -mvr4130-align -mno-vr4130-align -msynci -mno-synci @gol
> --mrelax-pic-calls -mno-relax-pic-calls}
> +-mrelax-pic-calls -mno-relax-pic-calls -mmcount-raaddr}
>
> @emph{MMIX Options}
> @gccoptlist{-mlibfuncs -mno-libfuncs -mepsilon -mno-epsilon -mabi=gnu @gol
> @@ -14192,6 +14192,19 @@ an assembler and a linker that supports
> directive and @code{-mexplicit-relocs} is in effect. With
> @code{-mno-explicit-relocs}, this optimization can be performed by the
> assembler and the linker alone without help from the compiler.
> +
> +@item -mmcount-raaddr
> +@itemx -mno-mcount-raaddr
> +@opindex mmcount-raaddr
> +@opindex mno-mcount-raaddr
> +Emit (do not emit) code to pass the address of the return address save
> +location to @code{_mcount}. The default is @option{-mno-mcount-raaddr}.
> +
> +@option{-mmcount-raaddr} can be used in conjunction with @option{-pg}
> +and a specially coded @code{_mcount} function to record function exit
> +by replacing the saved return address with a pointer to the function
> +exit profiling function.
> +
> @end table
>
> @node MMIX Options
> Index: gcc/testsuite/gcc.target/mips/mips.exp
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/mips.exp (revision 153502)
> +++ gcc/testsuite/gcc.target/mips/mips.exp (working copy)
> @@ -263,6 +263,7 @@ foreach option {
> sym32
> synci
> relax-pic-calls
> + mcount-raaddr
> } {
> lappend mips_option_groups $option "-m(no-|)$option"
> }
> Index: gcc/testsuite/gcc.target/mips/mmcount-raaddr-1.c
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/mmcount-raaddr-1.c (revision 0)
> +++ gcc/testsuite/gcc.target/mips/mmcount-raaddr-1.c (revision 0)
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
> +/* { dg-final { scan-assembler "\tmove\t\\\$12,\\\$0" } } */
> +int bazl(int i)
> +{
> + return i + 2;
> +}
> Index: gcc/testsuite/gcc.target/mips/mmcount-raaddr-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/mmcount-raaddr-2.c (revision 0)
> +++ gcc/testsuite/gcc.target/mips/mmcount-raaddr-2.c (revision 0)
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
> +/* { dg-final { scan-assembler "\tdaddiu\t\\\$12,\\\$sp,8" } } */
> +int foo (int);
> +int bar (int i)
> +{
> + return foo (i) + 2;
> +}
> Index: gcc/testsuite/gcc.target/mips/mmcount-raaddr-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/mmcount-raaddr-3.c (revision 0)
> +++ gcc/testsuite/gcc.target/mips/mmcount-raaddr-3.c (revision 0)
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
> +/* { dg-final { scan-assembler "\tdli\t\\\$12,200008" } } */
> +/* { dg-final { scan-assembler "\tdaddu\t\\\$12,\\\$12,\\\$sp" } } */
> +int foo (int *);
> +int bar(int i)
> +{
> + int big[50000];
> + return foo (big) + 2;
> +}
> Index: gcc/config/mips/mips.opt
> ===================================================================
> --- gcc/config/mips/mips.opt (revision 153502)
> +++ gcc/config/mips/mips.opt (working copy)
> @@ -208,6 +208,10 @@ mlong64
> Target Report RejectNegative Mask(LONG64)
> Use a 64-bit long type
>
> +mmcount-raaddr
> +Target Report Var(TARGET_MCOUNT_RAADDR)
> +Pass the address of the ra save location to _mcount in $12
> +
> mmemcpy
> Target Report Mask(MEMCPY)
> Don't optimize block moves
> Index: gcc/config/mips/mips-protos.h
> ===================================================================
> --- gcc/config/mips/mips-protos.h (revision 153502)
> +++ gcc/config/mips/mips-protos.h (working copy)
> @@ -344,5 +344,6 @@ extern bool mips_eh_uses (unsigned int);
> extern bool mips_epilogue_uses (unsigned int);
> extern void mips_final_prescan_insn (rtx, rtx *, int);
> extern int mips_trampoline_code_size (void);
> +extern void mips_function_profiler (FILE *);
>
> #endif /* ! GCC_MIPS_PROTOS_H */
> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c (revision 153502)
> +++ gcc/config/mips/mips.c (working copy)
> @@ -319,6 +319,9 @@ struct GTY(()) mips_frame_info {
> HOST_WIDE_INT acc_sp_offset;
> HOST_WIDE_INT cop0_sp_offset;
>
> + /* Similar, but the value passed to _mcount. */
> + HOST_WIDE_INT ra_fp_offset;
> +
> /* The offset of arg_pointer_rtx from the bottom of the frame. */
> HOST_WIDE_INT arg_pointer_offset;
>
> @@ -9616,6 +9619,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WI
> for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
> if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
> {
> + /* Record the ra offset for use by mips_function_profiler. */
> + if (regno == RETURN_ADDR_REGNUM)
> + cfun->machine->frame.ra_fp_offset = offset + sp_offset;
> mips_save_restore_reg (word_mode, regno, offset, fn);
> offset -= UNITS_PER_WORD;
> }
> @@ -16088,6 +16094,74 @@ mips_trampoline_init (rtx m_tramp, tree
> emit_insn (gen_add3_insn (end_addr, addr, GEN_INT (TRAMPOLINE_SIZE)));
> emit_insn (gen_clear_cache (addr, end_addr));
> }
> +
> +/* Implement FUNCTION_PROFILER. */
> +
> +void mips_function_profiler (FILE *file)
> +{
> + if (TARGET_MIPS16)
> + sorry ("mips16 function profiling");
> + if (TARGET_LONG_CALLS)
> + {
> + /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
> + if (Pmode == DImode)
> + fprintf (file, "\tdla\t%s,_mcount\n", reg_names[3]);
> + else
> + fprintf (file, "\tla\t%s,_mcount\n", reg_names[3]);
> + }
> + mips_push_asm_switch (&mips_noat);
> + fprintf (file, "\tmove\t%s,%s\t\t# save current return address\n",
> + reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]);
> + /* _mcount treats $2 as the static chain register. */
> + if (cfun->static_chain_decl != NULL)
> + fprintf (file, "\tmove\t%s,%s\n", reg_names[2],
> + reg_names[STATIC_CHAIN_REGNUM]);
> + if (TARGET_MCOUNT_RAADDR)
> + {
> + /* If TARGET_MCOUNT_RAADDR load $12 with the address of the ra save
> + location. */
> + if (cfun->machine->frame.ra_fp_offset == 0)
> + /* ra not saved, pass zero. */
> + fprintf (file, "\tmove\t%s,%s\t\t# address of ra\n",
> + reg_names[12], reg_names[0]);
> + else if (SMALL_OPERAND (cfun->machine->frame.ra_fp_offset))
> + fprintf (file,
> + "\t%s\t%s,%s," HOST_WIDE_INT_PRINT_DEC "\t\t# address of ra\n",
> + Pmode == DImode ? "daddiu" : "addiu",
> + reg_names[12], reg_names[STACK_POINTER_REGNUM],
> + cfun->machine->frame.ra_fp_offset);
> + else
> + {
> + fprintf (file,
> + "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "\n",
> + Pmode == DImode ? "dli" : "li",
> + reg_names[12],
> + cfun->machine->frame.ra_fp_offset);
> + fprintf (file, "\t%s\t%s,%s,%s\t\t# address of ra\n",
> + Pmode == DImode ? "daddu" : "addu",
> + reg_names[12], reg_names[12],
> + reg_names[STACK_POINTER_REGNUM]);
> + }
> + }
> + if (!TARGET_NEWABI)
> + {
> + fprintf (file,
> + "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
> + TARGET_64BIT ? "dsubu" : "subu",
> + reg_names[STACK_POINTER_REGNUM],
> + reg_names[STACK_POINTER_REGNUM],
> + Pmode == DImode ? 16 : 8);
> + }
> + if (TARGET_LONG_CALLS)
> + fprintf (file, "\tjalr\t%s\n", reg_names[3]);
> + else
> + fprintf (file, "\tjal\t_mcount\n");
> + mips_pop_asm_switch (&mips_noat);
> + /* _mcount treats $2 as the static chain register. */
> + if (cfun->static_chain_decl != NULL)
> + fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
> + reg_names[2]);
> +}
> \f
> /* Initialize the GCC target structure. */
> #undef TARGET_ASM_ALIGNED_HI_OP
> Index: gcc/config/mips/mips.h
> ===================================================================
> --- gcc/config/mips/mips.h (revision 153502)
> +++ gcc/config/mips/mips.h (working copy)
> @@ -2372,44 +2372,7 @@ typedef struct mips_args {
> /* Output assembler code to FILE to increment profiler label # LABELNO
> for profiling a function entry. */
>
> -#define FUNCTION_PROFILER(FILE, LABELNO) \
> -{ \
> - if (TARGET_MIPS16) \
> - sorry ("mips16 function profiling"); \
> - if (TARGET_LONG_CALLS) \
> - { \
> - /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */ \
> - if (Pmode == DImode) \
> - fprintf (FILE, "\tdla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
> - else \
> - fprintf (FILE, "\tla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
> - } \
> - mips_push_asm_switch (&mips_noat); \
> - fprintf (FILE, "\tmove\t%s,%s\t\t# save current return address\n", \
> - reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]); \
> - /* _mcount treats $2 as the static chain register. */ \
> - if (cfun->static_chain_decl != NULL) \
> - fprintf (FILE, "\tmove\t%s,%s\n", reg_names[2], \
> - reg_names[STATIC_CHAIN_REGNUM]); \
> - if (!TARGET_NEWABI) \
> - { \
> - fprintf (FILE, \
> - "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n", \
> - TARGET_64BIT ? "dsubu" : "subu", \
> - reg_names[STACK_POINTER_REGNUM], \
> - reg_names[STACK_POINTER_REGNUM], \
> - Pmode == DImode ? 16 : 8); \
> - } \
> - if (TARGET_LONG_CALLS) \
> - fprintf (FILE, "\tjalr\t%s\n", reg_names[GP_REG_FIRST + 3]); \
> - else \
> - fprintf (FILE, "\tjal\t_mcount\n"); \
> - mips_pop_asm_switch (&mips_noat); \
> - /* _mcount treats $2 as the static chain register. */ \
> - if (cfun->static_chain_decl != NULL) \
> - fprintf (FILE, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM], \
> - reg_names[2]); \
> -}
> +#define FUNCTION_PROFILER(FILE, LABELNO) mips_function_profiler ((FILE))
>
> /* The profiler preserves all interesting registers, including $31. */
> #define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) false
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
2009-10-26 19:08 ` [PATCH] MIPS: Add option to pass return address location to _mcount David Daney
2009-10-27 1:04 ` Wu Zhangjin
@ 2009-10-27 21:20 ` Richard Sandiford
2009-10-29 6:44 ` Wu Zhangjin
2009-10-29 18:11 ` David Daney
1 sibling, 2 replies; 9+ messages in thread
From: Richard Sandiford @ 2009-10-27 21:20 UTC (permalink / raw)
To: David Daney
Cc: GCC Patches, linux-mips, wuzhangjin, Adam Nemet, rostedt,
Thomas Gleixner, Ralf Baechle, Nicholas Mc Guire
David Daney <ddaney@caviumnetworks.com> writes:
>> Looks good otherwise, but I'd be interested in other suggestions for
>> the option name. I kept misreading "raloc" as a typo for "reloc".
>
> Well how about raaddr? (Return-Address Address)
Taking Wu Zhangjin's suggestion of an extra dash, how about
-mmcount-ra-address?
> +@option{-mmcount-raaddr} can be used in conjunction with @option{-pg}
> +and a specially coded @code{_mcount} function to record function exit
Doc conventions require "specially-coded", I think. We should
mention the $12 register too, and the treatment of leaf functions.
How about:
--------------------------
Allow (do not allow) @code{_mcount} to modify the calling function's
return address. When enabled, this option extends the usual @code{_mcount}
interface with a new @var{ra-address} parameter, which has type
@code{intptr_t *} and is passed in register @code{$12}. @code{_mcount}
can then modify the return address by doing both of the following:
@itemize
@item
Returning the new address in register @code{$31}.
@item
Storing the new address in @code{*@var{ra-address}},
if @var{ra-address} is nonnull.
@end @itemize
The default is @option{-mno-mcount-ra-address}.
--------------------------
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
This is a general feature, so I'd rather test with as few special options
as possible. Just "-O2 -pg" if we can. Same with the other tests.
Sorry to ask, but while you're here, would you mind fixing a couple
of pre-existing formatting problems too? Namely:
> + /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
Only one space for "For" and
> + if (!TARGET_NEWABI)
> + {
> + fprintf (file,
> + "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
> + TARGET_64BIT ? "dsubu" : "subu",
> + reg_names[STACK_POINTER_REGNUM],
> + reg_names[STACK_POINTER_REGNUM],
> + Pmode == DImode ? 16 : 8);
> + }
No braces here.
As far as the new bits are concerned:
> + /* If TARGET_MCOUNT_RAADDR load $12 with the address of the ra save
> + location. */
> + if (cfun->machine->frame.ra_fp_offset == 0)
> + /* ra not saved, pass zero. */
> + fprintf (file, "\tmove\t%s,%s\t\t# address of ra\n",
> + reg_names[12], reg_names[0]);
Let's drop the "# address of ra" comment. We don't add comments to
the other bits.
Everything in the following block:
> + else if (SMALL_OPERAND (cfun->machine->frame.ra_fp_offset))
> + fprintf (file,
> + "\t%s\t%s,%s," HOST_WIDE_INT_PRINT_DEC "\t\t# address of ra\n",
> + Pmode == DImode ? "daddiu" : "addiu",
> + reg_names[12], reg_names[STACK_POINTER_REGNUM],
> + cfun->machine->frame.ra_fp_offset);
> + else
> + {
> + fprintf (file,
> + "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "\n",
> + Pmode == DImode ? "dli" : "li",
> + reg_names[12],
> + cfun->machine->frame.ra_fp_offset);
> + fprintf (file, "\t%s\t%s,%s,%s\t\t# address of ra\n",
> + Pmode == DImode ? "daddu" : "addu",
> + reg_names[12], reg_names[12],
> + reg_names[STACK_POINTER_REGNUM]);
> + }
reduces to:
else
fprintf (file, "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "(%s)",
Pmode == DImode ? "dla" : "la", reg_names[12],
cfun->machine->frame.ra_fp_offset,
reg_names[STACK_POINTER_REGNUM]);
OK with those changes, thanks, if it passing testing, and if
Ralf is happy with the linux side.
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
2009-10-27 21:20 ` Richard Sandiford
@ 2009-10-29 6:44 ` Wu Zhangjin
2009-10-29 16:32 ` David Daney
2009-10-29 18:11 ` David Daney
1 sibling, 1 reply; 9+ messages in thread
From: Wu Zhangjin @ 2009-10-29 6:44 UTC (permalink / raw)
To: Richard Sandiford
Cc: David Daney, GCC Patches, linux-mips, Adam Nemet, rostedt,
Thomas Gleixner, Ralf Baechle, Nicholas Mc Guire
Hi,
On Tue, 2009-10-27 at 21:20 +0000, Richard Sandiford wrote:
[...]
> OK with those changes,
Just applied the above changes, and added one more for not getting the
ra_fp_offset when -pg is not enabled:
@@ -9619,6 +9622,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WIDE_INT
sp_offset,
for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
{
+ /* Record the ra offset for use by mips_function_profiler. */
+ if (crtl->profile && regno == RETURN_ADDR_REGNUM)
+ cfun->machine->frame.ra_fp_offset = offset + sp_offset;
mips_save_restore_reg (word_mode, regno, offset, fn);
offset -= UNITS_PER_WORD;
}
"crtl->profile &&" was added.
> thanks, if it passing testing, and if
> Ralf is happy with the linux side.
>
Just appended that patch with the above changes to the latest gcc 4.5 in
the git repository, and tested it, which works well.
this is the result of a non-leaf function:
ffffffff80243c08 <copy_process>:
ffffffff80243c08: 67bdff40 daddiu sp,sp,-192
ffffffff80243c0c: ffbe00b0 sd s8,176(sp)
ffffffff80243c10: 03a0f02d move s8,sp
ffffffff80243c14: ffbf00b8 sd ra,184(sp)
[...]
ffffffff80243c38: 3c038021 lui v1,0x8021
ffffffff80243c3c: 64631530 daddiu v1,v1,5424
ffffffff80243c40: 03e0082d move at,ra
ffffffff80243c44: 0060f809 jalr v1
ffffffff80243c48: 67ac00b8 daddiu t0,sp,184 --> Here it is
and for a leaf function:
ffffffff802054d0 <au1k_wait>:
ffffffff802054d0: 67bdfff0 daddiu sp,sp,-16
ffffffff802054d4: ffbe0008 sd s8,8(sp)
ffffffff802054d8: 03a0f02d move s8,sp
ffffffff802054dc: 3c038021 lui v1,0x8021
ffffffff802054e0: 64631530 daddiu v1,v1,5424
ffffffff802054e4: 03e0082d move at,ra
ffffffff802054e8: 0060f809 jalr v1
ffffffff802054ec: 0000602d move t0,zero --> Here it is
and with this new feature, I can safely remove the old
ftrace_get_parent_addr() and add several more instructions to make
function graph tracer work. and also, 'Cause this feature used the
t0($12) register, I must change the temp registers I have used in the
patches of ftrace for MIPS(t0 --> t1, t1 --> t2, t2-> t3...). no more
touch.
The latest revision of this patch will be sent out in the next E-mail,
and also, the -v8 revision of ftrace for MIPS will be sent out with this
new feature of Gcc asap.
Thanks & Regards,
Wu Zhangjin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] MIPS: Add option to pass return address location to _mcount
@ 2009-10-29 8:14 Wu Zhangjin
2009-10-29 8:14 ` Wu Zhangjin
2009-10-29 16:24 ` David Daney
0 siblings, 2 replies; 9+ messages in thread
From: Wu Zhangjin @ 2009-10-29 8:14 UTC (permalink / raw)
To: Richard Sandiford, David Daney, GCC Patches
Cc: linux-mips, Adam Nemet, rostedt, Ralf Baechle, Nicholas Mc Guire,
Wu Zhangjin, David Daney
From: Wu Zhangjin <wuzhangjin@gmail.com>
When we pass -pg -mmcount-ra-address, the address of the return address
relative to sp is passed in $12 to _mcount. If the return address is not
saved, $12 will be zero. this will work as registers are never saved with an
offset of zero. $12 is a temporary register that is not part of the ABI.
$12 is also used by libffi closure support, but its use there will not
interfere with _mcount.
2009-10-23 David Daney <ddaney@caviumnetworks.com>
* doc/invoke.texi (mmcount-ra-address): Document new command line option.
* config/mips/mips.opt (config/mips/mips.opt): New option.
* config/mips/mips-protos.h (mips_function_profiler): Declare new
function.
* config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset member.
(mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
(mips_function_profiler): Moved from FUNCTION_PROFILER, and rewritten.
* config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
mips_function_profiler.
Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
gcc/config/mips/mips-protos.h | 1 +
gcc/config/mips/mips.c | 67 +++++++++++++++++++-
gcc/config/mips/mips.h | 39 +-----------
gcc/config/mips/mips.opt | 4 +
gcc/doc/invoke.texi | 22 ++++++-
gcc/testsuite/gcc.target/mips/mips.exp | 1 +
.../gcc.target/mips/mmcount-ra-address-1.c | 7 ++
.../gcc.target/mips/mmcount-ra-address-2.c | 8 +++
.../gcc.target/mips/mmcount-ra-address-3.c | 10 +++
9 files changed, 119 insertions(+), 40 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
create mode 100644 gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
create mode 100644 gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 429a621..716b7ac 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -344,5 +344,6 @@ extern bool mips_eh_uses (unsigned int);
extern bool mips_epilogue_uses (unsigned int);
extern void mips_final_prescan_insn (rtx, rtx *, int);
extern int mips_trampoline_code_size (void);
+extern void mips_function_profiler (FILE *);
#endif /* ! GCC_MIPS_PROTOS_H */
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index f82091a..a1229be 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -319,6 +319,9 @@ struct GTY(()) mips_frame_info {
HOST_WIDE_INT acc_sp_offset;
HOST_WIDE_INT cop0_sp_offset;
+ /* Similar, but the value passed to _mcount. */
+ HOST_WIDE_INT ra_fp_offset;
+
/* The offset of arg_pointer_rtx from the bottom of the frame. */
HOST_WIDE_INT arg_pointer_offset;
@@ -9619,6 +9622,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WIDE_INT sp_offset,
for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
{
+ /* Record the ra offset for use by mips_function_profiler. */
+ if (crtl->profile && regno == RETURN_ADDR_REGNUM)
+ cfun->machine->frame.ra_fp_offset = offset + sp_offset;
mips_save_restore_reg (word_mode, regno, offset, fn);
offset -= UNITS_PER_WORD;
}
@@ -16091,7 +16097,66 @@ mips_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
emit_insn (gen_add3_insn (end_addr, addr, GEN_INT (TRAMPOLINE_SIZE)));
emit_insn (gen_clear_cache (addr, end_addr));
}
-\f
+
+/* Implement FUNCTION_PROFILER. */
+
+void mips_function_profiler (FILE *file)
+{
+ if (TARGET_MIPS16)
+ sorry ("mips16 function profiling");
+ if (TARGET_LONG_CALLS)
+ {
+ /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
+ if (Pmode == DImode)
+ fprintf (file, "\tdla\t%s,_mcount\n", reg_names[3]);
+ else
+ fprintf (file, "\tla\t%s,_mcount\n", reg_names[3]);
+ }
+ mips_push_asm_switch (&mips_noat);
+ fprintf (file, "\tmove\t%s,%s\t\t# save current return address\n",
+ reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[2],
+ reg_names[STATIC_CHAIN_REGNUM]);
+ if (TARGET_MCOUNT_RA_ADDRESS)
+ {
+ /* If TARGET_MCOUNT_RA_ADDRESS load $12 with the address of the ra
+ * save location. */
+ if (cfun->machine->frame.ra_fp_offset == 0)
+ /* ra not saved, pass zero. */
+ fprintf (file, "\tmove\t%s,%s\n",
+ reg_names[12], reg_names[0]);
+ else if (SMALL_OPERAND (cfun->machine->frame.ra_fp_offset))
+ fprintf (file,
+ "\t%s\t%s,%s," HOST_WIDE_INT_PRINT_DEC "\n",
+ Pmode == DImode ? "daddiu" : "addiu", reg_names[12],
+ reg_names[STACK_POINTER_REGNUM],
+ cfun->machine->frame.ra_fp_offset);
+ else
+ fprintf (file, "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "(%s)",
+ Pmode == DImode ? "dla" : "la", reg_names[12],
+ cfun->machine->frame.ra_fp_offset,
+ reg_names[STACK_POINTER_REGNUM]);
+ }
+ if (!TARGET_NEWABI)
+ fprintf (file,
+ "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
+ TARGET_64BIT ? "dsubu" : "subu",
+ reg_names[STACK_POINTER_REGNUM],
+ reg_names[STACK_POINTER_REGNUM],
+ Pmode == DImode ? 16 : 8);
+ if (TARGET_LONG_CALLS)
+ fprintf (file, "\tjalr\t%s\n", reg_names[3]);
+ else
+ fprintf (file, "\tjal\t_mcount\n");
+ mips_pop_asm_switch (&mips_noat);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
+ reg_names[2]);
+}
+
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
#define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 50bc4ea..2829708 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -2372,44 +2372,7 @@ typedef struct mips_args {
/* Output assembler code to FILE to increment profiler label # LABELNO
for profiling a function entry. */
-#define FUNCTION_PROFILER(FILE, LABELNO) \
-{ \
- if (TARGET_MIPS16) \
- sorry ("mips16 function profiling"); \
- if (TARGET_LONG_CALLS) \
- { \
- /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */ \
- if (Pmode == DImode) \
- fprintf (FILE, "\tdla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- } \
- mips_push_asm_switch (&mips_noat); \
- fprintf (FILE, "\tmove\t%s,%s\t\t# save current return address\n", \
- reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[2], \
- reg_names[STATIC_CHAIN_REGNUM]); \
- if (!TARGET_NEWABI) \
- { \
- fprintf (FILE, \
- "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n", \
- TARGET_64BIT ? "dsubu" : "subu", \
- reg_names[STACK_POINTER_REGNUM], \
- reg_names[STACK_POINTER_REGNUM], \
- Pmode == DImode ? 16 : 8); \
- } \
- if (TARGET_LONG_CALLS) \
- fprintf (FILE, "\tjalr\t%s\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tjal\t_mcount\n"); \
- mips_pop_asm_switch (&mips_noat); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM], \
- reg_names[2]); \
-}
+#define FUNCTION_PROFILER(FILE, LABELNO) mips_function_profiler ((FILE))
/* The profiler preserves all interesting registers, including $31. */
#define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) false
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 8462e46..188d5e1 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -208,6 +208,10 @@ mlong64
Target Report RejectNegative Mask(LONG64)
Use a 64-bit long type
+mmcount-ra-address
+Target Report Var(TARGET_MCOUNT_RA_ADDRESS)
+Pass the address of the ra save location to _mcount in $12
+
mmemcpy
Target Report Mask(MEMCPY)
Don't optimize block moves
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4a9ffbf..0e1490e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -709,7 +709,7 @@ Objective-C and Objective-C++ Dialects}.
-mbranch-cost=@var{num} -mbranch-likely -mno-branch-likely @gol
-mfp-exceptions -mno-fp-exceptions @gol
-mvr4130-align -mno-vr4130-align -msynci -mno-synci @gol
--mrelax-pic-calls -mno-relax-pic-calls}
+-mrelax-pic-calls -mno-relax-pic-calls -mmcount-ra-address}
@emph{MMIX Options}
@gccoptlist{-mlibfuncs -mno-libfuncs -mepsilon -mno-epsilon -mabi=gnu @gol
@@ -14207,6 +14207,26 @@ an assembler and a linker that supports the @code{.reloc} assembly
directive and @code{-mexplicit-relocs} is in effect. With
@code{-mno-explicit-relocs}, this optimization can be performed by the
assembler and the linker alone without help from the compiler.
+
+@item -mmcount-ra-address
+@itemx -mno-mcount-ra-address
+@opindex mmcount-ra-address
+@opindex mno-mcount-ra-address
+Allow (do not allow) @code{_mcount} to modify the calling function's
+return address. When enabled, this option extends the usual @code{_mcount}
+interface with a new @var{ra-address} parameter, which has type
+@code{intptr_t *} and is passed in register @code{$12}. @code{_mcount}
+can then modify the return address by doing both of the following:
+@itemize
+@item
+Returning the new address in register @code{$31}.
+@item
+Storing the new address in @code{*@var{ra-address}},
+if @var{ra-address} is nonnull.
+@end itemize
+
+The default is @option{-mno-mcount-ra-address}.
+
@end table
@node MMIX Options
diff --git a/gcc/testsuite/gcc.target/mips/mips.exp b/gcc/testsuite/gcc.target/mips/mips.exp
index aef473f..02e031c 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -263,6 +263,7 @@ foreach option {
sym32
synci
relax-pic-calls
+ mcount-ra-address
} {
lappend mips_option_groups $option "-m(no-|)$option"
}
diff --git a/gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
new file mode 100644
index 0000000..2af802d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tmove\t\\\$12,\\\$0" } } */
+int bazl(int i)
+{
+ return i + 2;
+}
diff --git a/gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
new file mode 100644
index 0000000..d1935c8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tdaddiu\t\\\$12,\\\$sp,8" } } */
+int foo (int);
+int bar (int i)
+{
+ return foo (i) + 2;
+}
diff --git a/gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
new file mode 100644
index 0000000..0798e04
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tdli\t\\\$12,200008" } } */
+/* { dg-final { scan-assembler "\tdaddu\t\\\$12,\\\$12,\\\$sp" } } */
+int foo (int *);
+int bar(int i)
+{
+ int big[50000];
+ return foo (big) + 2;
+}
--
1.6.2.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] MIPS: Add option to pass return address location to _mcount
2009-10-29 8:14 [PATCH] MIPS: Add option to pass return address location to _mcount Wu Zhangjin
@ 2009-10-29 8:14 ` Wu Zhangjin
2009-10-29 16:24 ` David Daney
1 sibling, 0 replies; 9+ messages in thread
From: Wu Zhangjin @ 2009-10-29 8:14 UTC (permalink / raw)
To: Richard Sandiford, David Daney, GCC Patches
Cc: linux-mips, Adam Nemet, rostedt, Ralf Baechle, Nicholas Mc Guire,
Wu Zhangjin
From: Wu Zhangjin <wuzhangjin@gmail.com>
When we pass -pg -mmcount-ra-address, the address of the return address
relative to sp is passed in $12 to _mcount. If the return address is not
saved, $12 will be zero. this will work as registers are never saved with an
offset of zero. $12 is a temporary register that is not part of the ABI.
$12 is also used by libffi closure support, but its use there will not
interfere with _mcount.
2009-10-23 David Daney <ddaney@caviumnetworks.com>
* doc/invoke.texi (mmcount-ra-address): Document new command line option.
* config/mips/mips.opt (config/mips/mips.opt): New option.
* config/mips/mips-protos.h (mips_function_profiler): Declare new
function.
* config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset member.
(mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
(mips_function_profiler): Moved from FUNCTION_PROFILER, and rewritten.
* config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
mips_function_profiler.
Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
gcc/config/mips/mips-protos.h | 1 +
gcc/config/mips/mips.c | 67 +++++++++++++++++++-
gcc/config/mips/mips.h | 39 +-----------
gcc/config/mips/mips.opt | 4 +
gcc/doc/invoke.texi | 22 ++++++-
gcc/testsuite/gcc.target/mips/mips.exp | 1 +
.../gcc.target/mips/mmcount-ra-address-1.c | 7 ++
.../gcc.target/mips/mmcount-ra-address-2.c | 8 +++
.../gcc.target/mips/mmcount-ra-address-3.c | 10 +++
9 files changed, 119 insertions(+), 40 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
create mode 100644 gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
create mode 100644 gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 429a621..716b7ac 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -344,5 +344,6 @@ extern bool mips_eh_uses (unsigned int);
extern bool mips_epilogue_uses (unsigned int);
extern void mips_final_prescan_insn (rtx, rtx *, int);
extern int mips_trampoline_code_size (void);
+extern void mips_function_profiler (FILE *);
#endif /* ! GCC_MIPS_PROTOS_H */
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index f82091a..a1229be 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -319,6 +319,9 @@ struct GTY(()) mips_frame_info {
HOST_WIDE_INT acc_sp_offset;
HOST_WIDE_INT cop0_sp_offset;
+ /* Similar, but the value passed to _mcount. */
+ HOST_WIDE_INT ra_fp_offset;
+
/* The offset of arg_pointer_rtx from the bottom of the frame. */
HOST_WIDE_INT arg_pointer_offset;
@@ -9619,6 +9622,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WIDE_INT sp_offset,
for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
{
+ /* Record the ra offset for use by mips_function_profiler. */
+ if (crtl->profile && regno == RETURN_ADDR_REGNUM)
+ cfun->machine->frame.ra_fp_offset = offset + sp_offset;
mips_save_restore_reg (word_mode, regno, offset, fn);
offset -= UNITS_PER_WORD;
}
@@ -16091,7 +16097,66 @@ mips_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
emit_insn (gen_add3_insn (end_addr, addr, GEN_INT (TRAMPOLINE_SIZE)));
emit_insn (gen_clear_cache (addr, end_addr));
}
-\f
+
+/* Implement FUNCTION_PROFILER. */
+
+void mips_function_profiler (FILE *file)
+{
+ if (TARGET_MIPS16)
+ sorry ("mips16 function profiling");
+ if (TARGET_LONG_CALLS)
+ {
+ /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
+ if (Pmode == DImode)
+ fprintf (file, "\tdla\t%s,_mcount\n", reg_names[3]);
+ else
+ fprintf (file, "\tla\t%s,_mcount\n", reg_names[3]);
+ }
+ mips_push_asm_switch (&mips_noat);
+ fprintf (file, "\tmove\t%s,%s\t\t# save current return address\n",
+ reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[2],
+ reg_names[STATIC_CHAIN_REGNUM]);
+ if (TARGET_MCOUNT_RA_ADDRESS)
+ {
+ /* If TARGET_MCOUNT_RA_ADDRESS load $12 with the address of the ra
+ * save location. */
+ if (cfun->machine->frame.ra_fp_offset == 0)
+ /* ra not saved, pass zero. */
+ fprintf (file, "\tmove\t%s,%s\n",
+ reg_names[12], reg_names[0]);
+ else if (SMALL_OPERAND (cfun->machine->frame.ra_fp_offset))
+ fprintf (file,
+ "\t%s\t%s,%s," HOST_WIDE_INT_PRINT_DEC "\n",
+ Pmode == DImode ? "daddiu" : "addiu", reg_names[12],
+ reg_names[STACK_POINTER_REGNUM],
+ cfun->machine->frame.ra_fp_offset);
+ else
+ fprintf (file, "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "(%s)",
+ Pmode == DImode ? "dla" : "la", reg_names[12],
+ cfun->machine->frame.ra_fp_offset,
+ reg_names[STACK_POINTER_REGNUM]);
+ }
+ if (!TARGET_NEWABI)
+ fprintf (file,
+ "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
+ TARGET_64BIT ? "dsubu" : "subu",
+ reg_names[STACK_POINTER_REGNUM],
+ reg_names[STACK_POINTER_REGNUM],
+ Pmode == DImode ? 16 : 8);
+ if (TARGET_LONG_CALLS)
+ fprintf (file, "\tjalr\t%s\n", reg_names[3]);
+ else
+ fprintf (file, "\tjal\t_mcount\n");
+ mips_pop_asm_switch (&mips_noat);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
+ reg_names[2]);
+}
+
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
#define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 50bc4ea..2829708 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -2372,44 +2372,7 @@ typedef struct mips_args {
/* Output assembler code to FILE to increment profiler label # LABELNO
for profiling a function entry. */
-#define FUNCTION_PROFILER(FILE, LABELNO) \
-{ \
- if (TARGET_MIPS16) \
- sorry ("mips16 function profiling"); \
- if (TARGET_LONG_CALLS) \
- { \
- /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */ \
- if (Pmode == DImode) \
- fprintf (FILE, "\tdla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- } \
- mips_push_asm_switch (&mips_noat); \
- fprintf (FILE, "\tmove\t%s,%s\t\t# save current return address\n", \
- reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[2], \
- reg_names[STATIC_CHAIN_REGNUM]); \
- if (!TARGET_NEWABI) \
- { \
- fprintf (FILE, \
- "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n", \
- TARGET_64BIT ? "dsubu" : "subu", \
- reg_names[STACK_POINTER_REGNUM], \
- reg_names[STACK_POINTER_REGNUM], \
- Pmode == DImode ? 16 : 8); \
- } \
- if (TARGET_LONG_CALLS) \
- fprintf (FILE, "\tjalr\t%s\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tjal\t_mcount\n"); \
- mips_pop_asm_switch (&mips_noat); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM], \
- reg_names[2]); \
-}
+#define FUNCTION_PROFILER(FILE, LABELNO) mips_function_profiler ((FILE))
/* The profiler preserves all interesting registers, including $31. */
#define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) false
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 8462e46..188d5e1 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -208,6 +208,10 @@ mlong64
Target Report RejectNegative Mask(LONG64)
Use a 64-bit long type
+mmcount-ra-address
+Target Report Var(TARGET_MCOUNT_RA_ADDRESS)
+Pass the address of the ra save location to _mcount in $12
+
mmemcpy
Target Report Mask(MEMCPY)
Don't optimize block moves
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4a9ffbf..0e1490e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -709,7 +709,7 @@ Objective-C and Objective-C++ Dialects}.
-mbranch-cost=@var{num} -mbranch-likely -mno-branch-likely @gol
-mfp-exceptions -mno-fp-exceptions @gol
-mvr4130-align -mno-vr4130-align -msynci -mno-synci @gol
--mrelax-pic-calls -mno-relax-pic-calls}
+-mrelax-pic-calls -mno-relax-pic-calls -mmcount-ra-address}
@emph{MMIX Options}
@gccoptlist{-mlibfuncs -mno-libfuncs -mepsilon -mno-epsilon -mabi=gnu @gol
@@ -14207,6 +14207,26 @@ an assembler and a linker that supports the @code{.reloc} assembly
directive and @code{-mexplicit-relocs} is in effect. With
@code{-mno-explicit-relocs}, this optimization can be performed by the
assembler and the linker alone without help from the compiler.
+
+@item -mmcount-ra-address
+@itemx -mno-mcount-ra-address
+@opindex mmcount-ra-address
+@opindex mno-mcount-ra-address
+Allow (do not allow) @code{_mcount} to modify the calling function's
+return address. When enabled, this option extends the usual @code{_mcount}
+interface with a new @var{ra-address} parameter, which has type
+@code{intptr_t *} and is passed in register @code{$12}. @code{_mcount}
+can then modify the return address by doing both of the following:
+@itemize
+@item
+Returning the new address in register @code{$31}.
+@item
+Storing the new address in @code{*@var{ra-address}},
+if @var{ra-address} is nonnull.
+@end itemize
+
+The default is @option{-mno-mcount-ra-address}.
+
@end table
@node MMIX Options
diff --git a/gcc/testsuite/gcc.target/mips/mips.exp b/gcc/testsuite/gcc.target/mips/mips.exp
index aef473f..02e031c 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -263,6 +263,7 @@ foreach option {
sym32
synci
relax-pic-calls
+ mcount-ra-address
} {
lappend mips_option_groups $option "-m(no-|)$option"
}
diff --git a/gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
new file mode 100644
index 0000000..2af802d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tmove\t\\\$12,\\\$0" } } */
+int bazl(int i)
+{
+ return i + 2;
+}
diff --git a/gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
new file mode 100644
index 0000000..d1935c8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tdaddiu\t\\\$12,\\\$sp,8" } } */
+int foo (int);
+int bar (int i)
+{
+ return foo (i) + 2;
+}
diff --git a/gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
new file mode 100644
index 0000000..0798e04
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
+/* { dg-final { scan-assembler "\tdli\t\\\$12,200008" } } */
+/* { dg-final { scan-assembler "\tdaddu\t\\\$12,\\\$12,\\\$sp" } } */
+int foo (int *);
+int bar(int i)
+{
+ int big[50000];
+ return foo (big) + 2;
+}
--
1.6.2.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: Add option to pass return address location to _mcount
2009-10-29 8:14 [PATCH] MIPS: Add option to pass return address location to _mcount Wu Zhangjin
2009-10-29 8:14 ` Wu Zhangjin
@ 2009-10-29 16:24 ` David Daney
1 sibling, 0 replies; 9+ messages in thread
From: David Daney @ 2009-10-29 16:24 UTC (permalink / raw)
To: Wu Zhangjin
Cc: Richard Sandiford, GCC Patches, linux-mips, Adam Nemet, rostedt,
Ralf Baechle, Nicholas Mc Guire
Wu Zhangjin wrote:
> From: Wu Zhangjin <wuzhangjin@gmail.com>
>
[...]
>
> 2009-10-23 David Daney <ddaney@caviumnetworks.com>
>
> * doc/invoke.texi (mmcount-ra-address): Document new command line option.
> * config/mips/mips.opt (config/mips/mips.opt): New option.
> * config/mips/mips-protos.h (mips_function_profiler): Declare new
> function.
> * config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset member.
> (mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
> (mips_function_profiler): Moved from FUNCTION_PROFILER, and rewritten.
> * config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
> mips_function_profiler.
>
> Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>
> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Two things:
1) This is a GCC patch. GCC doesn't use 'Signed-off-by'.
2) You cannot add my 'Signed-off-by', only I can do that. I would
request that you remove it.
Thanks,
David Daney
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
2009-10-29 6:44 ` Wu Zhangjin
@ 2009-10-29 16:32 ` David Daney
0 siblings, 0 replies; 9+ messages in thread
From: David Daney @ 2009-10-29 16:32 UTC (permalink / raw)
To: wuzhangjin
Cc: Richard Sandiford, GCC Patches, linux-mips, Adam Nemet, rostedt,
Thomas Gleixner, Ralf Baechle, Nicholas Mc Guire
Wu Zhangjin wrote:
> Hi,
>
> On Tue, 2009-10-27 at 21:20 +0000, Richard Sandiford wrote:
> [...]
>> OK with those changes,
>
> Just applied the above changes, and added one more for not getting the
> ra_fp_offset when -pg is not enabled:
>
> @@ -9619,6 +9622,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WIDE_INT
> sp_offset,
> for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
> if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
> {
> + /* Record the ra offset for use by mips_function_profiler. */
> + if (crtl->profile && regno == RETURN_ADDR_REGNUM)
> + cfun->machine->frame.ra_fp_offset = offset + sp_offset;
> mips_save_restore_reg (word_mode, regno, offset, fn);
> offset -= UNITS_PER_WORD;
> }
>
> "crtl->profile &&" was added.
>
I am a little confused.
I don't think your change is needed. The FUNCTION_PROFILER code is not
invoked unless -pg is passed.
Were you getting wrong code without your change? The ra_fp_offset field
will be unused if -pg is not specified, but its existence shouldn't
affect code generation.
David Daney
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
2009-10-27 21:20 ` Richard Sandiford
2009-10-29 6:44 ` Wu Zhangjin
@ 2009-10-29 18:11 ` David Daney
1 sibling, 0 replies; 9+ messages in thread
From: David Daney @ 2009-10-29 18:11 UTC (permalink / raw)
To: David Daney, GCC Patches, linux-mips, wuzhangjin, Adam Nemet,
rostedt, Thomas Gleixner, Ralf Baechle, Nicholas Mc Guire,
rdsandiford
[-- Attachment #1: Type: text/plain, Size: 4857 bytes --]
Richard Sandiford wrote:
> David Daney <ddaney@caviumnetworks.com> writes:
>>> Looks good otherwise, but I'd be interested in other suggestions for
>>> the option name. I kept misreading "raloc" as a typo for "reloc".
>> Well how about raaddr? (Return-Address Address)
>
> Taking Wu Zhangjin's suggestion of an extra dash, how about
> -mmcount-ra-address?
>
Right.
>> +@option{-mmcount-raaddr} can be used in conjunction with @option{-pg}
>> +and a specially coded @code{_mcount} function to record function exit
>
> Doc conventions require "specially-coded", I think. We should
> mention the $12 register too, and the treatment of leaf functions.
> How about:
>
> --------------------------
> Allow (do not allow) @code{_mcount} to modify the calling function's
> return address. When enabled, this option extends the usual @code{_mcount}
> interface with a new @var{ra-address} parameter, which has type
> @code{intptr_t *} and is passed in register @code{$12}. @code{_mcount}
> can then modify the return address by doing both of the following:
> @itemize
> @item
> Returning the new address in register @code{$31}.
> @item
> Storing the new address in @code{*@var{ra-address}},
> if @var{ra-address} is nonnull.
> @end @itemize
>
> The default is @option{-mno-mcount-ra-address}.
> --------------------------
Ok. I took a couple of liberties, but used essentially what you suggest.
>
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -pg -mmcount-raaddr -march=mips64 -mabi=64 -mno-abicalls -msym32" } */
>
> This is a general feature, so I'd rather test with as few special options
> as possible. Just "-O2 -pg" if we can. Same with the other tests.
>
As discussed in the other branch of the thread, I now use "-O2 -pg
-mmcount-raaddr -mabi=64"
> Sorry to ask, but while you're here, would you mind fixing a couple
> of pre-existing formatting problems too? Namely:
>
>> + /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
>
> Only one space for "For" and
>
>> + if (!TARGET_NEWABI)
>> + {
>> + fprintf (file,
>> + "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
>> + TARGET_64BIT ? "dsubu" : "subu",
>> + reg_names[STACK_POINTER_REGNUM],
>> + reg_names[STACK_POINTER_REGNUM],
>> + Pmode == DImode ? 16 : 8);
>> + }
>
> No braces here.
OK.
>
> As far as the new bits are concerned:
>
>> + /* If TARGET_MCOUNT_RAADDR load $12 with the address of the ra save
>> + location. */
>> + if (cfun->machine->frame.ra_fp_offset == 0)
>> + /* ra not saved, pass zero. */
>> + fprintf (file, "\tmove\t%s,%s\t\t# address of ra\n",
>> + reg_names[12], reg_names[0]);
>
> Let's drop the "# address of ra" comment. We don't add comments to
> the other bits.
>
OK.
> Everything in the following block:
>
>> + else if (SMALL_OPERAND (cfun->machine->frame.ra_fp_offset))
>> + fprintf (file,
>> + "\t%s\t%s,%s," HOST_WIDE_INT_PRINT_DEC "\t\t# address of ra\n",
>> + Pmode == DImode ? "daddiu" : "addiu",
>> + reg_names[12], reg_names[STACK_POINTER_REGNUM],
>> + cfun->machine->frame.ra_fp_offset);
>> + else
>> + {
>> + fprintf (file,
>> + "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "\n",
>> + Pmode == DImode ? "dli" : "li",
>> + reg_names[12],
>> + cfun->machine->frame.ra_fp_offset);
>> + fprintf (file, "\t%s\t%s,%s,%s\t\t# address of ra\n",
>> + Pmode == DImode ? "daddu" : "addu",
>> + reg_names[12], reg_names[12],
>> + reg_names[STACK_POINTER_REGNUM]);
>> + }
>
> reduces to:
>
> else
> fprintf (file, "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "(%s)",
> Pmode == DImode ? "dla" : "la", reg_names[12],
> cfun->machine->frame.ra_fp_offset,
> reg_names[STACK_POINTER_REGNUM]);
>
Right. I had not realized the the assembler was so 'smart'.
> OK with those changes, thanks, if it passing testing, and if
> Ralf is happy with the linux side.
>
No regressions, and discussed with Ralf.2009-10-29 David Daney
<ddaney@caviumnetworks.com>
* doc/invoke.texi (mmcount-ra-address): Document new command line
option.
* config/mips/mips.opt (mmcount-ra-address): New option.
* config/mips/mips-protos.h (mips_function_profiler): Declare new
function.
* config/mips/mips.c (struct mips_frame_info): Add ra_fp_offset
member.
(mips_for_each_saved_gpr_and_fpr): Set ra_fp_offset.
(mips_function_profiler): Moved from FUNCTION_PROFILER, and
rewritten.
* config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to
mips_function_profiler.
2009-10-29 David Daney <ddaney@caviumnetworks.com>
* gcc.target/mips/mips.exp (mips_option_groups): Add
mcount-ra-address.
* gcc.target/mips/mmcount-ra-address-1.c: New test.
* gcc.target/mips/mmcount-ra-address-2.c: New test.
* gcc.target/mips/mmcount-ra-address-3.c: New test.
FWIW, this is the version I committed:
[-- Attachment #2: mcount.patch --]
[-- Type: text/plain, Size: 9800 bytes --]
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 153716)
+++ gcc/doc/invoke.texi (working copy)
@@ -709,7 +709,7 @@ Objective-C and Objective-C++ Dialects}.
-mbranch-cost=@var{num} -mbranch-likely -mno-branch-likely @gol
-mfp-exceptions -mno-fp-exceptions @gol
-mvr4130-align -mno-vr4130-align -msynci -mno-synci @gol
--mrelax-pic-calls -mno-relax-pic-calls}
+-mrelax-pic-calls -mno-relax-pic-calls -mmcount-ra-address}
@emph{MMIX Options}
@gccoptlist{-mlibfuncs -mno-libfuncs -mepsilon -mno-epsilon -mabi=gnu @gol
@@ -14208,6 +14208,27 @@ an assembler and a linker that supports
directive and @code{-mexplicit-relocs} is in effect. With
@code{-mno-explicit-relocs}, this optimization can be performed by the
assembler and the linker alone without help from the compiler.
+
+@item -mmcount-ra-address
+@itemx -mno-mcount-ra-address
+@opindex mmcount-ra-address
+@opindex mno-mcount-ra-address
+Emit (do not emit) code that allows @code{_mcount} to modify the
+colling function's return address. When enabled, this option extends
+the usual @code{_mcount} interface with a new @var{ra-address}
+parameter, which has type @code{intptr_t *} and is passed in register
+@code{$12}. @code{_mcount} can then modify the return address by
+doing both of the following:
+@itemize
+@item
+Returning the new address in register @code{$31}.
+@item
+Storing the new address in @code{*@var{ra-address}},
+if @var{ra-address} is nonnull.
+@end itemize
+
+The default is @option{-mno-mcount-ra-address}.
+
@end table
@node MMIX Options
Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp (revision 153716)
+++ gcc/testsuite/gcc.target/mips/mips.exp (working copy)
@@ -263,6 +263,7 @@ foreach option {
sym32
synci
relax-pic-calls
+ mcount-ra-address
} {
lappend mips_option_groups $option "-m(no-|)$option"
}
Index: gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c (revision 0)
+++ gcc/testsuite/gcc.target/mips/mmcount-ra-address-1.c (revision 0)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -mabi=64" } */
+/* { dg-final { scan-assembler "\tmove\t\\\$12,\\\$0" } } */
+int bazl(int i)
+{
+ return i + 2;
+}
Index: gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c (revision 0)
+++ gcc/testsuite/gcc.target/mips/mmcount-ra-address-2.c (revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -mabi=64" } */
+/* { dg-final { scan-assembler "\tdla\t\\\$12,8\\(\\\$sp\\)" } } */
+int foo (int);
+int bar (int i)
+{
+ return foo (i) + 2;
+}
Index: gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c
===================================================================
--- gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c (revision 0)
+++ gcc/testsuite/gcc.target/mips/mmcount-ra-address-3.c (revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -pg -mmcount-ra-address -mabi=64" } */
+/* { dg-final { scan-assembler "\tdla\t\\\$12,200008\\(\\\$sp\\)" } } */
+int foo (int *);
+int bar(int i)
+{
+ int big[50000];
+ return foo (big) + 2;
+}
Index: gcc/config/mips/mips.opt
===================================================================
--- gcc/config/mips/mips.opt (revision 153716)
+++ gcc/config/mips/mips.opt (working copy)
@@ -208,6 +208,10 @@ mlong64
Target Report RejectNegative Mask(LONG64)
Use a 64-bit long type
+mmcount-ra-address
+Target Report Var(TARGET_MCOUNT_RA_ADDRESS)
+Pass the address of the ra save location to _mcount in $12
+
mmemcpy
Target Report Mask(MEMCPY)
Don't optimize block moves
Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h (revision 153716)
+++ gcc/config/mips/mips-protos.h (working copy)
@@ -344,5 +344,6 @@ extern bool mips_eh_uses (unsigned int);
extern bool mips_epilogue_uses (unsigned int);
extern void mips_final_prescan_insn (rtx, rtx *, int);
extern int mips_trampoline_code_size (void);
+extern void mips_function_profiler (FILE *);
#endif /* ! GCC_MIPS_PROTOS_H */
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c (revision 153716)
+++ gcc/config/mips/mips.c (working copy)
@@ -319,6 +319,9 @@ struct GTY(()) mips_frame_info {
HOST_WIDE_INT acc_sp_offset;
HOST_WIDE_INT cop0_sp_offset;
+ /* Similar, but the value passed to _mcount. */
+ HOST_WIDE_INT ra_fp_offset;
+
/* The offset of arg_pointer_rtx from the bottom of the frame. */
HOST_WIDE_INT arg_pointer_offset;
@@ -9666,6 +9669,9 @@ mips_for_each_saved_gpr_and_fpr (HOST_WI
for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
{
+ /* Record the ra offset for use by mips_function_profiler. */
+ if (regno == RETURN_ADDR_REGNUM)
+ cfun->machine->frame.ra_fp_offset = offset + sp_offset;
mips_save_restore_reg (word_mode, regno, offset, fn);
offset -= UNITS_PER_WORD;
}
@@ -16138,6 +16144,59 @@ mips_trampoline_init (rtx m_tramp, tree
emit_insn (gen_add3_insn (end_addr, addr, GEN_INT (TRAMPOLINE_SIZE)));
emit_insn (gen_clear_cache (addr, end_addr));
}
+
+/* Implement FUNCTION_PROFILER. */
+
+void mips_function_profiler (FILE *file)
+{
+ if (TARGET_MIPS16)
+ sorry ("mips16 function profiling");
+ if (TARGET_LONG_CALLS)
+ {
+ /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */
+ if (Pmode == DImode)
+ fprintf (file, "\tdla\t%s,_mcount\n", reg_names[3]);
+ else
+ fprintf (file, "\tla\t%s,_mcount\n", reg_names[3]);
+ }
+ mips_push_asm_switch (&mips_noat);
+ fprintf (file, "\tmove\t%s,%s\t\t# save current return address\n",
+ reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[2],
+ reg_names[STATIC_CHAIN_REGNUM]);
+ if (TARGET_MCOUNT_RA_ADDRESS)
+ {
+ /* If TARGET_MCOUNT_RA_ADDRESS load $12 with the address of the
+ ra save location. */
+ if (cfun->machine->frame.ra_fp_offset == 0)
+ /* ra not saved, pass zero. */
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[12], reg_names[0]);
+ else
+ fprintf (file, "\t%s\t%s," HOST_WIDE_INT_PRINT_DEC "(%s)\n",
+ Pmode == DImode ? "dla" : "la", reg_names[12],
+ cfun->machine->frame.ra_fp_offset,
+ reg_names[STACK_POINTER_REGNUM]);
+ }
+ if (!TARGET_NEWABI)
+ fprintf (file,
+ "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n",
+ TARGET_64BIT ? "dsubu" : "subu",
+ reg_names[STACK_POINTER_REGNUM],
+ reg_names[STACK_POINTER_REGNUM],
+ Pmode == DImode ? 16 : 8);
+
+ if (TARGET_LONG_CALLS)
+ fprintf (file, "\tjalr\t%s\n", reg_names[3]);
+ else
+ fprintf (file, "\tjal\t_mcount\n");
+ mips_pop_asm_switch (&mips_noat);
+ /* _mcount treats $2 as the static chain register. */
+ if (cfun->static_chain_decl != NULL)
+ fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
+ reg_names[2]);
+}
\f
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h (revision 153716)
+++ gcc/config/mips/mips.h (working copy)
@@ -2372,44 +2372,7 @@ typedef struct mips_args {
/* Output assembler code to FILE to increment profiler label # LABELNO
for profiling a function entry. */
-#define FUNCTION_PROFILER(FILE, LABELNO) \
-{ \
- if (TARGET_MIPS16) \
- sorry ("mips16 function profiling"); \
- if (TARGET_LONG_CALLS) \
- { \
- /* For TARGET_LONG_CALLS use $3 for the address of _mcount. */ \
- if (Pmode == DImode) \
- fprintf (FILE, "\tdla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tla\t%s,_mcount\n", reg_names[GP_REG_FIRST + 3]); \
- } \
- mips_push_asm_switch (&mips_noat); \
- fprintf (FILE, "\tmove\t%s,%s\t\t# save current return address\n", \
- reg_names[AT_REGNUM], reg_names[RETURN_ADDR_REGNUM]); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[2], \
- reg_names[STATIC_CHAIN_REGNUM]); \
- if (!TARGET_NEWABI) \
- { \
- fprintf (FILE, \
- "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from stack\n", \
- TARGET_64BIT ? "dsubu" : "subu", \
- reg_names[STACK_POINTER_REGNUM], \
- reg_names[STACK_POINTER_REGNUM], \
- Pmode == DImode ? 16 : 8); \
- } \
- if (TARGET_LONG_CALLS) \
- fprintf (FILE, "\tjalr\t%s\n", reg_names[GP_REG_FIRST + 3]); \
- else \
- fprintf (FILE, "\tjal\t_mcount\n"); \
- mips_pop_asm_switch (&mips_noat); \
- /* _mcount treats $2 as the static chain register. */ \
- if (cfun->static_chain_decl != NULL) \
- fprintf (FILE, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM], \
- reg_names[2]); \
-}
+#define FUNCTION_PROFILER(FILE, LABELNO) mips_function_profiler ((FILE))
/* The profiler preserves all interesting registers, including $31. */
#define MIPS_SAVE_REG_FOR_PROFILING_P(REGNO) false
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-10-29 18:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-29 8:14 [PATCH] MIPS: Add option to pass return address location to _mcount Wu Zhangjin
2009-10-29 8:14 ` Wu Zhangjin
2009-10-29 16:24 ` David Daney
-- strict thread matches above, loose matches on Subject: below --
2009-10-21 14:34 [PATCH -v4 1/9] tracing: convert trace_clock_local() as weak function Wu Zhangjin
2009-10-21 14:34 ` [PATCH -v4 2/9] MIPS: add mips_timecounter_read() to get high precision timestamp Wu Zhangjin
2009-10-21 14:34 ` [PATCH -v4 3/9] tracing: add MIPS specific trace_clock_local() Wu Zhangjin
2009-10-21 14:34 ` [PATCH -v4 4/9] tracing: add static function tracer support for MIPS Wu Zhangjin
2009-10-21 15:24 ` Steven Rostedt
2009-10-22 17:47 ` Wu Zhangjin
2009-10-22 18:34 ` David Daney
2009-10-22 22:17 ` Richard Sandiford
2009-10-23 22:48 ` [PATCH] MIPS: Add option to pass return address location to _mcount. Was: " David Daney
2009-10-24 9:12 ` Richard Sandiford
2009-10-26 19:08 ` [PATCH] MIPS: Add option to pass return address location to _mcount David Daney
2009-10-27 1:04 ` Wu Zhangjin
2009-10-27 21:20 ` Richard Sandiford
2009-10-29 6:44 ` Wu Zhangjin
2009-10-29 16:32 ` David Daney
2009-10-29 18:11 ` David Daney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).