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
>>>