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

[Roman Kennke]

Am 14.05.2018 um 13:19 schrieb Andrew Dinn:
> 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.
> 
> However, I am left wondering why this is not a problem on other
> architectures? Indeed, looking at AArch64 I believe it is actually a
> serious headache.

It is, and I have a solution here:
cr.openjdk.java.net/~rkennke/interp_primitives_aarch64/webrev.00

I will propose this for upstream soon.

 Method aastore looks like this:
> 
>   Address element_address(r4, arrayOopDesc::base_offset_in_bytes(T_OBJECT));
> 
>   index_check(r3, r2);     // kills r1
>   __ lea(r4, Address(r3, r2, Address::uxtw(UseCompressedOops? 2 : 3)));
>   . . .
> 
>   // Get the value we will store
>   __ ldr(r0, at_tos());
>   // Now store using the appropriate barrier
>   do_oop_store(_masm, element_address, r0, IN_HEAP | IN_HEAP_ARRAY);
> 
> The Address for r4 only includes a displacement and the index register
> offset has to be explicitly added to the base register using an lea. Of
> course, that's because AArch64 doesn't like an Address with both a disp
> and a index register. You can have one or the other but not both.
> 
> So, if the abstraction you are relying on is to rely on do_oop_store to
> unpack an object base address, offset and index register from its
> Address argument then I am afraid you are on to a loser. What do you
> propose to do about that? and should it not be done as part of this fix?


For AArch64 it will basically reshuffle address calculation from
currently (base + index) + disp (which we can't resolve) to base +
(index + disp) which we can.

The alternative would be to explicitely drag the base_obj through all
the BarrierSetAssembler calls. So far I could live without it (x86 and
aarch64).

Roman