RFR: JDK-8221766: Load-reference barriers for Shenandoah

[Andrew Haley]

On 4/4/19 11:27 AM, Roman Kennke wrote:
> Additional testing on aarch64 showed two problems there:
> - we must not use rscratch1 in the LRB asm code, there is at least one 
> place where the LRB is called with dst==rscratch1.

And it might be called with dst==rscratch2, right? You have to code
defensively against that. Or at least assert it.

> - we need enter()/leave() around the barrier because this is now called 
> in some places that don't have a proper frame, yet.
> 
> The fix for the rscratch1 problem also simplifies and streamlines the 
> LRB to be shaped like the C2 generated LRB. It checks for HAS_FORWARDED 
> gc-state, and then dives into the stub, which 1. checks obj in cset, 2. 
> resolves the fwd ptr, 3. compares obj == fwd, and only then dives into 
> slow-path.

>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8221766
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8221766/webrev.00/
>>
>> Can I please get reviews for this change?

One thing:

   Register obj = r0;
+  __ mov(rscratch2, r0);
+  resolve_forward_pointer_not_null(cgen->assembler(), r0);
+  __ cmp(rscratch2, r0);
+  __ br(Assembler::NE, done);

The macro assembler convention is that rscratchX is never preserved
across a macro, even if you wrote that macro yourself. I've been
trying to clean up these as I go, because this problem has bitten us
repeatedly. I guess you know that whatever is called by
resolve_forward_pointer_not_null(cgen->assembler(), r0) is okay...

Also, if you're going to say

   Register obj = r0;

please either use "obj" everywhere or "r0", not a mixture.
I'd just get rid of the Register alias.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671