Refactor barrier implementations (#332)

We recently added another write barrier, the `SATBBarrier`. Being
another barrier based on the per-object unlog bit, it shares much code
with the `ObjectBarrier`. We refactor the write barriers to improve code
sharing and clean up unused code.

We introduce abstract base classes `MMTkUnlogBitBarrierSetRuntime`,
`MMTkUnlogBitBarrierSetAssembler`, `MMTkUnlogBitBarrierSetC1` and
`MMTkUnlogBitBarrierSetC2`, which both `ObjectBarrier` and `SATBBarrier`
derive from. Those base classes include common parts related to unlog
bit handling, including the fast paths for checking if the unlog bit is
set, and also the generic implementation for both the pre and the post
barriers if applicable.

Runtime:

- Extracted a method for testing if the unlog bit is set for a given
object.

Assembler:

- Extracted a method for emitting the unlog bit checking fast path.
- Extracted a method for generating both the pre and the post write
barriers.
- Simplified the fast path to use fewer instructions and fewer scratch
registers.
- Worked around an issue where the `tmp1` and `tmp2` temporary registers
passed in from `BarrierSetAssembler::store_at` may overlap with
`dst.base()` and `dst_index()`, respectively.
  - See https://bugs.openjdk.org/browse/JDK-8301371

C1:

- Changed "runtime stubs" to be one per runtime function, not one per
pre/post/refload barrier. Those "runtime stubs" are now assembled by
`MMTkBarrierSetAssembler` instead of by `MMTkObjectBarrierSetAssembler`
or `MMTkSATBBarrierSetAssembler`. The intention is that all barriers can
call those functions.
- Extracted a method for emitting the unlog bit checking fast path.
- Extracted a method for emitting both the pre and the post write
barriers and their slow-path code stub.
- We no longer pass the `slot` and `new_val` to the runtime function in
the slow path. Object-logging barriers only needs the `obj` argument for
scanning its fields.
- For this reason, we moved the C1 CodeStub for the pre/post barrier
slow paths down from `mmtkBarrierSetC1.hpp` to `mmtkUnlogBitBarrier.hpp`
(and unified the pre/post stub implementation) because it no longer
needs the `slot` or `new_val` args, and is now specific to unlog bit
barriers. If we introduce field-logging barriers, we will implement a
different stub.
- The C1 SATBBarrier no longer needs code patching. The code patching
was intended for barriers that load from the field in the fast path
using LIR instructions, such as G1's SATB barrier. Our object-logging
barrier scans the fields in runtime functions
(`mmtk_object_reference_write_pre`), not using LIR instructions.

C2:

- Extracted a method for emitting the unlog bit checking fast path.
- Extracted a method for emitting both the pre and the post write
barriers.
- We no longer pass the `slot` and `new_val` to the runtime function in
the slow path.

Misc:

- Removed the `#define private public` hack as we no longer need it.
- Removed unused macros, dead code and irrelevant comments.
- Changed the macro `SOFT_REFERENCE_LOAD_BARRIER` to a run-time flag
`mmtk_enable_reference_load_barrier` settable by environment variable.

Author: wks

mmtk/Cargo.lock

@@ -36,7 +36,7 @@ probe = "0.5"
 # - change branch
 # - change repo name
 # But other changes including adding/removing whitespaces in commented lines may break the CI.
-mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "a4dd70cb70a116a32b1bbb20501c48f77f49181b" }
+mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "e25ad8bb969f5d650de3f1946582075923bafcc9" }
 # Uncomment the following to build locally
 # mmtk = { path = "../repos/mmtk-core" }
 

mmtk/Cargo.toml

@@ -2,15 +2,13 @@
 #include "mmtkObjectBarrier.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 
