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

[Christian Thalinger]

This is a very nice cleanup too.

+   assert(bang_size_in_bytes >= frame_size_in_bytes, "stack bang size incorrect”);

I’m pretty sure this is almost always true but it might not (for whatever reason).  I don’t see much value in that assert.

src/share/vm/opto/compile.cpp:

+ int Compile::bang_size_in_bytes() const {
+   int callee_locals = method() != NULL ? method()->max_locals() : 0;
+   int interpreter_frame_size = _interpreter_frame_size;
+   return MAX2(interpreter_frame_size, frame_size_in_bytes());
+ }

callee_locals is unused.  Is there a reason you load _interpreter_frame_size into a local variable?

src/share/vm/asm/assembler.cpp:

!     int bang_end = (StackShadowPages+1)*page_size;

Why +1?

src/share/vm/ci/ciMethod.cpp:

+ int ciMethod::get_stack_effect_at_invoke(int bci, Bytecodes::Code code, int& inputs) {
+ int ciMethod::stack_effect_if_at_invoke(int bci) {

Either both with or without “get”.

Since this is very hard to test what testing did you do?

On Mar 6, 2014, at 3:08 AM, Roland Westrelin <roland.westrelin at oracle.com> 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.
> 
> http://cr.openjdk.java.net/~roland/8032410/webrev.01/
> 
> Roland.