RFR: JDK-8203172: Primitive heap access for interpreter BarrierSetAssembler/aarch64
erik.osterlund at oracle.com
Mon Jun 4 12:20:53 UTC 2018
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
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 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.
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