RFR 8232992: Shenandoah: Implement self-fixing interpreter LRB

[Zhengyu Gu]

Hi Andrew,

Thanks for the suggestions. Filed JDK-8233337 to clean this up.

-Zhengyu

On 10/31/19 12:50 PM, Andrew Haley wrote:
> On 10/25/19 3:29 PM, Zhengyu Gu wrote:
>> Test:
>>     hotspot_gc_shenandoah (fastdebug and release)
>>     x86_64 and x86_32 on Linux
>>     aarch64 Linux
>>     Windows x86_64
> 
> I didn't see this because I don't read all the Shenandoah and GC
> messages.
> 
> The AArch64 code is unidiomatic and cumbersome in places, not to
> mention extremely confusing, and I can help with that.
> 
>   236 void ShenandoahBarrierSetAssembler::load_reference_barrier_not_null(MacroAssembler* masm, Register dst, Address load_addr) {
>   237   assert(ShenandoahLoadRefBarrier, "Should be enabled");
>   238   assert(dst != rscratch2, "need rscratch2");
>   239   assert_different_registers(load_addr.base(), load_addr.index(), rscratch1);
>   240   assert_different_registers(load_addr.base(), load_addr.index(), rscratch2);
>   241
>   242   Label done;
>   243   __ enter();
>   244   Address gc_state(rthread, in_bytes(ShenandoahThreadLocalData::gc_state_offset()));
>   245   __ ldrb(rscratch2, gc_state);
>   246
>   247   // Check for heap stability
>   248   __ tbz(rscratch2, ShenandoahHeap::HAS_FORWARDED_BITPOS, done);
>   249
>   250   // use r1 for load address
>   251   Register result_dst = dst;
>   252   if (dst == r1) {
>   253     __ mov(rscratch1, dst);
> 
> This is pointless. On AArch64 mov(Rn, Rm) generates no code if Rn == Rm.
> 
>   254     dst = rscratch1;
>   255   }
>   256
>   257   RegSet to_save_r1 = RegSet::of(r1);
>   258   // If outgoing register is r1, we can clobber it
>   259   if (result_dst != r1) {
>   260     __ push(to_save_r1, sp);
>   261   }
> 
> On AArch64 registers are always saved in pairs, so it makes sense to push
> individual registers. You might as well push both if either is to be saved.
> 
>   262   __ lea(r1, load_addr);
>   263
>   264   RegSet to_save_r0 = RegSet::of(r0);
>   265   if (dst != r0) {
>   266     __ push(to_save_r0, sp);
>   267     __ mov(r0, dst);
>   268   }
>   269
>   270   __ far_call(RuntimeAddress(CAST_FROM_FN_PTR(address, ShenandoahBarrierSetAssembler::shenandoah_lrb())));
>   271
>   272   if (result_dst != r0) {
>   273     __ mov(result_dst, r0);
>   274   }
>   275
>   276   if (dst != r0) {
>   277     __ pop(to_save_r0, sp);
>   278   }
>   279
>   280   if (result_dst != r1) {
>   281     __ pop(to_save_r1, sp);
>   282   }
>   283
>   284   __ bind(done);
>   285   __ leave();
>   286 }
> 
> So, you want to save r1 and r0, but if either of those is the destination you
> don't want to save it. The code at ShenandoahBarrierSetAssembler::shenandoah_lrb()
> preserves everything but r1 and r0.
> 
> I believe this is what you want:
> 
> void ShenandoahBarrierSetAssembler::load_reference_barrier_not_null(MacroAssembler* masm, Register dst, Address load_addr) {
>    assert(ShenandoahLoadRefBarrier, "Should be enabled");
>    assert(dst != rscratch2, "need rscratch2");
>    assert_different_registers(load_addr.base(), load_addr.index(), rscratch1, rscratch2);
> 
>    Label done;
>    __ enter();
>    Address gc_state(rthread, in_bytes(ShenandoahThreadLocalData::gc_state_offset()));
>    __ ldrb(rscratch2, gc_state);
> 
>    // Check for heap stability
>    __ tbz(rscratch2, ShenandoahHeap::HAS_FORWARDED_BITPOS, done);
> 
>    // use r1 for load address
>    Register result_dst = dst;
>    if (dst == r1) {
>      __ mov(rscratch1, dst);
>      dst = rscratch1;
>    }
> 
>    RegSet to_save = RegSet::of(r0, r1) - result_dst;
>    __ push(to_save, sp);
>    __ lea(r1, load_addr);
>    __ mov(r0, dst);
> 
>    __ far_call(RuntimeAddress(CAST_FROM_FN_PTR(address, ShenandoahBarrierSetAssembler::shenandoah_lrb())));
> 
>    __ mov(result_dst, r0);
>    __ pop(to_save, sp);
> 
>    __ bind(done);
>    __ leave();
> }
> 
> 
> Please forward any patches which contain AArch64 assembly code to the
> aarch64-port-dev at openjdk.java.net list.
> 
> I don't mean any criticism of you personally, but the AArch64 code in
> the Shenandoah GC barriers is gnarly and some of the most difficult to
> read in the whole port, probably because its authors, while
> undoubtedly brilliant, were not experienced AArch64 programmers. Let
> me help.  :-)
>