David Daney <ddaney@xxxxxxxxxxxxxxxxxx> 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