+//////////////////// Runtime ////////////////////
+
 void MMTkObjectBarrierSetRuntime::object_probable_write(oop new_obj) const {
   if (mmtk_enable_barrier_fastpath) {
     // Do fast-path check before entering mmtk rust code, to improve mutator performance.
     // This is identical to calling `mmtk_object_probable_write` directly without a fast-path.
-    intptr_t addr = (intptr_t) (void*) new_obj;
-    uint8_t* meta_addr = (uint8_t*) (SIDE_METADATA_BASE_ADDRESS + (addr >> 6));
-    intptr_t shift = (addr >> 3) & 0b111;
-    uint8_t byte_val = *meta_addr;
-    if (((byte_val >> shift) & 1) == 1) {
+    if (is_unlog_bit_set(new_obj)) {
       // Only promoted objects will reach here.
       // The duplicated unlog bit check inside slow-path still remains correct.
       mmtk_object_probable_write((MMTk_Mutator) &Thread::current()->third_party_heap_mutator, (void*) new_obj);
@@ -23,70 +21,21 @@ void MMTkObjectBarrierSetRuntime::object_probable_write(oop new_obj) const {
 
 void MMTkObjectBarrierSetRuntime::object_reference_write_post(oop src, oop* slot, oop target) const {
   if (mmtk_enable_barrier_fastpath) {
-    intptr_t addr = (intptr_t) (void*) src;
-    uint8_t* meta_addr = (uint8_t*) (SIDE_METADATA_BASE_ADDRESS + (addr >> 6));
-    intptr_t shift = (addr >> 3) & 0b111;
-    uint8_t byte_val = *meta_addr;
-    if (((byte_val >> shift) & 1) == 1) {
-      // MMTkObjectBarrierSetRuntime::object_reference_write_pre_slow()((void*) src);
+    if (is_unlog_bit_set(src)) {
       object_reference_write_slow_call((void*) src, (void*) slot, (void*) target);
     }
   } else {
     object_reference_write_post_call((void*) src, (void*) slot, (void*) target);
   }
 }
 
+//////////////////// Assembler ////////////////////
+
 #define __ masm->
 
 void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2, bool compensate_val_reg) const {
   if (can_remove_barrier(decorators, val, /* skip_const_null */ true)) return;
-
-  bool is_not_null = (decorators & IS_NOT_NULL) != 0;
-
-  Label done;
-  Register obj = dst.base();
-  if (mmtk_enable_barrier_fastpath) {
-    Register tmp3 = rscratch1;
-    Register tmp4 = rscratch2;
-    assert_different_registers(obj, tmp2, tmp3);
-    assert_different_registers(tmp4, rcx);
-
-    // tmp2 = load-byte (SIDE_METADATA_BASE_ADDRESS + (obj >> 6));
-    __ movptr(tmp3, obj);
-    __ shrptr(tmp3, 6);
-    __ movptr(tmp2, SIDE_METADATA_BASE_ADDRESS);
-    __ movb(tmp2, Address(tmp2, tmp3));
-    // tmp3 = (obj >> 3) & 7
-    __ movptr(tmp3, obj);
-    __ shrptr(tmp3, 3);
-    __ andptr(tmp3, 7);
-    // tmp2 = tmp2 >> tmp3
-    __ movptr(tmp4, rcx);
-    __ movl(rcx, tmp3);
-    __ shrptr(tmp2);
-    __ movptr(rcx, tmp4);
-    // if ((tmp2 & 1) == 1) goto slowpath;
-    __ andptr(tmp2, 1);
-    __ cmpptr(tmp2, 1);
-    __ jcc(Assembler::notEqual, done);
-  }
-
-  __ movptr(c_rarg0, obj);
-  __ xorptr(c_rarg1, c_rarg1);
-  // Note: If `compensate_val_reg == true && UseCompressedOops === true`, the `val` register will be
-  // holding a compressed pointer to the target object. If the write barrier needs to know the
-  // target, we will need to decompress it before passing it to the barrier slow path. However,
-  // since we know the semantics of `mmtk::plan::barriers::ObjectBarrier`, i.e. it logs the object
-  // without looking at the `slot` or the `target` parameter at all, we simply pass nullptr to both
-  // parameters.
-  __ xorptr(c_rarg2, c_rarg2);
-
-  if (mmtk_enable_barrier_fastpath) {
-    __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_slow_call), 3);
-    __ bind(done);
-  } else {
-    __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_post_call), 3);
-  }
+  object_reference_write_pre_or_post(masm, decorators, dst, val, /* pre = */ false);
 }
 
 void MMTkObjectBarrierSetAssembler::arraycopy_prologue(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Register src, Register dst, Register count) {
@@ -135,56 +84,7 @@ void MMTkObjectBarrierSetAssembler::arraycopy_epilogue(MacroAssembler* masm, Dec
 
 #undef __
 
-#define __ sasm->
-
-void MMTkObjectBarrierSetAssembler::generate_c1_post_write_barrier_runtime_stub(StubAssembler* sasm) const {
-  __ prologue("mmtk_object_barrier", false);
-
-  Label done, runtime;
-
-  __ push(c_rarg0);
-  __ push(c_rarg1);
-  __ push(c_rarg2);
-  __ push(rax);
-
-  __ load_parameter(0, c_rarg0);
-  __ load_parameter(1, c_rarg1);
-  __ load_parameter(2, c_rarg2);
-
-  __ bind(runtime);
-
-  __ save_live_registers_no_oop_map(true);
-
-  if (mmtk_enable_barrier_fastpath) {
-    __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_slow_call), 3);
-  } else {
-    __ call_VM_leaf_base(FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_post_call), 3);
-  }
-
-  __ restore_live_registers(true);
-
-  __ bind(done);
-  __ pop(rax);
-  __ pop(c_rarg2);
-  __ pop(c_rarg1);
-  __ pop(c_rarg0);
-
-  __ epilogue();
-}
-
-#undef __
-#define __ ce->masm()->
-
-void MMTkObjectBarrierSetAssembler::generate_c1_post_write_barrier_stub(LIR_Assembler* ce, MMTkC1PostBarrierStub* stub) const {
-  MMTkBarrierSetC1* bs = (MMTkBarrierSetC1*) BarrierSet::barrier_set()->barrier_set_c1();
-  __ bind(*stub->entry());
-  ce->store_parameter(stub->src->as_pointer_register(), 0);
-  ce->store_parameter(stub->slot->as_pointer_register(), 1);
-  ce->store_parameter(stub->new_val->as_pointer_register(), 2);
-  __ call(RuntimeAddress(bs->post_barrier_c1_runtime_code_blob()->code_begin()));
-  __ jmp(*stub->continuation());
-}
-#undef __
+//////////////////// C1 ////////////////////
 
 #ifdef ASSERT
 #define __ gen->lir(__FILE__, __LINE__)->
