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

[Roland Westrelin]

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.
>>