Request for reviews (M): 6833129 : specjvm98 fails with NullPointerException in the compiler with -XX:DeoptimizeALot
changpeng fang - Sun Microsystems - Santa Clara United States
Changpeng.Fang at Sun.COM
Thu Jul 9 14:41:36 PDT 2009
On 07/07/09 13:33, Tom Rodriguez wrote:
> I think the term reexecute should be used in place of restart since
> that terminology is used elsewhere. Actually I think should_reexecute
> is better than is_reexecute as well.
I will use reexecute and should_reexecute throughout the change. Thanks.
> I don't really like that the restart bit is associated with the bci.
> It implies that any scope can be reexecuted when it fact it's only the
> topmost one that can be. Given how these describing scopes is
> structured I'm not sure I see a better way though. I don't see any
> asserts to enforce this for the scopeDescs either.
Do you mean assert like this :
assert(!_reexecute || is_top(), "reexecute only applies to top scope");
If yes, then I must have something wrong in setting up the reexecute bit
in JVMState in C2.
> The printing forms for ScopeDesc and JVMState should indicate if this
> is a restarted bytecode or not. The SA also needs to be updated to
> read these modified ScopeDescs.
There may be some bugs in JVMState::dump()! I will investigate it and
add the reexecute bit.
>>> We talked about using PreserveJVMState pjvms(this) instead
>>> of restoring stack and restart flag in intrinsics?
>>> Why you did not use it?
>> Just as we discussed a moment ago, I added an assert restart false
>> we set it to true.
> I think manually toggling the restart bit back and forth should be
> avoided. Preserve the original and pass on the modified one or have a
> helper object that toggles the bit in it's constructor/destructor.
I prefer "Preserve the original and pass on the modified one". We can
not use PreserveJVMState only for the purpose of preserving _sp (the
map will be changed during the scope of preservation, and
may cause undesired consequence).
>> Here is the updated webrev link!
>>> changpeng fang - Sun Microsystems - Santa Clara United States wrote:
>>>> The problem is in intrinsics Object.clone and Arrays.copyOf. When
>>>> de-optimization occurs on the slow path for array/instance
>>>> allocation, the Interpreter will continue execution in next bc
>>>> after the slow allocation calls without actual copying.
>>>> Add a restart bit in debuginfo to direct the deopt to restart
>>>> execution of the bytecode that invokes Object.clone/Arrays.copyOf.
>>>> The restart bit is set up in Inline Intrinsics of
>>>> Passed specjvm98, JPRT and the test case of clone in the bug report.
More information about the hotspot-compiler-dev