RFR: 8255762: Shenandoah: Consolidate/streamline interpreter LRBs [v2]

[Aleksey Shipilev]

On Tue, 3 Nov 2020 11:11:11 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> We currently have two LRB implementations in interpreter: one normal and one for native/weak LRB. We should consolidate them into one.
>> 
>> The main issue here is that the two implementations follow different approaches:
>> - the normal LRB calls through the shenandoah_lrb stub which does additional null- and cset-checking
>> - the weak LRB calls to runtime directly and must not do cset-checking
>> 
>> The reason for calling through the stub is that it gives more freedom to allocate two registers that are required for the cset check. However, we can invert the cset addressing like we did in JDK-8245465 and save a register. We can also eliminate the null-check and let the cset-check subsume it (like we do everywhere else). Allocating a single register for the cset-check is easy, and we can do so in-line without the extra jump through the stub. The runtime call through the stub has also been very costly: it dumps 2KB of register data on the stack at each call, that is very excessive. save_xmm_registers() should be more than enough (in-fact, I am almost certain that this is excessive too, and we should only need to save/restore xmm0 - but not in this patch). Not needing to generate the call-stub is also helpful for backportability, because in jdk8-shenandoah we cannot do that.
>> 
>> Testing: hotspot_gc_shenandoah (x86_64, x86_32, aarch64)
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   A few touch-ups

Looks good, apart from two nits.

src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp line 301:

> 299:       Register r = as_Register(i);
> 300:       if (r != rsp && r != rbp && r != dst && r != src.base() && r != src.index()) {
> 301:         tmp1 = r;

Minor nit: looks like you just want to `break` from here, and not check `tmp1->is_valid()` in the loop predicate?

src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp line 335:

> 333:   assert(slot == 0, "must use all slots");
> 334: 
> 335:   Register tmp2 = dst == rsi ? rdx : rsi;

Please do parentheses around `(dst == rsi)`.

-------------

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1010