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

[Roman Kennke]

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?

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