|
»
»
[PATCH] MIPS: Add option to pass return address location to _mcount. Was: [PATCH -v4 4/9] tracing: add static function tracer support for MIPS
|
View:
New views
12 Messages
—
Rating Filter:
Alert me
|
 |
|
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount. Was: [PATCH -v4 4/9] tracing: add static function tracer support for MIPS
by Richard Sandiford-5
::
Rate this Message:
Thanks for the patch.
David Daney <ddaney@...> writes: > Richard Sandiford wrote: >> David Daney <ddaney@...> writes: >>> Wu Zhangjin wrote: >>>> On Wed, 2009-10-21 at 11:24 -0400, Steven Rostedt wrote: >>> [...] >>>>>> + >>>>>> +NESTED(_mcount, PT_SIZE, ra) >>>>>> + RESTORE_SP_FOR_32BIT >>>>>> + PTR_LA t0, ftrace_stub >>>>>> + PTR_L t1, ftrace_trace_function /* please don't use t1 later, safe? */ >>>>> Is t0 and t1 safe for mcount to use? Remember, mcount does not follow >>>>> the dynamics of C function ABI. >>>> So, perhaps we can use the saved registers(a0,a1...) instead. >>>> >>> a0..a7 may not always be saved. >>> >>> You can use at, v0, v1 and all the temporary registers. Note that for >>> the 64-bit ABIs sometimes the names t0-t4 shadow a4-a7. So for a 64-bit >>> kernel, you can use: $1, $2, $3, $12, $13, $14, $15, $24, $25, noting >>> that at == $1 and contains the callers ra. For a 32-bit kernel you can >>> add $8, $9, $10, and $11 >>> >>> This whole thing seems a little fragile. >>> >>> I think it might be a good idea to get input from Richard Sandiford, >>> and/or Adam Nemet about this approach (so I add them to the CC). >>> >>> This e-mail thread starts here: >>> >>> http://www.linux-mips.org/archives/linux-mips/2009-10/msg00286.html >>> >>> and here: >>> >>> http://www.linux-mips.org/archives/linux-mips/2009-10/msg00290.html >> >> I'm not sure that the "search for a save of RA" thing is really a good idea. >> The last version of that seemed to be "assume that any register stores >> will be in a block that immediately precedes the move into RA", but even >> if that's true now, it might not be in future. And as Wu Zhangjin says, >> it doesn't cope with long calls, where the target address is loaded >> into a temporary register before the call. >> >> 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. > > When we pass -pg -mmcount-raloc, 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. I think 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. 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 (It sounds from Wu Zhangjin's reply like he'd alread suggested the offset thing before I sent my message. If so, sorry for not using that earlier message as context.) > + if (TARGET_RALOC) > + { > + /* If TARGET_RALOC load $12 with the offset of the ra save > + location. */ > + if (mips_raloc_in_delay_slot_p()) > + emit_small_ra_offset = 1; > + else > + { > + if (Pmode == DImode) > + fprintf (file, "\tdli\t%s,%d\t\t# offset of ra\n", reg_names[12], > + cfun->machine->frame.ra_fp_offset); > + else > + fprintf (file, "\tli\t%s,%d\t\t# offset of ra\n", reg_names[12], > + cfun->machine->frame.ra_fp_offset); > + } > + } We shouldn't need to do the delay slot dance. With either the pointer ((D)LA) or offset ((D)LI) approach, the macro won't use $at, so we can insert the new code immediately before the jump, leaving the assembler to fill the delay slot. This is simpler and should mean that the delay slot gets filled more often in the multi-insn-macro cases. Looks good otherwise, but I'd be interested in other suggestions for the option name. I kept misreading "raloc" as a typo for "reloc". Richard |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount. Was: [PATCH -v4 4/9] tracing: add static function tracer support for MIPS
by Wu Zhangjin
::
Rate this Message:
Hi,
On Sat, 2009-10-24 at 10:12 +0100, Richard Sandiford wrote: > Thanks for the patch. [...] > > How about this patch, I think it does what you suggest. > > > > When we pass -pg -mmcount-raloc, 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. I think 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. > > 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 > Agree with you. if not with -fno-omit-frame-pointer, we also need to calculate the frame pointer, and then plus it with the offset. with pointer, we can get it directly, but it may need a more instruction(lui..., addiu...) for loading the pointer. of course, at last, the pointer will save more time for us :-) so, David, could you please use pointer instead? and then I will test it asap(cloning the latest gcc currently). thanks! > (It sounds from Wu Zhangjin's reply like he'd alread suggested the > offset thing before I sent my message. If so, sorry for not using > that earlier message as context.) > It doesn't matter, Seems at that time, you were not added in the CC list, but added by David Daney later. > > + if (TARGET_RALOC) > > + { > > + /* If TARGET_RALOC load $12 with the offset of the ra save > > + location. */ > > + if (mips_raloc_in_delay_slot_p()) > > + emit_small_ra_offset = 1; > > + else > > + { > > + if (Pmode == DImode) > > + fprintf (file, "\tdli\t%s,%d\t\t# offset of ra\n", reg_names[12], > > + cfun->machine->frame.ra_fp_offset); > > + else > > + fprintf (file, "\tli\t%s,%d\t\t# offset of ra\n", reg_names[12], > > + cfun->machine->frame.ra_fp_offset); > > + } > > + } > > We shouldn't need to do the delay slot dance. With either the pointer > ((D)LA) or offset ((D)LI) approach, the macro won't use $at, so we can > insert the new code immediately before the jump, leaving the assembler > to fill the delay slot. This is simpler and should mean that the delay > slot gets filled more often in the multi-insn-macro cases. > > Looks good otherwise, but I'd be interested in other suggestions for > the option name. I kept misreading "raloc" as a typo for "reloc". > The same misreading to me, what about -mmcount-ra-loc? add one "-", or -mcount-ra-location? BTW: Just made dynamic function tracer for MIPS support module tracing with the help of -mlong-calls. after some more tests, I will send it as -v5 revision later. hope the -v6 revision work with this new feature of gcc from David Daney. Regards, Wu Zhangjin |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by David Daney-4
::
Rate this Message:
Richard Sandiford wrote:
> Thanks for the patch. > > David Daney <ddaney@...> 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 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@...> * 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@...> * 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. 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]); +} /* 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 |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by Wu Zhangjin
::
Rate this Message:
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@...> > > * 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@...> > > * 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]); > +} > > /* 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 |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by Richard Sandiford-5
::
Rate this Message:
David Daney <ddaney@...> 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 |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by David Daney-4
::
Rate this Message:
Richard Sandiford wrote:
> David Daney <ddaney@...> writes: [...] >> +/* { 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. > The tests check for the exact offset. This varies with ABI, so I would like to keep the options that force the ABI. I could probably get rid of -msys32 and -mno-abicalls. David Daney |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by Richard Sandiford-5
::
Rate this Message:
David Daney <ddaney@...> writes:
> Richard Sandiford wrote: >> David Daney <ddaney@...> writes: > [...] >>> +/* { 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. >> > > The tests check for the exact offset. This varies with ABI, so I would > like to keep the options that force the ABI. I could probably get rid > of -msys32 and -mno-abicalls. Ah, yeah, good point. Let's go for "-O2 -pg -mcount-blah -mabi=64" then. (The testsuite will force an appropriate architecture.) Thanks, Richard |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by Wu Zhangjin
::
Rate this Message:
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 |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by David Daney-4
::
Rate this Message:
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 |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by David Daney-4
::
Rate this Message:
Richard Sandiford wrote:
> David Daney <ddaney@...> 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}. > -------------------------- > >> +/* { 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. > > 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. > No regressions, and discussed with Ralf.2009-10-29 David Daney <ddaney@...> * 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@...> * 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: 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]); +} /* 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 |
|
|
Re: [PATCH] MIPS: Add option to pass return address location to _mcount.
by Richard Sandiford-5
::
Rate this Message:
David Daney <ddaney@...> writes:
> Richard Sandiford wrote: >> David Daney <ddaney@...> writes: >>> +@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. Yeah, the reworked version looks good, thanks, but I noticed a typo: > +Emit (do not emit) code that allows @code{_mcount} to modify the > +colling function's return address. When enabled, this option extends ^^^^^^^ Fixed as follows and applied. Richard gcc/ * doc/invoke.texi: Fix typo. Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi 2009-11-03 21:25:03.000000000 +0000 +++ gcc/doc/invoke.texi 2009-11-03 21:25:03.000000000 +0000 @@ -14216,7 +14216,7 @@ assembler and the linker alone without h @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 +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 |
| Free embeddable forum powered by Nabble | Forum Help |
