Review Request: Zero JSR 292 support

[Christian Thalinger]

On Apr 5, 2011, at 4:04 PM, Gary Benson wrote:
> Christian Thalinger wrote:
>> On Apr 1, 2011, at 4:33 PM, Gary Benson wrote:
>>> This webrev adds support for JSR 292 to Zero:
>>> 
>>> http://cr.openjdk.java.net/~gbenson/zero-jsr292-01/
>> 
>> hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp:
>> 
>> +    // NB the x86 code for this (in methodHandles_x86.cpp, search for
>> +    // "genericInvoker") is really really odd.  I'm hoping it's trying
>> +    // to accomodate odd VM/class library combinations I can ignore.
>> 
>> Do you mean the code around sorry_no_invoke_generic?
> 
> Yeah, this bit:
> 
>  // load up an adapter from the calling type (Java weaves this)
>  __ load_heap_oop(rdx_temp, Address(rax_mtype, __ delayed_value(java_lang_invoke_MethodType::form_offset_in_bytes, rdi_temp)));
>  Register rdx_adapter = rdx_temp;
>  // __ load_heap_oop(rdx_adapter, Address(rdx_temp, java_lang_invoke_MethodTypeForm::genericInvoker_offset_in_bytes()));
>  // deal with old JDK versions:
>  __ lea(rdi_temp, Address(rdx_temp, __ delayed_value(java_lang_invoke_MethodTypeForm::genericInvoker_offset_in_bytes, rdi_temp)));
>  __ cmpptr(rdi_temp, rdx_temp);
>  Label sorry_no_invoke_generic;
>  __ jcc(Assembler::below, sorry_no_invoke_generic);
> 
> What I did in Zero is the commented out line:
> 
>  oop adapter = java_lang_invoke_MethodTypeForm::genericInvoker(form);
> 
> The not commented-out bit looks like its C++ equivalent would be:
> 
>  if ((intptr_t) adapter < (intptr_t) form) {
>    // what sorry_no_invoke_generic does
>  }
> 
> but I couldn't get it to work.  What's it for?

I think this code can go away when we remove the transitional JSR 292 support.  Right, John?

> 
>> hotspot/src/share/vm/interpreter/bytecodeInterpreter.cpp:
>> 
>> +          assert(false, "Should have thrown incompatible class change exception");
>> 
>> I'd use ShouldNotReachHere instead.
> 
> That happens in a lot of places in BytecodeInterpreter::run.  How
> about I make another webrev that changes them all?

In the current version?  I only find one at line 1722.

-- Christian