review for 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
vladimir.kozlov at oracle.com
Mon May 2 10:24:10 PDT 2011
Tom Rodriguez wrote:
> On Apr 29, 2011, at 11:05 PM, Vladimir Kozlov wrote:
>> 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.
> What would you like to see?
Register naming something like r_method_handle and r_saved_sp to have the same
call code for 32- and 64-bit VM. Also why you move values to c_rarg?
super_call_VM_leaf() will move them again.
>> On 4/29/11 3:29 PM, Tom Rodriguez wrote:
>>> 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
>>> 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.
More information about the hotspot-compiler-dev