Request for review (M): 7102657: JSR 292: C1 deoptimizes unlinked invokedynamic call sites infinitely

[Christian Thalinger]

On Oct 20, 2011, at 6:52 PM, Tom Rodriguez wrote:

> 
> On Oct 20, 2011, at 5:32 AM, Christian Thalinger wrote:
> 
>> http://cr.openjdk.java.net/~twisti/7102657/
>> 
>> 7102657: JSR 292: C1 deoptimizes unlinked invokedynamic call sites infinitely
>> Reviewed-by:
>> 
>> The current logic for invokedynamic in LIRGenerator::do_Invoke doesn't
>> actually initiate a deoptimization.  We need some new logic to do
>> that.
> 
> c1_Runtime.cpp:
> 
> This comment isn't true:
> 
> +   // It's possible the nmethod was invalidated in the last
> +   // safepoint, but if it's still alive then make it not_entrant.
> 
> You've just come from the compiled code so it's required that the caller exists.  Can you change this test to an assert?

Done.

> 
> Also I know you copied this comment:
> 
> +   // Bypass VM_DeoptimizeFrame and deoptimize the frame directly.
> 
> but it's no really right in any place it's used.  It corresponds to the old code where you could pick how to call the deopt code.  I modified it a while back so that there's only one way and that interface chooses how to call into the deoptimization code.  Can you just correct it to "// Deoptimize the caller frame".

I also changed the comment from where I copied it.

> 
> In the stubs, you don't need to explicit logic to dispatch to the deopt blob.  The normal return generated by ~StubFrame will return to the deopt blob because the deoptimize call has rewritten the return address.  That's how all stubs that get deoptimized end up returning to the deopt blob.

Hmm.  I changed the stub to return instead of dispatching to the deopt blob and that produces wrong results with my test case.  Are you sure the return address gets rewritten correctly?

> 
> Otherwise it looks good.

Thanks.

-- Chris

> 
> tom
> 
>> 
>> src/share/vm/c1/c1_Runtime1.cpp
>> src/share/vm/c1/c1_Runtime1.hpp
>> src/cpu/sparc/vm/c1_CodeStubs_sparc.cpp
>> src/cpu/sparc/vm/c1_Runtime1_sparc.cpp
>> src/cpu/x86/vm/c1_CodeStubs_x86.cpp
>> src/cpu/x86/vm/c1_Runtime1_x86.cpp
>> 
>