Richard Sandiford wrote: > David Daney 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. $12 is also used by libffi closure support, but I think its use there will not interfere with _mcount. It is very lightly tested, I would bootstrap and regression test with some new test cases if it were deemed acceptable. 2009-10-23 David Daney * doc/invoke.texi (mmcount-raloc): 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_raloc_in_delay_slot_p): New function. (mips_function_profiler): Moved from FUNCTION_PROFILER, and rewritten. * config/mips/mips.h (FUNCTION_PROFILER): Body of macro moved to mips_function_profiler.