Re: [PATCH] Add a new target hook to compute the frame layout

[Jeff Law <law at redhat dot com>]

On 06/16/2016 08:47 AM, Bernd Edlinger wrote:
> Hi!
>
>
> By the design of the target hook INITIAL_ELIMINATION_OFFSET
> it is necessary to call this function several times with
> different register combinations.
> Most targets use a cached data structure that describes the
> exact frame layout of the current function.
>
> It is safe to skip the computation when reload_completed = true,
> and most targets do that already.
>
> However while reload is doing its work, it is not clear when to
> do the computation and when not.  This results in unnecessary
> work.  Computing the frame layout can be a simple function or an
> arbitrarily complex one, that walks all instructions of the current
> function for instance, which is more or less the common case.
>
>
> This patch adds a new optional target hook that can be used
> by the target to factor the INITIAL_ELIMINATION_OFFSET-hook
> into a O(n) computation part, and a O(1) result function.
>
> The patch implements a compute_frame_layout target hook just
> for ARM in the moment, to show the principle.
> Other targets may also implement that hook, if it seems appropriate.
>
>
> Boot-strapped and reg-tested on arm-linux-gnueabihf.
> OK for trunk?
>
>
> Thanks
> Bernd.
>
>
> changelog-frame-layout.txt
>
>
> 2016-06-16  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	* target.def (compute_frame_layout): New optional target hook.
> 	* doc/tm.texi.in (TARGET_COMPUTE_FRAME_LAYOUT): Add hook.
> 	* doc/tm.texi (TARGET_COMPUTE_FRAME_LAYOUT): Add documentation.
> 	* lra-eliminations.c (update_reg_eliminate): Call compute_frame_layout
> 	target hook.
> 	* reload1.c (verify_initial_elim_offsets): Likewise.
> 	* config/arm/arm.c (TARGET_COMPUTE_FRAME_LAYOUT): Define.
> 	(use_simple_return_p): Call arm_compute_frame_layout if needed.
> 	(arm_get_frame_offsets): Split up into this ...
> 	(arm_compute_frame_layout): ... and this function.
The ARM maintainers would need to chime in on the ARM specific changes 
though.



> Index: gcc/target.def
> ===================================================================
> --- gcc/target.def	(Revision 233176)
> +++ gcc/target.def	(Arbeitskopie)
> @@ -5245,8 +5245,19 @@ five otherwise.  This is best for most machines.",
>   unsigned int, (void),
>   default_case_values_threshold)
>
> -/* Retutn true if a function must have and use a frame pointer.  */
s/Retutn/Return

> +/* Optional callback to advise the target to compute the frame layout.  */
>  DEFHOOK
> +(compute_frame_layout,
> + "This target hook is called immediately before reload wants to call\n\
> +@code{INITIAL_ELIMINATION_OFFSET} and allows the target to cache the frame\n\
> +layout instead of re-computing it on every invocation.  This is particularly\n\
> +useful for targets that have an O(n) frame layout function.  Implementing\n\
> +this callback is optional.",
> + void, (void),
> + hook_void_void)
So the docs say "immediately before", but that's not actually reality in 
lra-eliminations.  I think you can just say "This target hook is called 
before reload or lra-eliminations calls 
@code{INITIAL_ELIMINATION_OFFSET} and allows ..."


How does this macro interact with INITIAL_FRAME_POINTER_OFFSET?

I'm OK with this conceptually.  I think you need a minor doc update and 
OK from the ARM maintainers before it can be installed though.

jeff