RFR: JDK-8203172: Primitive heap access for interpreter BarrierSetAssembler/aarch64
rkennke at redhat.com
Mon Jun 4 15:24:38 UTC 2018
Ok, right. Very good catch!
This should do it, right? Sorry, I couldn't easily make an incremental diff:
Unfortunately, I cannot really test it because of:
> 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);
> 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
> 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.
> On 2018-05-14 22:23, Roman Kennke wrote:
>> Similar to x86
>> here comes the primitive heap access changes for aarch64:
>> 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
More information about the hotspot-dev