mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Daney <ddaney@caviumnetworks.com>
To: rdsandiford@googlemail.com, GCC Patches <gcc-patches@gcc.gnu.org>
Cc: wuzhangjin@gmail.com, Adam Nemet <anemet@caviumnetworks.com>,
	rostedt@goodmis.org, linux-kernel@vger.kernel.org,
	linux-mips@linux-mips.org, Thomas Gleixner <tglx@linutronix.de>,
	Ralf Baechle <ralf@linux-mips.org>,
	Nicholas Mc Guire <der.herr@hofr.at>
Subject: [PATCH] MIPS: Add option to pass return address location to _mcount. Was: [PATCH -v4 4/9] tracing: add static function tracer support for MIPS
Date: Fri, 23 Oct 2009 15:48:13 -0700	[thread overview]
Message-ID: <4AE232AD.4050308@caviumnetworks.com> (raw)
In-Reply-To: <87y6n36plp.fsf@firetop.home>

[-- Attachment #1: Type: text/plain, Size: 3509 bytes --]

Richard Sandiford wrote:
> David Daney <ddaney@caviumnetworks.com> 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  <ddaney@caviumnetworks.com>

	* 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.


[-- Attachment #2: mcount.patch --]
[-- Type: text/plain, Size: 8422 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-raloc}
 
 @emph{MMIX Options}
 @gccoptlist{-mlibfuncs  -mno-libfuncs  -mepsilon  -mno-epsilon  -mabi=gnu @gol
@@ -14192,6 +14192,20 @@ 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-raloc
+@itemx -mno-mcount-raloc
+@opindex mmcount-raloc
+@opindex mno-mcount-raloc
+Emit (do not emit) code to pass the offset (from the stack pointer) of
+the return address save location to @code{_mcount}.  The default is
+@option{-mno-mcount-raloc}.
+
+@option{-mmcount-raloc} can be used in conjunction with @option{-pg}
+and a specially coded @code{_mcount} function to record function exit
+by replacing the return address on the stack with a pointer to the
+function exit profiling function.
+
 @end table
 
 @node MMIX Options
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-raloc
+Target Report Var(TARGET_RALOC)
+Pass the offset from sp 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,86 @@ 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));
 }
+
+/* Return true if the loading of $12 can be done in the jal/jalr delay
+   slot.  We can do this only if the jal will expand to a single
+   instruction and only a single instruction is required to load the
+   value to $12.  */
+
+static bool mips_raloc_in_delay_slot_p(void)
+{
+  return (SMALL_OPERAND_UNSIGNED(cfun->machine->frame.ra_fp_offset)
+	  && !TARGET_USE_GOT);
+}
+
+/* Implement FUNCTION_PROFILER.  */
+
+void mips_function_profiler (FILE *file)
+{
+  int emit_small_ra_offset;
+
+  emit_small_ra_offset = 0;
+
+  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]);
+    }
+  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);
+	}
+    }
+  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 (emit_small_ra_offset)
+    mips_push_asm_switch (&mips_noreorder);
+  if (TARGET_LONG_CALLS)
+    fprintf (file, "\tjalr\t%s\n", reg_names[3]);
+  else
+    fprintf (file, "\tjal\t_mcount\n");
+  if (emit_small_ra_offset)
+    {
+      fprintf (file, "\tori\t%s,%s,%d\t\t# offset of ra\n",
+	       reg_names[12], reg_names[0], cfun->machine->frame.ra_fp_offset);
+      mips_pop_asm_switch (&mips_noreorder);
+    }
+  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

  parent reply	other threads:[~2009-10-23 22:50 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1256135456.git.wuzhangjin@gmail.com>
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-22 13:03   ` pajko
2009-10-21 14:34 ` [PATCH -v4 3/9] tracing: add MIPS specific trace_clock_local() Wu Zhangjin
2009-10-21 14:46   ` Steven Rostedt
2009-10-21 15:11     ` 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 17:59       ` Steven Rostedt
2009-10-22 18:34         ` Wu Zhangjin
2009-10-22 18:34       ` David Daney
2009-10-22 19:13         ` Wu Zhangjin
2009-10-22 20:30         ` Adam Nemet
2009-10-22 20:52           ` Steven Rostedt
2009-10-22 21:09             ` Frederic Weisbecker
2009-10-22 21:29             ` Adam Nemet
2009-10-22 21:55               ` Steven Rostedt
2009-10-23  1:09                 ` Wu Zhangjin
2009-10-22 22:17         ` Richard Sandiford
2009-10-23  9:32           ` Wu Zhangjin
2009-10-23 22:48           ` David Daney [this message]
2009-10-24  9:12             ` [PATCH] MIPS: Add option to pass return address location to _mcount. Was: " Richard Sandiford
2009-10-24 15:53               ` Wu Zhangjin
2009-10-23  7:21         ` Wu Zhangjin
2009-10-21 14:34 ` [PATCH -v4 5/9] tracing: enable HAVE_FUNCTION_TRACE_MCOUNT_TEST " Wu Zhangjin
2009-10-21 14:35 ` [PATCH -v4 6/9] tracing: add an endian argument to scripts/recordmcount.pl Wu Zhangjin
2009-10-21 15:26   ` Steven Rostedt
2009-10-21 14:35 ` [PATCH -v4 7/9] tracing: add dynamic function tracer support for MIPS Wu Zhangjin
2009-10-21 14:35 ` [PATCH -v4 8/9] tracing: not trace mips_timecounter_init() in MIPS Wu Zhangjin
2009-10-21 14:35 ` [PATCH -v4 9/9] tracing: add function graph tracer support for MIPS Wu Zhangjin
2009-10-21 15:21   ` Wu Zhangjin
2009-10-21 16:14     ` Steven Rostedt
2009-10-21 16:12   ` Steven Rostedt
2009-10-21 16:37     ` David Daney
2009-10-21 16:46       ` Steven Rostedt
2009-10-21 17:07         ` David Daney
2009-10-21 17:23           ` Steven Rostedt
2009-10-21 17:48             ` David Daney
2009-10-21 18:09               ` Steven Rostedt
2009-10-21 18:17                 ` Nicholas Mc Guire
2009-10-21 18:34                   ` Steven Rostedt
2009-10-21 18:25                 ` David Daney
2009-10-22 11:38             ` Wu Zhangjin
2009-10-22 13:17               ` Steven Rostedt
2009-10-22 13:31                 ` Wu Zhangjin
2009-10-22 15:20                   ` Steven Rostedt
2009-10-22 15:59               ` David Daney
2009-10-22 16:11                 ` Steven Rostedt
2009-10-22 16:16                   ` David Daney
2009-10-22 18:00                     ` Steven Rostedt
2009-10-22 17:39     ` Wu Zhangjin
2009-10-22 17:58       ` Steven Rostedt
2009-10-22 11:37   ` pajko
2009-10-25 10:48     ` Wu Zhangjin
2009-10-25 13:37       ` Patrik Kluba
2009-10-25 14:22         ` Wu Zhangjin
2009-10-25 15:55       ` Richard Sandiford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AE232AD.4050308@caviumnetworks.com \
    --to=ddaney@caviumnetworks.com \
    --cc=anemet@caviumnetworks.com \
    --cc=der.herr@hofr.at \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=rdsandiford@googlemail.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=wuzhangjin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome