Request for reviews (M): 6833129 : specjvm98 fails with NullPointerException in the compiler with -XX:DeoptimizeALot

[changpeng fang - Sun Microsystems - Santa Clara United States]

On 07/06/09 22:52, John Rose wrote:
> On Jul 6, 2009, at 5:21 PM, changpeng fang - Sun Microsystems - Santa 
> Clara United States wrote:
>
>> http://cr.openjdk.java.net/~cfang/6833129/webrev.01/
>
> Here's a nit:  is_restart() in scopeDesc.hpp should be a const function.
>
Yes. Don't know how could I missed that. Thanks.

> I'd like to see comment in each case demonstrating why that the values 
> captured in dummy_restart, and the throwaway restart in 
> javaClasses.cpp (which should be called dummy_restart also) don't matter.
>
They are not going to participate in restart/continue decision. Yes, it 
is better to write some comments there for better understanding and/or 
potential
future situation changes. Thanks.

> I'd still like to see the restart decision made by continuation_for 
> turned into an assert.  At least compare it with is_restart():
>
>>    pc = Interpreter::continuation_for(method(), bcp, 
>> callee_parameters, is_top_frame, use_next_mdp);
>> + assert(is_restart() == !use_next_mdp, "");
> If the assert fails, there may be a bug.
>
Yes, it is a little confusion here. I know you want the restart 
decisions all made by the compilers (the right approach and we will
definitely  do it).  You may  have already noticed that  the restart 
bits  are all  zero  in the C1 interface,  and  for  C2,  only
intrinsics Object.clone and Arrays.copyOf turned on their restart bits. 
As a result, this work just sets up the framework with the
necessary logics to fix this bug.

> -- John