[ping] RFR(S): 8156992: [ppc] Implement template interpreter stack overflow checks as on x86/sparc.

[Coleen Phillimore]

On 5/30/16 7:23 PM, David Holmes wrote:
> On 30/05/2016 5:28 PM, Lindenmaier, Goetz wrote:
>> Hi David,
>>
>>> -----Original Message-----
>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>> Sent: Samstag, 28. Mai 2016 00:10
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>>> dev at openjdk.java.net; Frederic Parain <frederic.parain at oracle.com>
>>> Subject: Re: [ping] RFR(S): 8156992: [ppc] Implement template 
>>> interpreter
>>> stack overflow checks as on x86/sparc.
>>>
>>> On 28/05/2016 1:35 AM, Lindenmaier, Goetz wrote:
>>>> Hi David,
>>>>
>>>> well, yes, Dean had similar questions.
>>>> X86/sparc load stack_base and stack_size, compute stack_end and then
>>> add the
>>>> Max(shadow, quard). All in assembly in the template interpreter 
>>>> prolog.
>>> On ppc,
>>>> we load it from the precomputed field stackoverflow_limit.  The 
>>>> value in
>>> that field
>>>> differed from what x86/sparc computed, which is the error I want to 
>>>> fix
>>> here.
>>>
>>> That all sounds a bit broken. :(
>> Broken as far as my change is concerned, or broken wrt. to the existing
>> implementation?
>
> The existing implementation.
>
>>>> I adapted the drawing to the change of that field.
>>> I still don't see how the drawing matches the code given the MAX usage.
>> If guard_zones > shadow zone, stack_overflow_limit points to 
>> stack_reserved_zone_base().
>> If guard_zones < shadow zone, stack_overflow_limit points to 
>> somewhere in the shadow zone.
>
> Ah! I was using frames view on the webrev and the "somewhere in here" 
> comment was off the page.

I'm having trouble with the same picture too.

The generate_stack_overflow_check() should be checking that the frame 
fits under the stack_guard_zone or stack_reserved_zone_base. Or what 
used to be yellow+red zones.

Why is it

   // Use the bigger size for banging.
   const int max_bang_size = (int)MAX2(JavaThread::stack_shadow_zone_size(),
JavaThread::stack_guard_zone_size());


Which would put the pointer for _stack_overflow_limit in the "somewhere 
in here" place.  But I don't get it.

This comment and variable name is wrong in the x86 code, I think, but 
this isn't your problem.

The code looks good, but we might need a comment.   And we should file 
an RFE to use Thread::_stack_overflow_limit for the other platforms.

Coleen
>
> Thanks,
> David
>
>> Best regards,
>>   Goetz.
>>
>>> I can't convince myself that any of this stuff is correct. Given
>>> Frederic was working in this area most recently I wonder if he can
>>> comment (but I know he may not be around at the moment).
>>>
>>> Thanks,
>>> David
>>>
>>>> Best regards,
>>>>   Goetz
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>>>> Sent: Donnerstag, 26. Mai 2016 00:36
>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>>>>> dev at openjdk.java.net
>>>>> Subject: Re: [ping] RFR(S): 8156992: [ppc] Implement template 
>>>>> interpreter
>>>>> stack overflow checks as on x86/sparc.
>>>>>
>>>>> Hi Goetz,
>>>>>
>>>>> I know you were having discussions with Dean in relation to this 
>>>>> but I
>>>>> really don't understand the change to the diagram in thread.hpp 
>>>>> nor the
>>>>> change to the definition of stackoverflow_limit. To me the limit 
>>>>> has to
>>>>> be the sum of the guard zone and shadow zone. With your MAX change
>>> the
>>>>> diagram doesn't even make sense AFAICS.
>>>>>
>>>>> ??
>>>>>
>>>>> David
>>>>>
>>>>> On 25/05/2016 6:07 PM, Lindenmaier, Goetz wrote:
>>>>>> Hi,
>>>>>>
>>>>>> could someone else have a look at this change?  I need a reviewer,
>>> please.
>>>>>> I also please need a sponsor.
>>>>>>
>>>>>> Best regards,
>>>>>>   Goetz.
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Lindenmaier, Goetz
>>>>>>> Sent: Mittwoch, 18. Mai 2016 12:02
>>>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>>>> Subject: RFR(S): 8156992: [ppc] Implement template interpreter 
>>>>>>> stack
>>>>>>> overflow checks as on x86/sparc.
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> When porting the template interpreter, we implemented a different
>>>>>>> approach to
>>>>>>> Stack overflow handling.  See also the detailed description in 
>>>>>>> the Jira
>>> bug.
>>>>>>>
>>>>>>> This change implements the stack overflow check as on x86/sparc.
>>>>>>> It requires simple shared changes, but only to code only used on 
>>>>>>> ppc.
>>>>>>> The changes should not affect the other platforms.
>>>>>>>
>>>>>>> Please review this change. I please need a sponsor.
>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8156922-
>>>>> ppcStackFix/webrev.01/
>>>>>>>
>>>>>>> Best regards,
>>>>>>>   Goetz.
>>>>>>