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

[Bernd Edlinger]

On 05/05/17 16:59, Richard Earnshaw (lists) wrote:
> On 05/09/16 17:43, Bernd Edlinger wrote:
>> Hi Richard,
>>
>> what do you think of this patch, is it OK (with the suggested wording)?
>>
>
> Bernd,
>
> Apologies, this seems to have fallen through a crack.
>
> I'm happy with this.  Does it still apply?
>

Yes, only in a few places the context lines changed slightly.
So I attached a refreshed patch for your reference.

> If so, I suggest applying it after a 24-hour cooling off period for any
> final comments.
>

Fine, then I will apply it on monday evening, unless someone objects.


Thanks
Bernd.

> R.
>
>>
>> Thanks
>> Bernd.
>>
>> On 08/05/16 16:06, Richard Earnshaw (lists) wrote:
>>> On 05/08/16 13:49, Bernd Edlinger wrote:
 On 08/05/16 11:29, Richard Earnshaw (lists) wrote:
> On 04/08/16 22:16, Bernd Edlinger wrote:
>> Hi,
>>
>> this patch introduces a new target hook that allows the target's
>> INITIAL_ELIMINATION_OFFSET function to use cached values instead of
>> re-computing the frame layout every time.
>>
>> I have updated the documentation a bit and hope it is clearer this time.
>>
>> It still needs a review by ARM port maintainers.
>>
>> If the ARM port maintainers find this patch useful, that would be fine.
>>
>
> I need to look into this more, but my first thought was that the
> documentation is confusing, or there is a real problem in this patch.
>

 Thanks for your quick response.

 The documentation is actually the most difficult part for me.

> As I understand it the frame has to be re-laid out each time the
> contents of the frame changes (an extra register becomes live or another
> spill slot is needed).  So saying that it is laid out once can't be
> right if (as I read it initially) you mean 'once per function' since I
> think it needs to be 'once for each time the frame contents changes'.
>
> Of course, things might be a bit different with LRA compared with
> reload, but I strongly suspect this is never going to be an 'exactly
> once per function' operation.
>

 Right.  It will be done 2 or 3 times for each function.
 LRA and reload behave identical in that respect.

 But each time reload changes something in the input data the
 INITIAL_EMIMINATION_OFFSET is called several times, and the results
 have to be consistent in each iteration.

 The frame layout function has no way to know if the frame layout
 is expected to change or not.

 Many targets use reload_completed as an indication when the frame layout
 may not change at all, but that is only an approximation.

> Can you clarify your meaning in the documentation please?
>

 I meant 'once' in the sense of 'once for each time the frame contents
 change'.

 Thus I'd change that sentence to:

 "This target hook allows the target to compute the frame layout once for
 each time the frame contents change and make use of the cached frame
 layout in @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing it
 on every invocation.  This is particularly useful for targets that have
 an expensive frame layout function.  Implementing this callback is
 optional."

>>>
>>> Thanks, that's pretty much what I expected would be the case.
>>>
>>> Could I suggest:
>>>
>>> This target hook is called once each time the frame layout needs to be
>>> recalculated.  The calculations can be cached by the target and can then
>>> be used by @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing the
>>> layout on every invocation of that hook.  This is particularly useful
>>> for targets that have an expensive frame layout function.  Implementing
>>> this callback is optional.
>>>
>>> R.
>>>

 Thanks
 Bernd.


> R.
>
>>
>> Thanks
>> Bernd.
>>
>> On 06/21/16 23:29, Jeff Law wrote:
>>> 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 

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

[Richard Earnshaw (lists)]

On 05/09/16 17:43, Bernd Edlinger wrote:
> Hi Richard,
> 
> what do you think of this patch, is it OK (with the suggested wording)?
> 

Bernd,

Apologies, this seems to have fallen through a crack.

I'm happy with this.  Does it still apply?

If so, I suggest applying it after a 24-hour cooling off period for any
final comments.

R.

> 
> Thanks
> Bernd.
> 
> On 08/05/16 16:06, Richard Earnshaw (lists) wrote:
>> On 05/08/16 13:49, Bernd Edlinger wrote:
>>> On 08/05/16 11:29, Richard Earnshaw (lists) wrote:
 On 04/08/16 22:16, Bernd Edlinger wrote:
