RFR: JDK-8202016: Use obj+offset in interpreter array access

[B. Blaser]

On 15 May 2018 at 11:26, Roman Kennke <rkennke at redhat.com> wrote:
> Am 14.05.2018 um 23:47 schrieb B. Blaser:
>> Hi,
>>
>> On 14 May 2018 at 13:19, Andrew Dinn <adinn at redhat.com> wrote:
>>> On 14/05/18 11:15, Roman Kennke wrote:
>>>> Am 07.05.2018 um 21:47 schrieb Roman Kennke:
>>>>> In the TemplateTable::aastore() and
>>>>> InterpreterMacroAssembler::load_resolved_reference_at_index(), the
>>>>> element address is flattened, and then passed to the BarrierSetAssembler
>>>>> for actual access. GCs like Shenandoah need the original obj though.
>>>>>
>>>>> The proposed change replaces the flattening with base+index+disp
>>>>> addressing, and removes the shift and add computations. In the case of
>>>>> aastore, we need to re-fetch the rcx/index from the stack because it
>>>>> gets trashed on the way.
>>>>>
>>>>> Testing: passed: hotspot/jtreg:tier1
>>>>>
>>>>> Can I please get a review?
>>>>> Thanks, Roman
>>> Well, that x86 code change looks ok/ish/ -- although the need to reload
>>> from the stack is a tad disappointing.
>>
>> Maybe I'm a bit late, but I think we could use r8 & r9 to avoid some
>> unnecessary stack access, as next.
>> What do you think (tier1 seems OK)?
>>
> Yes, that is a good idea and I believe it would work. I think it can be
> improved even further by only moving it out to r8/r9 once, and use those
> registers for addressing. Do you want to file a followup-issue and do that?

Yes, good suggestion.
I'll provide an improved patch and create a JBS issue.

Thanks for your feedback,
Bernard


> Thanks, Roman
>
>
>> Regards,
>> Bernard
>>
>>
>> diff -r 24151f48582b src/hotspot/cpu/x86/templateTable_x86.cpp
>> --- a/src/hotspot/cpu/x86/templateTable_x86.cpp    Mon May 14 21:56:07
>> 2018 +0200
>> +++ b/src/hotspot/cpu/x86/templateTable_x86.cpp    Mon May 14 23:03:40
>> 2018 +0200
>> @@ -1098,6 +1098,11 @@
>>    __ movl(rcx, at_tos_p1()); // index
>>    __ movptr(rdx, at_tos_p2()); // array
>>
>> +#ifdef _LP64
>> +  __ movptr(r8, rax);
>> +  __ movl(r9, rcx);
>> +#endif
>> +
>>    Address element_address(rdx, rcx,
>>                            UseCompressedOops? Address::times_4 :
>> Address::times_ptr,
>>                            arrayOopDesc::base_offset_in_bytes(T_OBJECT));
>> @@ -1125,8 +1130,13 @@
>>    __ bind(ok_is_subtype);
>>
>>    // Get the value we will store
>> +#ifdef _LP64
>> +  __ movptr(rax, r8);
>> +  __ movl(rcx, r9);
>> +#else
>>    __ movptr(rax, at_tos());
>>    __ movl(rcx, at_tos_p1()); // index
>> +#endif
>>    // Now store using the appropriate barrier
>>    do_oop_store(_masm, element_address, rax, IN_HEAP_ARRAY);
>>    __ jmp(done);
>>
>>
>>> [...]
>>>
>>> regards,
>>>
>>>
>>> Andrew Dinn
>>> -----------
>>> Senior Principal Software Engineer
>>> Red Hat UK Ltd
>>> Registered in England and Wales under Company Registration No. 03798903
>>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>
>