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

[Erik Österlund]

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
>