> Hi,
>
> this patch introduces a new target hook that allows the target's
> INITIAL_ELIMINATION_OFFSET function to use cached values instead of
> re-computing the frame layout every time.
>
> I have updated the documentation a bit and hope it is clearer this time.
>
> It still needs a review by ARM port maintainers.
>
> If the ARM port maintainers find this patch useful, that would be fine.
>

 I need to look into this more, but my first thought was that the
 documentation is confusing, or there is a real problem in this patch.

>>>
>>> Thanks for your quick response.
>>>
>>> The documentation is actually the most difficult part for me.
>>>
 As I understand it the frame has to be re-laid out each time the
 contents of the frame changes (an extra register becomes live or another
 spill slot is needed).  So saying that it is laid out once can't be
 right if (as I read it initially) you mean 'once per function' since I
 think it needs to be 'once for each time the frame contents changes'.

 Of course, things might be a bit different with LRA compared with
 reload, but I strongly suspect this is never going to be an 'exactly
 once per function' operation.

>>>
>>> Right.  It will be done 2 or 3 times for each function.
>>> LRA and reload behave identical in that respect.
>>>
>>> But each time reload changes something in the input data the
>>> INITIAL_EMIMINATION_OFFSET is called several times, and the results
>>> have to be consistent in each iteration.
>>>
>>> The frame layout function has no way to know if the frame layout
>>> is expected to change or not.
>>>
>>> Many targets use reload_completed as an indication when the frame layout
>>> may not change at all, but that is only an approximation.
>>>
 Can you clarify your meaning in the documentation please?

>>>
>>> I meant 'once' in the sense of 'once for each time the frame contents
>>> change'.
>>>
>>> Thus I'd change that sentence to:
>>>
>>> "This target hook allows the target to compute the frame layout once for
>>> each time the frame contents change and make use of the cached frame
>>> layout in @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing it
>>> on every invocation.  This is particularly useful for targets that have
>>> an expensive frame layout function.  Implementing this callback is
>>> optional."
>>>
>>
>> Thanks, that's pretty much what I expected would be the case.
>>
>> Could I suggest:
>>
>> This target hook is called once each time the frame layout needs to be
>> recalculated.  The calculations can be cached by the target and can then
>> be used by @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing the
>> layout on every invocation of that hook.  This is particularly useful
>> for targets that have an expensive frame layout function.  Implementing
>> this callback is optional.
>>
>> R.
>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
 R.

>
> Thanks
> Bernd.
>
> On 06/21/16 23:29, Jeff Law wrote:
>> 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.

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

[Bernd Edlinger]

Hi Richard,

what do you think of this patch, is it OK (with the suggested wording)?


Thanks
Bernd.

On 08/05/16 16:06, Richard Earnshaw (lists) wrote:
> On 05/08/16 13:49, Bernd Edlinger wrote:
>> On 08/05/16 11:29, Richard Earnshaw (lists) wrote:
>>> On 04/08/16 22:16, Bernd Edlinger wrote:
 Hi,

 this patch introduces a new target hook that allows the target's
 INITIAL_ELIMINATION_OFFSET function to use cached values instead of
 re-computing the frame layout every time.

 I have updated the documentation a bit and hope it is clearer this time.

 It still needs a review by ARM port maintainers.

 If the ARM port maintainers find this patch useful, that would be fine.

>>>
>>> I need to look into this more, but my first thought was that the
>>> documentation is confusing, or there is a real problem in this patch.
>>>
>>
>> Thanks for your quick response.
>>
>> The documentation is actually the most difficult part for me.
>>
>>> As I understand it the frame has to be re-laid out each time the
>>> contents of the frame changes (an extra register becomes live or another
>>> spill slot is needed).  So saying that it is laid out once can't be
>>> right if (as I read it initially) you mean 'once per function' since I
>>> think it needs to be 'once for each time the frame contents changes'.
>>>
>>> Of course, things might be a bit different with LRA compared with
>>> reload, but I strongly suspect this is never going to be an 'exactly
>>> once per function' operation.
>>>
>>
>> Right.  It will be done 2 or 3 times for each function.
>> LRA and reload behave identical in that respect.
>>
>> But each time reload changes something in the input data the
>> INITIAL_EMIMINATION_OFFSET is called several times, and the results
>> have to be consistent in each iteration.
>>
>> The frame layout function has no way to know if the frame layout
>> is expected to change or not.
>>
>> Many targets use reload_completed as an indication when the frame layout
>> may not change at all, but that is only an approximation.
>>
>>> Can you clarify your meaning in the documentation please?
>>>
>>
>> I meant 'once' in the sense of 'once for each time the frame contents
>> change'.
>>
>> Thus I'd change that sentence to:
>>
>> "This target hook allows the target to compute the frame layout once for
>> each time the frame contents change and make use of the cached frame
>> layout in @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing it
>> on every invocation.  This is particularly useful for targets that have
>> an expensive frame layout function.  Implementing this callback is
>> optional."
>>
>
> Thanks, that's pretty much what I expected would be the case.
>
> Could I suggest:
>
> This target hook is called once each time the frame layout needs to be
> recalculated.  The calculations can be cached by the target and can then
> be used by @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing the
> layout on every invocation of that hook.  This is particularly useful
> for targets that have an expensive frame layout function.  Implementing
> this callback is optional.
>
> R.
>
>>
>> Thanks
>> Bernd.
>>
>>
>>> R.
>>>

 Thanks
 Bernd.

 On 06/21/16 23:29, Jeff Law wrote:
