Request for reviews (XL): 6829193: JSR 292 needs to support SPARC

[Vladimir Kozlov]

On 4/9/10 8:13 AM, Christian Thalinger wrote:
>>
>> MethodHandles::generate_method_handle_interpreter_entry():
>>
>> !     Register tem = G5_method;
>> !     for (jint* pchase = methodOopDesc::method_type_offsets_chain(); (*pchase) != -1; pchase++) {
>> !       __ ld_ptr(tem, *pchase, G5_method_type);
>> !       tem = G5_method_type;       // yes, it's the same register...
>>               ^ it is loop invariant, why you put it inside?
>> !     }
>
> I'm not sure I understand.  G5_method_type is loaded in the ld-ptr
> instruction from Address(tem, *pchase).

tem = G5_method_type is register renaming - it is not moving value,
so you don't need to do renaming inside loop.

>
>>
>>
>> MethodHandles::generate_method_handle_interpreter_entry() has next
>> code but I don't see O1 initialization
>> and the comment says "O0, O1: garbage temps, blown away"
>>
>> !   // given the MethodType, find out where the MH argument is buried
>> !   __ ld_ptr(G5_method_type, __ delayed_value(java_dyn_MethodType::form_offset_in_bytes, O1_scratch), O0_argslot);
>> !   __ ld(O0_argslot, __ delayed_value(java_dyn_MethodTypeForm::vmslots_offset_in_bytes, O1_scratch), O0_argslot);
>> !   __ ld_ptr(__ argument_address(O0_argslot), G3_method_handle);
>> !
>> !   __ check_method_handle_type(G5_method_type, G3_method_handle, O1_scratch, wrong_method_type);
>> !   __ jump_to_method_handle_entry(G3_method_handle, O1_scratch);
>> !
>> !   return entry_point;
>> ! }
>
> It's set in delayed_value.

OK, I see it now.

>
>>
>> Also MacroAssembler::load_method_handle_vmslots() has next code which
>> looks similar to above but has the check "!= 0"
>>
>> !   if (java_dyn_MethodHandle::vmslots_offset_in_bytes() != 0) {
>> !     // hoist vmslots into every mh to avoid dependent load chain
>> !     ld(    Address(mh_reg,    delayed_value(java_dyn_MethodHandle::vmslots_offset_in_bytes, temp_reg)),   vmslots_reg);
>> !   } else {
>> !     Register temp2_reg = vmslots_reg;
>> !     ld_ptr(Address(mh_reg,    delayed_value(java_dyn_MethodHandle::type_offset_in_bytes, temp_reg)),      temp2_reg);
>> !     ld_ptr(Address(temp2_reg, delayed_value(java_dyn_MethodType::form_offset_in_bytes, temp_reg)),        temp2_reg);
>> !     ld(    Address(temp2_reg, delayed_value(java_dyn_MethodTypeForm::vmslots_offset_in_bytes, temp_reg)), vmslots_reg);
>> !   }
>
> You want me to reuse that code?

No. I was confused that in load_method_handle_vmslots() there is separate code when vmslots_offset_in_bytes() != 0 and 
generate_method_handle_interpreter_entry() does not have such code.

Vladimir

>
>>
>> In verify_argslot() and other places where you are comparing 64 bit
>> values (in 64 bit VM) you need to use brx() instead of br().
>
> Done.
>
> Here is the updated webrev:
>
> http://cr.openjdk.java.net/~twisti/6829193/webrev.02/
>
> -- Christian
>