RFR(L): 8032410: compiler/uncommontrap/TestStackBangRbp.java times out on Solaris-Sparc V9

[Volker Simonis]

Hi,

thanks for thinking about PPC.
Goetz will have a look at this tomorrow.
If it's not too urgent, it would be nice if you could wait with
pushing until we get it running on PPC64 as well.

Thanks,
Volker


On Tue, Mar 11, 2014 at 5:35 PM, Roland Westrelin
<roland.westrelin at oracle.com> wrote:
>
> Hi Vladimir,
>
>> Changes are good in general.
>
> Thanks for reviewing this.
>
>> I don't see corresponding changes in MachPrologNode in
>> src/cpu/ppc/vm/ppc.ad. Do we need changes there?
>
> We must, I guess. I forgot about c2 ppc. I'll look into it.
>
>> src/share/vm/opto/output.cpp should be
>>
>> +            DEBUG_ONLY(|| true)));
>
> Indeed. Thanks for spotting that.
>
>> On 3/6/14 3:08 AM, Roland Westrelin wrote:
>>> This test causes a deadlock because when the stack bang in the deopt
>>> or uncommon trap blobs triggers an exception, we throw the exception
>>> right away even if the deoptee has some monitors locked. We had
>>> several issues recently with the stack banging in the deopt/uncommon
>>> trap blobs and so rather than add more code to fix stack banging on
>>> deoptimization, this change removes the need for stack banging on
>>> deoptimization as discussed previously:
>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2014-February/013519.html
>>>
>>> The compilers compute by how much deoptimization would bang the stack
>>> at every possible deoptimization points in the compiled code and use
>>> the worst case to generate the stack banging in the nmethod. In debug
>>> builds, the stack banging code is still performed in the
>>> deopt/uncommon trap blobs but only to verify that the compiled code
>>> has done the stack banging correctly. Otherwise, the stack banging
>>> from deoptimization causes the VM to abort.
>>>
>>> This change contains some code
>>> refactoring. AbstractInterpreter::size_activation() is currently
>>> implemented as a call to AbstractInterpreter::layout_activation() but
>>> on most platforms, the logic to do the actual lay out of the
>>> activation and the logic to calculate its size are largely
>>> independent and having both done by layout_activation() feels wrong
>>> to me and error prone. I made AbstractInterpreter::size_activation()
>>> and AbstractInterpreter::layout_activation() two independent methods
>>> that share common helper functions if some code needs to be shared. I
>>> dropped unnecessary arguments to size_activation() in the current
>>> implementation as well. I also made it a template method so that it
>>> can be called with either a Method* (from the deoptimization code) or
>>> a ciMethod* (from the compilers).
>>>
>>> I created src/cpu/x86/vm/templateInterpreter_x86.cpp to put code that is common to 32 and 64 bit x86.
>>>
>>> This change in AbstractAssembler::generate_stack_overflow_check():
>>>
>>> 137     int bang_end = (StackShadowPages+1)*page_size;
>>>
>>> is so that the stack banging code from the deopt/uncommon trap blobs
>>> and in the compiled code are consistent. Let’s say frame size is less
>>> than 1 page. During deoptimization, we bang sp+1 page and then sp+2
>>> pages … sp+(StackShadowPages+1) pages. If the frame size is > 1 page
>>> and <= 2 pages, we bang sp+1page, sp+2pages and then sp+3pages …
>>> sp+(ShadowPages+2) pages. In the compiled code, to be consistent, for
>>> a frame size of less than 1 page we need to bang
>>> sp+(StackShadowPages+1) pages, if it’s > 1 page and <= 2 pages then
>>> we need to bang sp+(StackShadowPages+2) pages etc.
>>
>> With +1 you will touch yellow page because it is inclusive if I read it
>> right:
>>
>>    while (bang_offset <= bang_end) {
>>
>> Can you test with StackShadowPages=1?
>
> Are you suggesting I run with StackShadowPages=1 to check if:
>
> 137     int bang_end = (StackShadowPages+1)*page_size;
>
> is ok?
> What would I run with StackShadowPages=1? The hotspot-comp regression
> tests? All testing?
>
> Do you agree that if I revert to:
>
> 137     int bang_end = StackShadowPages*page_size;
>
> I need to change stack banging in the deopt/uncommon trap blobs to bang
> one less page?
>
> Roland.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> http://cr.openjdk.java.net/~roland/8032410/webrev.01/
>>>
>>> Roland.
>>>