linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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).