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: