Re: [PATCH][IRA] Analysis of register usage of functions for usage by IRA.

[Richard Earnshaw <rearnsha@arm.com>]

On 09/01/14 20:42, Tom de Vries wrote:
> On 09-01-14 15:41, Richard Earnshaw wrote:
>> On 30/03/13 16:10, Tom de Vries wrote:
>>> On 29/03/13 13:54, Tom de Vries wrote:
>>>> I split the patch up into 10 patches, to facilitate further review:
>>>> ...
>>>> 0001-Add-command-line-option.patch
>>>> 0002-Add-new-reg-note-REG_CALL_DECL.patch
>>>> 0003-Add-implicit-parameter-to-find_all_hard_reg_sets.patch
>>>> 0004-Add-TARGET_FN_OTHER_HARD_REG_USAGE-hook.patch
>>>> 0005-Implement-TARGET_FN_OTHER_HARD_REG_USAGE-hook-for-ARM.patch
>>>> 0006-Collect-register-usage-information.patch
>>>> 0007-Use-collected-register-usage-information.patch
>>>> 0008-Enable-by-default-at-O2-and-higher.patch
>>>> 0009-Add-documentation.patch
>>>> 0010-Add-test-case.patch
>>>> ...
>>>> I'll post these in reply to this email.
>>>>
>>>
>>> Something went wrong with those emails, which were generated.
>>>
>>> I tested the emails by sending them to my work email, where they looked fine.
>>> I managed to reproduce the problem by sending them to my private email.
>>> It seems the problem was inconsistent EOL format.
>>>
>>> I've written a python script to handle composing the email, and posted it here
>>> using that script: http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01311.html.
>>> Given that that email looks ok, I think I've addressed the problems now.
>>>
>>> I'll repost the patches. Sorry about the noise.
>>>
>>> Thanks,
>>> - Tom
>>>
>>>
>>
>> It's unfortunate that this feature doesn't fail safe when a port has not
>> explicitly defined what should happen.
>>
> 
> Richard,
> 
> Attached tentative patch (an update of patch 4 in the series) changes the hook 
> in the way you propose.
> 
> Is this patch OK for stage1 (after proper retesting)?

I certainly think that's safer.  Though of course it means that target
maintainers will now have to explicitly enable this when appropriate.
C'est la vie.

> 
>> Consequently, you'll need to add a patch for AArch64 which has two
>> registers clobbered by PLT-based calls.
>>
> 
> Thanks for pointing that out. That's r16 and r17, right? I can propose the hook 
> for AArch64, once we all agree on how the hook should look.
> 

Yes; and thanks!

R.

