RFR: JDK-8203172: Primitive heap access for interpreter BarrierSetAssembler/aarch64

[Roman Kennke]

Ok, right. Very good catch!

This should do it, right? Sorry, I couldn't easily make an incremental diff:

http://cr.openjdk.java.net/~rkennke/JDK-8203172/webrev.01/

Unfortunately, I cannot really test it because of:
http://mail.openjdk.java.net/pipermail/aarch64-port-dev/2018-May/005843.html

Roman


> Hi Roman,
> 
> Oh man, I was hoping I would never have to look at jni fast get field
> again. Here goes...
> 
>  93   speculative_load_pclist[count] = __ pc();   // Used by the
> segfault handler
>  94   __ access_load_at(type, IN_HEAP, noreg /* tos: r0/v0 */,
> Address(robj, roffset), noreg, noreg);
>  95
> 
> I see that here you load straight to tos, which is r0 for integral
> types. But r0 is also c_rarg0. So it seems like if after loading the
> primitive to r0, the subsequent safepoint counter check fails, then the
> code will revert back to a slowpath call, but this time with c_rarg0
> clobbered, leading to a broken JNI env pointer being passed in to the
> slow path C function. That does not seem right to me.
> 
> This JNI fast get field code is so error prone. :(
> 
> Unfortunately, the proposed API can not load floating point numbers to
> anything but ToS, which seems like a problem in the jni fast get field
> code.
> I think to make this work properly, you need to load integral types to
> result and not ToS, so that you do not clobber r0, and rely on ToS being
> v0 for floating point types, which does not clobber r0. That way we can
> dance around the issue for now I suppose.
> 
> Thanks,
> /Erik
> 
> On 2018-05-14 22:23, Roman Kennke wrote:
>> Similar to x86
>> (http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-May/032114.html)
>> here comes the primitive heap access changes for aarch64:
>>
>> http://cr.openjdk.java.net/~rkennke/JDK-8203172/webrev.00/
>>
>> Some notes:
>> - array access used to compute base_obj + index, and then use indexed
>> addressing with base_offset. This means we cannot get base_obj in the
>> BarrierSetAssembler API, but we need that, e.g. for resolving the target
>> object via forwarding pointer. I changed (base_obj+index)+base_offset to
>> base_obj+(index+base_offset) in all the relevant places.
>>
>> - in jniFastGetField_aarch64.cpp, we are using a trick to ensure correct
>> ordering field-load with the load of the safepoint counter: we make them
>> address dependend. For float and double loads this meant to load the
>> value as int/long, and then later moving those into v0. This doesn't
>> work when going through the BarrierSetAssembler API: it loads straight
>> to v0. Instead I am inserting a LoadLoad membar for float/double (which
>> should be rare enough anyway).
>>
>> Other than that it's pretty much analogous to x86.
>>
>> Testing: no regressions in hotspot/tier1
>>
>> Can I please get a review?
>>
>> Thanks, Roman
>>
>