RFR: 8228369: Shenandoah: Refactor LRB C1 stubs

[Roman Kennke]

Hi Aleksey,

Finally found time to pick this up again.

> On 7/19/19 4:27 PM, Roman Kennke wrote:
>> Please review this updated patch:
>>
>> http://cr.openjdk.java.net/~rkennke/JDK-8228369/webrev.01/
> 
> Brief look:
> 
> *) The comment here is inconsistent with the implementation: mentions "not_resolved", while argument
> is "resolved": shenandoahBarrierSetAssembler_aarch64.cpp
> 
>  485 // Generate check if object is resolved. Branch to not_resolved label, if not. Otherwise return
> resolved
>  486 // object in obj register.
>  487 // obj: object, resolved object on normal return
>  488 // tmp1, tmp2: temp registers, trashed
>  489 void ShenandoahBarrierSetAssembler::gen_resolved_check(MacroAssembler* masm, Register obj,
> Register tmp1, Register tmp2, Label& resolved) {
>  490   __ mov(tmp2, obj);
>  491   resolve_forward_pointer_not_null(masm, obj, tmp1);
>  492   __ cmp(tmp2, obj);
>  493   __ br(Assembler::EQ, resolved);
>  494 }

All that stuff goes away with this:

> *) Is there any actual benefit for separating gen_cset_check and gen_resolved_check? It seems to
> have only two uses, and copy-paste looks like less evil.

Ok. I usually don't like to keep two identical code paths around, but it
doesn't seem catastrophic here, plus it allows for some tiny
improvements in branching.

> *) In x86 code, can't you use resolve_forward_pointer too, instead of doing decoding by hand?

The opposite: the x86 is better because it avoids the actual
decoding+comparison+use of extra register. The aarch64 is a result of
laziness where I simply plugged in the original path into it. The new
version is a little more efficient.

Incremental:
http://cr.openjdk.java.net/~rkennke/JDK-8228369/webrev.02.diff/
Full:
http://cr.openjdk.java.net/~rkennke/JDK-8228369/webrev.02/

Better now?

Roman