On Fri, Jun 22, 2018 at 1:57 PM, David Holmes <david.hol...@oracle.com> wrote:
> On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
>>
>> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes <david.hol...@oracle.com>
>> wrote:
>>>
>>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
>>>>
>>>>
>>>> On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz <marti...@google.com
>>>> <mailto:marti...@google.com>> wrote:
>>>>
>>>> Hi David and build-dev folk,
>>>>
>>>> After way too much build/hotspot hacking, I have a better fix:
>>>>
>>>> clang inlined os::current_stack_pointer into its caller __in the
>>>> same translation unit___ (that could be fixed in a separate change)
>>>> so of course in this case it didn't have to follow the ABI. Fix is
>>>> obvious in hindsight:
>>>>
>>>> -address os::current_stack_pointer() {
>>>> +NOINLINE address os::current_stack_pointer() {
>>>>
>>>>
>>>> If y'all like the addition of NOINLINE, it should probably be added to
>>>> all
>>>> of the 14 variants of os::current_stack_pointer.
>>>> Gives me a chance to try out the submit repo.
>>>
>>>
>>>
>>> I can't help but think other platforms actually rely on it being inlined
>>> so
>>> that it really does return the stack pointer of the method calling
>>> os::current_stack_pointer!
>>>
>>
>> But we only inline today if caller is in the same translation unit and
>> builds with optimization, no?
>
>
> Don't know, but Martin's encountering a case where it is being inlined - is
> that likely to be unique for some reason?
>
Okay I may have confused myself.
My original reasoning was: A lot of calls to
os::current_stack_pointer() today happen not-inlined. That includes
calls from outside os_<os>_<cpu>.cpp, and calls from inside
os_<os>_<cpu>.cpp if slowdebug. Hence, since the VM - in particular
the slowdebug one - seems to work fine, it should be okay to mark
os::current_stack_pointer() unconditionally as NOINLINE.
However, then I saw that the only "real" function (real meaning not
just asserting something) using os::current_stack_pointer() and living
in the same translation unit is os::current_frame(), e.g. on linux:
frame os::current_frame() {
intptr_t* fp = _get_previous_fp();
frame myframe((intptr_t*)os::current_stack_pointer(),
(intptr_t*)fp,
CAST_FROM_FN_PTR(address, os::current_frame));
if (os::is_first_C_frame(&myframe)) {
// stack is not walkable
return frame();
} else {
return os::get_sender_for_C_frame(&myframe);
}
}
how does this even work if os::current_stack_pointer() is not inlined?
In slowdebug? Would the frame object in this case not contain the SP
from the frame built up for os::current_stack_pointer()?
So, now I wonder if making os::current_stack_pointer() NOINLINE would
break os::current_frame() - make if produce frames with a slightly-off
SP. os::current_frame() is used e.g. in NMT. So, I wonder if NMT still
works if os::current_stack_pointer is made NOINLINE, or in slowdebug.
> I never assume the compiler does the obvious these days :) or that there
> can't be clever link-time tricks as well.
Oh. I did not think of that at all, you are right.
>
> Maybe the safest thing to do is to only make a change for the clang case and
> leave everything else alone.
>
It would be better to know for sure, though.
..Thomas
> David
> -----
>
>>
>> ..Thomas
>>
>>> David
