review for 7045514: SPARC assembly code for JSR 292 ricochet frames
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu Jun 2 09:20:48 PDT 2011
On 6/1/11 9:10 PM, Tom Rodriguez wrote:
>
> On Jun 1, 2011, at 6:15 PM, Vladimir Kozlov wrote:
>
>> Christian is hero to write all this :)
>
> Yep.
>
>>
>> Did you test with -XX:-EnableInvokeDynamic (off)?
>
> No. Are you asking if we can run normal programs with -EnableInvokeDynamic?
Yes, this flag is on by default so I want to know if we can run with it off.
>
>>
>> methodHandles_sparc.cpp:
>>
>> Can you do the decrement of slot_num as separate expressions?:
>
> -= is the same as predecrement not postdecrement, right?
I am not sure, this is why is why I asked to separate it.
>
>>
>> + intptr_t* loc =&base[slot_num -= 1];
>
> int slot_num = slot_count - 1;
> intptr_t* loc =&base[slot_num];
>
>>
>> + loc =&base[slot_num -= type2size[ptype]];
>
> slot_num -= type2size[ptype];
> loc =&base[slot_num];
>
Good.
>>
>> In load_conversion_vminfo() could you just use the same assert (CONV_VMINFO_SHIFT == 0) as next method and simplify address expression.
>
> This?
>
> void MethodHandles::load_conversion_vminfo(MacroAssembler* _masm, Address conversion_field_addr, Register reg) {
> assert(CONV_VMINFO_SHIFT == 0, "preshifted");
> assert(CONV_VMINFO_MASK == right_n_bits(BitsPerBte), "else change type of following load");
> __ ldub(conversion_field_addr.plus_disp(BytesPerInt - 1), reg);
> }
Good.
>
>>
>> Could you move following verification code under first condition?:
>>
>> + if (collect_count_constant>= 0) {
>> + collect_count = collect_count_constant;
>> + } else {
>> + __ load_method_handle_vmslots(O1_collect_count, G3_method_handle, O2_scratch);
>> + collect_count = O1_collect_count;
>> + }
>> + #ifdef ASSERT
>> + if (VerifyMethodHandles&& collect_count_constant>= 0) {
>> + BLOCK_COMMENT("verify collect_count_constant {");
>> + __ load_method_handle_vmslots(O3_scratch, G3_method_handle, O2_scratch);
>>
>> The same for dest_slot_constant verification.
>
> I can join them but I think I might swap the conditions.
Swap is fine.
Thanks,
Vladimir
>
> tom
>
>>
>> Thanks,
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> http://cr.openjdk.java.net/~never/7045514
>>> 2389 lines changed: 1833 ins; 363 del; 193 mod; 46455 unchg
>>> 7045514: SPARC assembly code for JSR 292 ricochet frames
>>> Reviewed-by:
>>> This is the complete support for Ricochet frames on sparc. Christian
>>> did all the work and testing and I just did some final testing and bug
>>> fixing.
>>> A potential issue with checkcasts reusing locals in
>>> methodHandleWalk.cpp was fixed. Comments weren't being transferred
>>> onto the MethodHandlesAdapterBlob. A derived oop issue was found
>>> where an assert was complaining that an offset was too large but
>>> there's no real restriction on the offset of derived oops so I
>>> disabled the assert. ResourceMarks were added in verification logic.
>>> A verify_vmargslot call was verifying against the wrong signature
>>> resulting in occasional incorrect exceptions. I updated the
>>> MacroAssembler::debug assertion messages to include the passed in
>>> message. Many of blob declarations were moved into shared code so
>>> that we don't have to replicate code. Some x86 method handles code
>>> was changed to make signatures match.
>>> Currently it passes all the jdk regression tests on sparc but I can't
>>> run any of the others because of version mismatches between the JDK
>>> and the tests. Earlier versions ran those as well they had
>>> previously. I also ran the jruby tests and those seemed clean as well.
>
More information about the hotspot-compiler-dev
mailing list