> 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  
>>
>>   * target.def (compute_frame_layout): New optional target hook.
>>   * doc/tm.texi.in (TARGET_COMPUTE_FRAME_LAYOUT): Add hook.
>>   * doc/tm.texi 

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

[Bernd Edlinger]

On 08/05/16 16:06, Richard Earnshaw (lists) wrote:
>
> Thanks, that's pretty much what I expected would be the case.
>
> Could I suggest:
>
> This target hook is called once each time the frame layout needs to be
> recalculated.  The calculations can be cached by the target and can then
> be used by @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing the
> layout on every invocation of that hook.  This is particularly useful
> for targets that have an expensive frame layout function.  Implementing
> this callback is optional.
>
> R.
>

Excellent!  I like this wording very much.


Thanks
Bernd.

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

[Richard Earnshaw (lists)]

On 05/08/16 13:49, Bernd Edlinger wrote:
> On 08/05/16 11:29, Richard Earnshaw (lists) wrote:
>> On 04/08/16 22:16, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> this patch introduces a new target hook that allows the target's
>>> INITIAL_ELIMINATION_OFFSET function to use cached values instead of
>>> re-computing the frame layout every time.
>>>
>>> I have updated the documentation a bit and hope it is clearer this time.
>>>
>>> It still needs a review by ARM port maintainers.
>>>
>>> If the ARM port maintainers find this patch useful, that would be fine.
>>>
>>
>> I need to look into this more, but my first thought was that the
>> documentation is confusing, or there is a real problem in this patch.
>>
> 
> Thanks for your quick response.
> 
> The documentation is actually the most difficult part for me.
> 
>> As I understand it the frame has to be re-laid out each time the
>> contents of the frame changes (an extra register becomes live or another
>> spill slot is needed).  So saying that it is laid out once can't be
>> right if (as I read it initially) you mean 'once per function' since I
>> think it needs to be 'once for each time the frame contents changes'.
>>
>> Of course, things might be a bit different with LRA compared with
>> reload, but I strongly suspect this is never going to be an 'exactly
>> once per function' operation.
>>
> 
> Right.  It will be done 2 or 3 times for each function.
> LRA and reload behave identical in that respect.
> 
> But each time reload changes something in the input data the
> INITIAL_EMIMINATION_OFFSET is called several times, and the results
> have to be consistent in each iteration.
> 
> The frame layout function has no way to know if the frame layout
> is expected to change or not.
> 
> Many targets use reload_completed as an indication when the frame layout
> may not change at all, but that is only an approximation.
> 
>> Can you clarify your meaning in the documentation please?
>>
> 
> I meant 'once' in the sense of 'once for each time the frame contents 
> change'.
> 
> Thus I'd change that sentence to:
> 
> "This target hook allows the target to compute the frame layout once for
> each time the frame contents change and make use of the cached frame
> layout in @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing it
> on every invocation.  This is particularly useful for targets that have
> an expensive frame layout function.  Implementing this callback is
> optional."
> 

Thanks, that's pretty much what I expected would be the case.

Could I suggest:

This target hook is called once each time the frame layout needs to be
recalculated.  The calculations can be cached by the target and can then
be used by @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing the
layout on every invocation of that hook.  This is particularly useful
for targets that have an expensive frame layout function.  Implementing
this callback is optional.

R.

> 
> Thanks
> Bernd.
> 
> 
>> R.
>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>> On 06/21/16 23:29, Jeff Law wrote:
 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  
>
>  * 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.

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

[Bernd Edlinger]