@@ -193,100 +93,21 @@ void MMTkObjectBarrierSetAssembler::generate_c1_post_write_barrier_stub(LIR_Asse
 #endif
 
 void MMTkObjectBarrierSetC1::object_reference_write_post(LIRAccess& access, LIR_Opr src, LIR_Opr slot, LIR_Opr new_val) const {
-  LIRGenerator* gen = access.gen();
-  DecoratorSet decorators = access.decorators();
-  if ((decorators & IN_HEAP) == 0) return;
-  if (!src->is_register()) {
-    LIR_Opr reg = gen->new_pointer_register();
-    if (src->is_constant()) {
-      __ move(src, reg);
-    } else {
-      __ leal(src, reg);
-    }
-    src = reg;
-  }
-  assert(src->is_register(), "must be a register at this point");
-  if (!slot->is_register()) {
-    LIR_Opr reg = gen->new_pointer_register();
-    if (slot->is_constant()) {
-      __ move(slot, reg);
-    } else {
-      __ leal(slot, reg);
-    }
-    slot = reg;
-  }
-  assert(slot->is_register(), "must be a register at this point");
-  if (!new_val->is_register()) {
-    LIR_Opr new_val_reg = gen->new_register(T_OBJECT);
-    if (new_val->is_constant()) {
-      __ move(new_val, new_val_reg);
-    } else {
-      __ leal(new_val, new_val_reg);
-    }
-    new_val = new_val_reg;
-  }
-  assert(new_val->is_register(), "must be a register at this point");
-  CodeStub* slow = new MMTkC1PostBarrierStub(src, slot, new_val);
-
-  if (mmtk_enable_barrier_fastpath) {
-    LIR_Opr addr = src;
-    // uint8_t* meta_addr = (uint8_t*) (SIDE_METADATA_BASE_ADDRESS + (addr >> 6));
-    LIR_Opr offset = gen->new_pointer_register();
-    __ move(addr, offset);
-    __ unsigned_shift_right(offset, 6, offset);
-    LIR_Opr base = gen->new_pointer_register();
-    __ move(LIR_OprFact::longConst(SIDE_METADATA_BASE_ADDRESS), base);
-    LIR_Address* meta_addr = new LIR_Address(base, offset, T_BYTE);
-    // uint8_t byte_val = *meta_addr;
-    LIR_Opr byte_val = gen->new_register(T_INT);
-    __ move(meta_addr, byte_val);
-    // intptr_t shift = (addr >> 3) & 0b111;
-    LIR_Opr shift = gen->new_register(T_INT);
-    __ move(addr, shift);
-    __ unsigned_shift_right(shift, 3, shift);
-    __ logical_and(shift, LIR_OprFact::intConst(0b111), shift);
-    // if (((byte_val >> shift) & 1) == 1) slow;
-    LIR_Opr result = byte_val;
-    __ unsigned_shift_right(result, shift, result, LIR_OprFact::illegalOpr);
-    __ logical_and(result, LIR_OprFact::intConst(1), result);
-    __ cmp(lir_cond_equal, result, LIR_OprFact::intConst(1));
-    __ branch(lir_cond_equal, T_BYTE, slow);
-  } else {
-    __ jump(slow);
-  }
-
-  __ branch_destination(slow->continuation());
+  object_reference_write_pre_or_post(access, src, /* pre = */ false);
 }
 
 #undef __
 
+//////////////////// C2 ////////////////////
+
 #define __ ideal.
 
 void MMTkObjectBarrierSetC2::object_reference_write_post(GraphKit* kit, Node* src, Node* slot, Node* val) const {
   if (can_remove_barrier(kit, &kit->gvn(), src, slot, val, /* skip_const_null */ true)) return;
 
   MMTkIdealKit ideal(kit, true);
 
-  if (mmtk_enable_barrier_fastpath) {
-    Node* no_base = __ top();
-    float unlikely  = PROB_UNLIKELY(0.999);
-
-    Node* zero  = __ ConI(0);
-    Node* addr = __ CastPX(__ ctrl(), src);
-    Node* meta_addr = __ AddP(no_base, __ ConP(SIDE_METADATA_BASE_ADDRESS), __ URShiftX(addr, __ ConI(6)));
-    Node* byte = __ load(__ ctrl(), meta_addr, TypeInt::INT, T_BYTE, Compile::AliasIdxRaw);
-    Node* shift = __ URShiftX(addr, __ ConI(3));
-    shift = __ AndI(__ ConvL2I(shift), __ ConI(7));
-    Node* result = __ AndI(__ URShiftI(byte, shift), __ ConI(1));
-
-    __ if_then(result, BoolTest::ne, zero, unlikely); {
-      const TypeFunc* tf = __ func_type(TypeOopPtr::BOTTOM, TypeOopPtr::BOTTOM, TypeOopPtr::BOTTOM);
-      Node* x = __ make_leaf_call(tf, FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_slow_call), "mmtk_barrier_call", src, slot, val);
-    } __ end_if();
-  } else {
-    const TypeFunc* tf = __ func_type(TypeOopPtr::BOTTOM, TypeOopPtr::BOTTOM, TypeOopPtr::BOTTOM);
-    Node* x = __ make_leaf_call(tf, FN_ADDR(MMTkBarrierSetRuntime::object_reference_write_post_call), "mmtk_barrier_call", src, slot, val);
-  }
+  object_reference_write_pre_or_post(ideal, src, /* pre = */ false);
 
   kit->final_sync(ideal); // Final sync IdealKit and GraphKit.
 }

