RFR 8232992: Shenandoah: Implement self-fixing interpreter LRB

[Andrew Haley]

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.  :-)

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