review for 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp

[Vladimir Kozlov]

Looks fine as far as I understand it and thank you for fixing loop predicates output.
One thing which was not clear is registers usage in 32-bit code in trace_method_handle() in methodHandles_x86.cpp. Yes, 
from caller of trace_method_handle() you can find it out but in trace_method_handle() it looks like mess.

Vladimir

On 4/29/11 3:29 PM, Tom Rodriguez wrote:
> http://cr.openjdk.java.net/~never/7009361
>
> 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
> Reviewed-by:
>
> In the invokedynamic world the signature of the method at an invoke
> bytecode might note be the same as the signature of the callee you
> finally end up in.  This all works correctly during normal execution
> but it can break the logic that deopt uses to layout the frames.  In
> particular on sparc we attempt to place the locals in the same
> location that the interpreter execution would have produced by using
> the callers expression stack address and callee->size_of_parameters()
> to figure out where the top of the live stack is.  If the size of the
> parameters at the original call site is less than the size of the
> parameters of the actual callee then the locals will end up top of the
> callers live expression stack.  The x86 version of this code places
> the locals at the bottom of the frame which keeps this from happening
> and I've modified sparc to do the same.  There's no reason they need
> to be in the same location.
>
> The other potential problem is that deopt also has logic that makes
> sure the existing caller is enlarged enough to account for the
> difference between the callee parameters and the callee locals.  In
> the current world we don't really need to enlarge this space because
> the method handle machinery also operates like the interpreter so it
> extends the caller frame when injecting extra arguments, thus making
> sure there's really enough space when we deopt.  Since it doesn't have
> to work this way I decided to fix this logic to grab the caller notion
> of the number of arguments and correct the last frame adjust using
> this value.
>
> Additionally the TraceMethodHandles logic was very broken in x86 so I
> fixed it.  It was mainly broken because some of the
> trace_method_handle calls are generated using an
> InterpreterMacroAssembler which has extra asserts in call_VM_leaf_base
> that don't really apply for the method handle adapters.  There were
> also problems with the number of arguments being passed in 64 bit.  I
> ended up moving super_call_VM_leaf up into MacroAssembler since
> there's no way to figure out that you are using an
> InterpreterMacroAssembler versus a MacroAssembler.
>
> To debug this I added a new printing function,
> JavaThread::print_frame_layout, that prints an annotated view of the
> stack memory similar to what the SA's memory viewer does.  It also
> includes some logic to check that the space owned by a frame isn't
> claimed by another frame.  That actually detects the original bug
> immediately.  It's not as full featured as it might be but it reports
> everything you need to know about interpreter frames.
>
> I also made a small change in loopPredicate because the ttyLocker was
> producing spurious output in the log all the time.
>
> Tested with original failing test from test and DeoptimizeALot testing
> on sparc.
>