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

[Christian Thalinger]

On Thu, 2010-04-08 at 15:50 -0700, Vladimir Kozlov wrote:
> Next new method is missing assert_different_registers:
> MacroAssembler::load_method_handle_vmslots

I added it to the x86 version too.

> 
> New parameter receiver_can_be_null is not used in profile_virtual_call().

Oops, I missed that during the port.  Thanks.

> 
> Could you rename Rtmp and Rdst into cache and tmp in
> 
> IntRtmperpreterMacroAssembler::get_index_at_bcp()
> 
> since it is confusing?
> I known that get_{2|4}_byte_integer_at_bcp() use Rtmp and Rdst
> but callers used cache and tmp.

Good point, I changed that.  Additionally I renamed the method to
get_cache_index_at_bcp to be in sync with x86.

> 
> 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).

> 
> 
> 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.

> 
> 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?

> 
> 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