openjdk/barriers/mmtkObjectBarrier.cpp

@@ -6,19 +6,22 @@
 #include "../mmtkBarrierSetAssembler_x86.hpp"
 #include "../mmtkBarrierSetC1.hpp"
 #include "../mmtkBarrierSetC2.hpp"
+#include "mmtkUnlogBitBarrier.hpp"
 #include "c1/c1_LIRAssembler.hpp"
 #include "c1/c1_MacroAssembler.hpp"
 #include "gc/shared/barrierSet.hpp"
 #include "opto/callnode.hpp"
 #include "opto/idealKit.hpp"
 
-#define SIDE_METADATA_WORST_CASE_RATIO_LOG 1
-#define LOG_BYTES_IN_CHUNK 22
-#define CHUNK_MASK ((1L << LOG_BYTES_IN_CHUNK) - 1)
+/// This file supports the `ObjectBarrier` in MMTk core,
+/// i.e. the barrier that remembers the object when it is first modified.
+/// Despite the name, `ObjectBarrier` is not the only barrier that uses the object-grained unlogging bit.
+/// `SATBBarrier` also uses the object-grained unlog bit.
+/// We keep the name in sync with the MMTk core.
 
-const intptr_t SIDE_METADATA_BASE_ADDRESS = (intptr_t) GLOBAL_SIDE_METADATA_VM_BASE_ADDRESS;
+//////////////////// Runtime ////////////////////
 
