RFR 8232992: Shenandoah: Implement self-fixing interpreter LRB

Aleksey Shipilev shade at redhat.com
Fri Oct 25 14:48:17 UTC 2019


On 10/25/19 4:29 PM, Zhengyu Gu wrote:
> Please review this patch that implements self-fixing interpreter LRB.
> 
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232992
> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8232992/webrev.00/

*) I believe we can drop "fixup" from ShenandoahRuntime::load_reference_barrier_fixup(_narrow), as
there no not-fixup versions left.

*) shenandoahBarrierSetAssembler_x86.cpp nit: space before parenthesis

 346   if(setup_addr_first) {

*) shenandoahBarrierSetAssembler_x86.cpp: why pop(thread) is in the middle here?

 315   __ testb(gc_state, ShenandoahHeap::HAS_FORWARDED);
 316 #ifndef _LP64
 317   __ pop(thread);
 318 #endif
 319   __ jccb(Assembler::zero, done);

*) shenandoahBarrierSetAssembler_x86.cpp: seems to me it is cleaner to initialize the boolean
variable first, and then use it. Also, suggestion for name: "need_addr_setup".

    // Use rsi for src address
    const Register src_addr = rsi;
    bool need_addr_setup = (src_addr != dst);

    if (need_addr_setup) {
      ...
    } else {
      ...
    }

    __ call(RuntimeAddress(CAST_FROM_FN_PTR(...);

    if (need_addr_setup) {
      ...

*) shenandoahBarrierSetAssembler_x86.cpp, shenandoahBarrierSetAssembler_aarch64.cpp: since this code
now uses rscratch1, it has to assert that registers do not clash. For example with:

   assert_different_registers(dst, rscratch1, rscratch2);

*) shenandoahBarrierSetC2.cpp: this change looks like a bug fix for matching _narrow. Please RFR it
separately, it should go in sooner.

-- 
Thanks,
-Aleksey




More information about the hotspot-gc-dev mailing list