> Thanks,
> - Tom
> 
>> R.
>>
>>
>> fuse-caller-save-hook.patch
>>
>>
>> 2013-04-29  Radovan Obradovic  <robradovic@mips.com>
>>             Tom de Vries  <tom@codesourcery.com>
>>
>> 	* hooks.c (hook_bool_hard_reg_set_containerp_false): New function.
>> 	* hooks.h (hook_bool_hard_reg_set_containerp_false): Declare.
>> 	* target.def (fn_other_hard_reg_usage): New DEFHOOK.
>> 	* doc/tm.texi.in (@node Stack and Calling): Add Miscellaneous Register
>> 	Hooks to @menu.
>> 	(@node Miscellaneous Register Hooks): New node.
>> 	(@hook TARGET_FN_OTHER_HARD_REG_USAGE): New hook.
>> 	* doc/tm.texi: Regenerate.
>>
>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>> index f204936..1bae6bb 100644
>> --- a/gcc/doc/tm.texi
>> +++ b/gcc/doc/tm.texi
>> @@ -3091,6 +3091,7 @@ This describes the stack layout and calling conventions.
>>  * Profiling::
>>  * Tail Calls::
>>  * Stack Smashing Protection::
>> +* Miscellaneous Register Hooks::
>>  @end menu
>>  
>>  @node Frame Layout
>> @@ -5016,6 +5017,14 @@ normally defined in @file{libgcc2.c}.
>>  Whether this target supports splitting the stack when the options described in @var{opts} have been passed.  This is called after options have been parsed, so the target may reject splitting the stack in some configurations.  The default version of this hook returns false.  If @var{report} is true, this function may issue a warning or error; if @var{report} is false, it must simply return a value
>>  @end deftypefn
>>  
>> +@node Miscellaneous Register Hooks
>> +@subsection Miscellaneous register hooks
>> +@cindex miscellaneous register hooks
>> +
>> +@deftypefn {Target Hook} bool TARGET_FN_OTHER_HARD_REG_USAGE (struct hard_reg_set_container *@var{regs})
>> +Add any hard registers to @var{regs} that are set or clobbered by a call to the function.  This hook only needs to add registers that cannot be found by examination of the final RTL representation of a function.  This hook returns true if it managed to determine which registers need to be added.  The default version of this hook returns false.
>> +@end deftypefn
>> +
>>  @node Varargs
>>  @section Implementing the Varargs Macros
>>  @cindex varargs implementation
>> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
>> index 50f412c..bf75446 100644
>> --- a/gcc/doc/tm.texi.in
>> +++ b/gcc/doc/tm.texi.in
>> @@ -2720,6 +2720,7 @@ This describes the stack layout and calling conventions.
>>  * Profiling::
>>  * Tail Calls::
>>  * Stack Smashing Protection::
>> +* Miscellaneous Register Hooks::
>>  @end menu
>>  
>>  @node Frame Layout
>> @@ -3985,6 +3986,12 @@ the function prologue.  Normally, the profiling code comes after.
>>  
>>  @hook TARGET_SUPPORTS_SPLIT_STACK
>>  
>> +@node Miscellaneous Register Hooks
>> +@subsection Miscellaneous register hooks
>> +@cindex miscellaneous register hooks
>> +
>> +@hook TARGET_FN_OTHER_HARD_REG_USAGE
>> +
>>  @node Varargs
>>  @section Implementing the Varargs Macros
>>  @cindex varargs implementation
>> diff --git a/gcc/hooks.c b/gcc/hooks.c
>> index 1c67bdf..44f1d06 100644
>> --- a/gcc/hooks.c
>> +++ b/gcc/hooks.c
>> @@ -467,3 +467,12 @@ void
>>  hook_void_gcc_optionsp (struct gcc_options *opts ATTRIBUTE_UNUSED)
>>  {
>>  }
>> +
>> +/* Generic hook that takes a struct hard_reg_set_container * and returns
>> +   false.  */
>> +
>> +bool
>> +hook_bool_hard_reg_set_containerp_false (struct hard_reg_set_container *regs ATTRIBUTE_UNUSED)
>> +{
>> +  return false;
>> +}
>> diff --git a/gcc/hooks.h b/gcc/hooks.h
>> index 896b41d..f0afdbd 100644
>> --- a/gcc/hooks.h
>> +++ b/gcc/hooks.h
>> @@ -73,6 +73,7 @@ extern void hook_void_tree (tree);
>>  extern void hook_void_tree_treeptr (tree, tree *);
>>  extern void hook_void_int_int (int, int);
>>  extern void hook_void_gcc_optionsp (struct gcc_options *);
>> +extern bool hook_bool_hard_reg_set_containerp_false (struct hard_reg_set_container *);
>>  
>>  extern int hook_int_uint_mode_1 (unsigned int, enum machine_mode);
>>  extern int hook_int_const_tree_0 (const_tree);
>> diff --git a/gcc/target.def b/gcc/target.def
>> index 3a64cd1..8bee4c3 100644
>> --- a/gcc/target.def
>> +++ b/gcc/target.def
>> @@ -5130,6 +5130,19 @@ FRAME_POINTER_REGNUM, ARG_POINTER_REGNUM, and the PIC_OFFSET_TABLE_REGNUM.",
>>   void, (bitmap regs),
>>   hook_void_bitmap)
>>  
>> +/* Targets should define this target hook to mark which registers are clobbered
>> +   on entry to the function.  They should should set their bits in the struct
>> +   hard_reg_set_container passed in, and return true.  */
>> +DEFHOOK
>> +(fn_other_hard_reg_usage,
>> + "Add any hard registers to @var{regs} that are set or clobbered by a call to\
>> + the function.  This hook only needs to add registers that cannot be found by\
>> + examination of the final RTL representation of a function.  This hook returns\
>> + true if it managed to determine which registers need to be added.  The\
>> + default version of this hook returns false.",
>> + bool, (struct hard_reg_set_container *regs),
>> + hook_bool_hard_reg_set_containerp_false)
>> +
>>  /* Fill in additional registers set up by prologue into a regset.  */
>>  DEFHOOK
>>  (set_up_by_prologue,