Re: [PING] 3 patches waiting for approval/review

[Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>]

On 16/10/13 22:25, Jeff Law wrote:
> On 10/11/13 11:23, Andreas Krebbel wrote:
>> On 10/10/13 18:41, Jeff Law wrote:
>>> On 10/10/13 04:00, Andreas Krebbel wrote:
>>>> On 09/10/13 21:46, Jeff Law wrote:
>>>>> On 08/21/13 03:21, Andreas Krebbel wrote:
>>>>>> [RFC] Allow functions calling mcount before prologue to be leaf functions
>>>>>> http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00993.html
>>>>> I don't think this is necessarily correct for all targets.  ISTM the
>>>>> ability to consider a function calling mcount as a leaf needs to be a
>>>>> property of the target.
>>>>
>>>> We have already "profile_before_prologue" as a target property. Shouldn't this be enough to decide
>>>> upon this? When a function calls mcount before the prologue it shouldn't matter whether the function
>>>> is leaf or not.
>>> I don't think so, I think it'd break the PA's 32 bit ABI, maybe the 64
>>> bit ABI as well.  It's the caller's responsibility to build a mini stack
>>> frame if the function makes any calls.  If the code in the prologue
>>> expander uses "leafness" to make the decision about whether or not to
>>> allocate the mini frame, then it'd do the wrong thing here.
>>
>> Since it seems to be about PROFILE_HOOKS vs FUNCTION_PROFILER targets what about the following patch:
> It's not about PROFILE_HOOKS vs FUNCTION_PROFILER, but the ABI and how 
> the backend changes its behavior on the return value of leaf_function_p.
> 
> Effectively you want to have functions which are not leaf functions 
> (they call mcount) be treated as leaf functions.  I have great concerns 
> about the safety of allowing that without exports on each port weighing 
> in on its correctness for their port.
> 
> 
> I still really feel this should be a target hook that is off by default 
> so that the target maintainers can audit their target to ensure it 
> operates correctly.
> 
> Maybe I'm missing something, so perhaps another approach.  Can you 
> explain why you think it is safe to ignore calls to mcount when trying 
> to determine if a function is a leaf or not?

In general it is not safe to ignore calls to mcount. But if a target does insert the call to mcount
before the function prologue then mcount will not use the stack space allocated by the current
function. It will instead use the stack space allocated by the caller for the current function. The
current function can still be a leaf function since the call does not happen within the scope of its
stack frame.

The difference between PROFILE_HOOKS and FUNCTION_PROFILER is that for the first the call to mcount
is inserted always after the function prologue no matter what the backend returns for the
profile_before_prologue target hook.

Bye,

-Andreas-


> 
> 
> BTW, there is a hunk of this patch that can and should go forward now 
> rather than later.  Specifically removing the profile_arc_flag part of 
> the test.  If you wanted to push that forward immediately, I'd support 
> that and you should consider it pre-approved.

Ok. Thanks.

> 
> 
> jeff
>