On 08/05/16 11:29, Richard Earnshaw (lists) wrote:
> On 04/08/16 22:16, Bernd Edlinger wrote:
>> Hi,
>>
>> this patch introduces a new target hook that allows the target's
>> INITIAL_ELIMINATION_OFFSET function to use cached values instead of
>> re-computing the frame layout every time.
>>
>> I have updated the documentation a bit and hope it is clearer this time.
>>
>> It still needs a review by ARM port maintainers.
>>
>> If the ARM port maintainers find this patch useful, that would be fine.
>>
>
> I need to look into this more, but my first thought was that the
> documentation is confusing, or there is a real problem in this patch.
>

Thanks for your quick response.

The documentation is actually the most difficult part for me.

> As I understand it the frame has to be re-laid out each time the
> contents of the frame changes (an extra register becomes live or another
> spill slot is needed).  So saying that it is laid out once can't be
> right if (as I read it initially) you mean 'once per function' since I
> think it needs to be 'once for each time the frame contents changes'.
>
> Of course, things might be a bit different with LRA compared with
> reload, but I strongly suspect this is never going to be an 'exactly
> once per function' operation.
>

Right.  It will be done 2 or 3 times for each function.
LRA and reload behave identical in that respect.

But each time reload changes something in the input data the
INITIAL_EMIMINATION_OFFSET is called several times, and the results
have to be consistent in each iteration.

The frame layout function has no way to know if the frame layout
is expected to change or not.

Many targets use reload_completed as an indication when the frame layout
may not change at all, but that is only an approximation.

> Can you clarify your meaning in the documentation please?
>

I meant 'once' in the sense of 'once for each time the frame contents 
change'.

Thus I'd change that sentence to:

"This target hook allows the target to compute the frame layout once for
each time the frame contents change and make use of the cached frame
layout in @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing it
on every invocation.  This is particularly useful for targets that have
an expensive frame layout function.  Implementing this callback is
optional."


Thanks
Bernd.


> R.
>
>>
>> Thanks
>> Bernd.
>>
>> On 06/21/16 23:29, Jeff Law wrote:
>>> 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  

  * 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 

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

[Richard Earnshaw (lists)]

On 04/08/16 22:16, Bernd Edlinger wrote:
> Hi,
> 
> this patch introduces a new target hook that allows the target's
> INITIAL_ELIMINATION_OFFSET function to use cached values instead of 
> re-computing the frame layout every time.
> 
> I have updated the documentation a bit and hope it is clearer this time.
> 
> It still needs a review by ARM port maintainers.
> 
> If the ARM port maintainers find this patch useful, that would be fine.
> 

I need to look into this more, but my first thought was that the
documentation is confusing, or there is a real problem in this patch.

As I understand it the frame has to be re-laid out each time the
contents of the frame changes (an extra register becomes live or another
spill slot is needed).  So saying that it is laid out once can't be
right if (as I read it initially) you mean 'once per function' since I
think it needs to be 'once for each time the frame contents changes'.

Of course, things might be a bit different with LRA compared with
reload, but I strongly suspect this is never going to be an 'exactly
once per function' operation.

Can you clarify your meaning in the documentation please?

R.

> 
> Thanks
> Bernd.
> 
> On 06/21/16 23:29, Jeff Law wrote:
>> 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  
>>>
>>> * 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
>>
>> changelog-frame-layout.txt
>>
>>
>> 2016-06-16  Bernd Edlinger  
>>
>>  * target.def (compute_frame_layout): New optional target hook.
>>  * doc/tm.texi.in 

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

[Bernd Edlinger]

Hi,

this patch introduces a new target hook that allows the target's
INITIAL_ELIMINATION_OFFSET function to use cached values instead of 
re-computing the frame layout every time.

I have updated the documentation a bit and hope it is clearer this time.

It still needs a review by ARM port maintainers.

If the ARM port maintainers find this patch useful, that would be fine.


Thanks
Bernd.

On 06/21/16 23:29, Jeff Law wrote:
> 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  
>>
>> * 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
2016-06-16  Bernd Edlinger  

* 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.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c	(revision 239144)
+++ gcc/config/arm/arm.c	(working copy)
@@ -81,6 +81,7 @@ static bool arm_const_not_ok_for_debug_p (rtx);
 static bool arm_needs_doubleword_align (machine_mode, const_tree);
 static int arm_compute_static_chain_stack_bytes (void);
 static arm_stack_offsets