-class MMTkObjectBarrierSetRuntime: public MMTkBarrierSetRuntime {
+class MMTkObjectBarrierSetRuntime: public MMTkUnlogBitBarrierSetRuntime {
 public:
   // Interfaces called by `MMTkBarrierSet::AccessBarrier`
   virtual void object_reference_write_post(oop src, oop* slot, oop target) const override;
@@ -28,18 +31,19 @@ class MMTkObjectBarrierSetRuntime: public MMTkBarrierSetRuntime {
   virtual void object_probable_write(oop new_obj) const override;
 };
 
-class MMTkObjectBarrierSetAssembler: public MMTkBarrierSetAssembler {
+//////////////////// Assembler ////////////////////
+
+class MMTkObjectBarrierSetAssembler: public MMTkUnlogBitBarrierSetAssembler {
 protected:
   virtual void object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2, bool compensate_val_reg) const override;
-  /// Generate C1 write barrier slow-call assembly code
-  virtual void generate_c1_post_write_barrier_runtime_stub(StubAssembler* sasm) const override;
 public:
-  virtual void generate_c1_post_write_barrier_stub(LIR_Assembler* ce, MMTkC1PostBarrierStub* stub) const override;
   virtual void arraycopy_prologue(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Register src, Register dst, Register count) override;
   virtual void arraycopy_epilogue(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Register src, Register dst, Register count) override;
 };
 
-class MMTkObjectBarrierSetC1: public MMTkBarrierSetC1 {
+//////////////////// C1 ////////////////////
+
+class MMTkObjectBarrierSetC1: public MMTkUnlogBitBarrierSetC1 {
 protected:
   virtual void object_reference_write_post(LIRAccess& access, LIR_Opr src, LIR_Opr slot, LIR_Opr new_val) const override;
 
@@ -48,11 +52,15 @@ class MMTkObjectBarrierSetC1: public MMTkBarrierSetC1 {
   }
 };
 
-class MMTkObjectBarrierSetC2: public MMTkBarrierSetC2 {
+//////////////////// C2 ////////////////////
+
+class MMTkObjectBarrierSetC2: public MMTkUnlogBitBarrierSetC2 {
 protected:
   virtual void object_reference_write_post(GraphKit* kit, Node* src, Node* slot, Node* val) const override;
 };
 
+//////////////////// Impl ////////////////////
+
 struct MMTkObjectBarrier: MMTkBarrierImpl<
   MMTkObjectBarrierSetRuntime,
   MMTkObjectBarrierSetAssembler,