diff options
31 files changed, 405 insertions, 910 deletions
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 415c6bf71e..462225aafb 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -599,298 +599,6 @@ void JumpTableARM64::EmitTable(CodeGeneratorARM64* codegen) { } } -// Abstract base class for read barrier slow paths marking a reference -// `ref`. -// -// Argument `entrypoint` must be a register location holding the read -// barrier marking runtime entry point to be invoked or an empty -// location; in the latter case, the read barrier marking runtime -// entry point will be loaded by the slow path code itself. -class ReadBarrierMarkSlowPathBaseARM64 : public SlowPathCodeARM64 { - protected: - ReadBarrierMarkSlowPathBaseARM64(HInstruction* instruction, Location ref, Location entrypoint) - : SlowPathCodeARM64(instruction), ref_(ref), entrypoint_(entrypoint) { - DCHECK(kEmitCompilerReadBarrier); - } - - const char* GetDescription() const OVERRIDE { return "ReadBarrierMarkSlowPathBaseARM64"; } - - // Generate assembly code calling the read barrier marking runtime - // entry point (ReadBarrierMarkRegX). - void GenerateReadBarrierMarkRuntimeCall(CodeGenerator* codegen) { - // No need to save live registers; it's taken care of by the - // entrypoint. Also, there is no need to update the stack mask, - // as this runtime call will not trigger a garbage collection. - CodeGeneratorARM64* arm64_codegen = down_cast<CodeGeneratorARM64*>(codegen); - DCHECK_NE(ref_.reg(), LR); - DCHECK_NE(ref_.reg(), WSP); - DCHECK_NE(ref_.reg(), WZR); - // IP0 is used internally by the ReadBarrierMarkRegX entry point - // as a temporary, it cannot be the entry point's input/output. - DCHECK_NE(ref_.reg(), IP0); - DCHECK(0 <= ref_.reg() && ref_.reg() < kNumberOfWRegisters) << ref_.reg(); - // "Compact" slow path, saving two moves. - // - // Instead of using the standard runtime calling convention (input - // and output in W0): - // - // W0 <- ref - // W0 <- ReadBarrierMark(W0) - // ref <- W0 - // - // we just use rX (the register containing `ref`) as input and output - // of a dedicated entrypoint: - // - // rX <- ReadBarrierMarkRegX(rX) - // - if (entrypoint_.IsValid()) { - arm64_codegen->ValidateInvokeRuntimeWithoutRecordingPcInfo(instruction_, this); - __ Blr(XRegisterFrom(entrypoint_)); - } else { - // Entrypoint is not already loaded, load from the thread. - int32_t entry_point_offset = - Thread::ReadBarrierMarkEntryPointsOffset<kArm64PointerSize>(ref_.reg()); - // This runtime call does not require a stack map. - arm64_codegen->InvokeRuntimeWithoutRecordingPcInfo(entry_point_offset, instruction_, this); - } - } - - // The location (register) of the marked object reference. - const Location ref_; - - // The location of the entrypoint if it is already loaded. - const Location entrypoint_; - - private: - DISALLOW_COPY_AND_ASSIGN(ReadBarrierMarkSlowPathBaseARM64); -}; - -// Slow path loading `obj`'s lock word, loading a reference from -// object `*(obj + offset + (index << scale_factor))` into `ref`, and -// marking `ref` if `obj` is gray according to the lock word (Baker -// read barrier). If needed, this slow path also atomically updates -// the field `obj.field` in the object `obj` holding this reference -// after marking. -// -// This means that after the execution of this slow path, both `ref` -// and `obj.field` will be up-to-date; i.e., after the flip, both will -// hold the same to-space reference (unless another thread installed -// another object reference (different from `ref`) in `obj.field`). -// -// Argument `entrypoint` must be a register location holding the read -// barrier marking runtime entry point to be invoked or an empty -// location; in the latter case, the read barrier marking runtime -// entry point will be loaded by the slow path code itself. -class LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARM64 - : public ReadBarrierMarkSlowPathBaseARM64 { - public: - LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARM64( - HInstruction* instruction, - Location ref, - Register obj, - uint32_t offset, - Location index, - size_t scale_factor, - bool needs_null_check, - bool use_load_acquire, - Register temp, - Location entrypoint = Location::NoLocation()) - : ReadBarrierMarkSlowPathBaseARM64(instruction, ref, entrypoint), - obj_(obj), - offset_(offset), - index_(index), - scale_factor_(scale_factor), - needs_null_check_(needs_null_check), - use_load_acquire_(use_load_acquire), - temp_(temp) { - DCHECK(kEmitCompilerReadBarrier); - DCHECK(kUseBakerReadBarrier); - } - - const char* GetDescription() const OVERRIDE { - return "LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARM64"; - } - - void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { - LocationSummary* locations = instruction_->GetLocations(); - Register ref_reg = WRegisterFrom(ref_); - DCHECK(locations->CanCall()); - DCHECK(ref_.IsRegister()) << ref_; - DCHECK(!locations->GetLiveRegisters()->ContainsCoreRegister(ref_.reg())) << ref_.reg(); - DCHECK(obj_.IsW()); - DCHECK_NE(ref_.reg(), LocationFrom(temp_).reg()); - - // This slow path is only used by the UnsafeCASObject intrinsic at the moment. - DCHECK((instruction_->IsInvokeVirtual() && instruction_->GetLocations()->Intrinsified())) - << "Unexpected instruction in read barrier marking and field updating slow path: " - << instruction_->DebugName(); - DCHECK(instruction_->GetLocations()->Intrinsified()); - DCHECK_EQ(instruction_->AsInvoke()->GetIntrinsic(), Intrinsics::kUnsafeCASObject); - DCHECK_EQ(offset_, 0u); - DCHECK_EQ(scale_factor_, 0u); - DCHECK_EQ(use_load_acquire_, false); - // The location of the offset of the marked reference field within `obj_`. - Location field_offset = index_; - DCHECK(field_offset.IsRegister()) << field_offset; - - // Temporary register `temp_`, used to store the lock word, must - // not be IP0 nor IP1, as we may use them to emit the reference - // load (in the call to GenerateRawReferenceLoad below), and we - // need the lock word to still be in `temp_` after the reference - // load. - DCHECK_NE(LocationFrom(temp_).reg(), IP0); - DCHECK_NE(LocationFrom(temp_).reg(), IP1); - - __ Bind(GetEntryLabel()); - - // The implementation is: - // - // uint32_t rb_state = Lockword(obj->monitor_).ReadBarrierState(); - // lfence; // Load fence or artificial data dependency to prevent load-load reordering - // HeapReference<mirror::Object> ref = *src; // Original reference load. - // bool is_gray = (rb_state == ReadBarrier::GrayState()); - // if (is_gray) { - // old_ref = ref; - // ref = entrypoint(ref); // ref = ReadBarrier::Mark(ref); // Runtime entry point call. - // compareAndSwapObject(obj, field_offset, old_ref, ref); - // } - - // /* int32_t */ monitor = obj->monitor_ - uint32_t monitor_offset = mirror::Object::MonitorOffset().Int32Value(); - __ Ldr(temp_, HeapOperand(obj_, monitor_offset)); - if (needs_null_check_) { - codegen->MaybeRecordImplicitNullCheck(instruction_); - } - // /* LockWord */ lock_word = LockWord(monitor) - static_assert(sizeof(LockWord) == sizeof(int32_t), - "art::LockWord and int32_t have different sizes."); - - // Introduce a dependency on the lock_word including rb_state, - // to prevent load-load reordering, and without using - // a memory barrier (which would be more expensive). - // `obj` is unchanged by this operation, but its value now depends - // on `temp`. - __ Add(obj_.X(), obj_.X(), Operand(temp_.X(), LSR, 32)); - - // The actual reference load. - // A possible implicit null check has already been handled above. - CodeGeneratorARM64* arm64_codegen = down_cast<CodeGeneratorARM64*>(codegen); - arm64_codegen->GenerateRawReferenceLoad(instruction_, - ref_, - obj_, - offset_, - index_, - scale_factor_, - /* needs_null_check */ false, - use_load_acquire_); - - // Mark the object `ref` when `obj` is gray. - // - // if (rb_state == ReadBarrier::GrayState()) - // ref = ReadBarrier::Mark(ref); - // - // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); - static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); - __ Tbz(temp_, LockWord::kReadBarrierStateShift, GetExitLabel()); - - // Save the old value of the reference before marking it. - // Note that we cannot use IP to save the old reference, as IP is - // used internally by the ReadBarrierMarkRegX entry point, and we - // need the old reference after the call to that entry point. - DCHECK_NE(LocationFrom(temp_).reg(), IP0); - __ Mov(temp_.W(), ref_reg); - - GenerateReadBarrierMarkRuntimeCall(codegen); - - // If the new reference is different from the old reference, - // update the field in the holder (`*(obj_ + field_offset)`). - // - // Note that this field could also hold a different object, if - // another thread had concurrently changed it. In that case, the - // LDXR/CMP/BNE sequence of instructions in the compare-and-set - // (CAS) operation below would abort the CAS, leaving the field - // as-is. - __ Cmp(temp_.W(), ref_reg); - __ B(eq, GetExitLabel()); - - // Update the the holder's field atomically. This may fail if - // mutator updates before us, but it's OK. This is achieved - // using a strong compare-and-set (CAS) operation with relaxed - // memory synchronization ordering, where the expected value is - // the old reference and the desired value is the new reference. - - MacroAssembler* masm = arm64_codegen->GetVIXLAssembler(); - UseScratchRegisterScope temps(masm); - - // Convenience aliases. - Register base = obj_.W(); - Register offset = XRegisterFrom(field_offset); - Register expected = temp_.W(); - Register value = ref_reg; - Register tmp_ptr = temps.AcquireX(); // Pointer to actual memory. - Register tmp_value = temps.AcquireW(); // Value in memory. - - __ Add(tmp_ptr, base.X(), Operand(offset)); - - if (kPoisonHeapReferences) { - arm64_codegen->GetAssembler()->PoisonHeapReference(expected); - if (value.Is(expected)) { - // Do not poison `value`, as it is the same register as - // `expected`, which has just been poisoned. - } else { - arm64_codegen->GetAssembler()->PoisonHeapReference(value); - } - } - - // do { - // tmp_value = [tmp_ptr] - expected; - // } while (tmp_value == 0 && failure([tmp_ptr] <- r_new_value)); - - vixl::aarch64::Label loop_head, comparison_failed, exit_loop; - __ Bind(&loop_head); - __ Ldxr(tmp_value, MemOperand(tmp_ptr)); - __ Cmp(tmp_value, expected); - __ B(&comparison_failed, ne); - __ Stxr(tmp_value, value, MemOperand(tmp_ptr)); - __ Cbnz(tmp_value, &loop_head); - __ B(&exit_loop); - __ Bind(&comparison_failed); - __ Clrex(); - __ Bind(&exit_loop); - - if (kPoisonHeapReferences) { - arm64_codegen->GetAssembler()->UnpoisonHeapReference(expected); - if (value.Is(expected)) { - // Do not unpoison `value`, as it is the same register as - // `expected`, which has just been unpoisoned. - } else { - arm64_codegen->GetAssembler()->UnpoisonHeapReference(value); - } - } - - __ B(GetExitLabel()); - } - - private: - // The register containing the object holding the marked object reference field. - const Register obj_; - // The offset, index and scale factor to access the reference in `obj_`. - uint32_t offset_; - Location index_; - size_t scale_factor_; - // Is a null check required? - bool needs_null_check_; - // Should this reference load use Load-Acquire semantics? - bool use_load_acquire_; - // A temporary register used to hold the lock word of `obj_`; and - // also to hold the original reference value, when the reference is - // marked. - const Register temp_; - - DISALLOW_COPY_AND_ASSIGN(LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARM64); -}; - // Slow path generating a read barrier for a heap reference. class ReadBarrierForHeapReferenceSlowPathARM64 : public SlowPathCodeARM64 { public: @@ -1276,9 +984,12 @@ void CodeGeneratorARM64::Finalize(CodeAllocator* allocator) { case BakerReadBarrierKind::kGcRoot: { DCHECK_GE(literal_offset, 4u); uint32_t prev_insn = GetInsn(literal_offset - 4u); - // LDR (immediate) with correct root_reg. const uint32_t root_reg = BakerReadBarrierFirstRegField::Decode(encoded_data); - CHECK_EQ(prev_insn & 0xffc0001fu, 0xb9400000u | root_reg); + // Usually LDR (immediate) with correct root_reg but + // we may have a "MOV marked, old_value" for UnsafeCASObject. + if ((prev_insn & 0xffe0ffff) != (0x2a0003e0 | root_reg)) { // MOV? + CHECK_EQ(prev_insn & 0xffc0001fu, 0xb9400000u | root_reg); // LDR? + } break; } default: @@ -1463,8 +1174,24 @@ void CodeGeneratorARM64::MarkGCCard(Register object, Register value, bool value_ if (value_can_be_null) { __ Cbz(value, &done); } + // Load the address of the card table into `card`. __ Ldr(card, MemOperand(tr, Thread::CardTableOffset<kArm64PointerSize>().Int32Value())); + // Calculate the offset (in the card table) of the card corresponding to + // `object`. __ Lsr(temp, object, gc::accounting::CardTable::kCardShift); + // Write the `art::gc::accounting::CardTable::kCardDirty` value into the + // `object`'s card. + // + // Register `card` contains the address of the card table. Note that the card + // table's base is biased during its creation so that it always starts at an + // address whose least-significant byte is equal to `kCardDirty` (see + // art::gc::accounting::CardTable::Create). Therefore the STRB instruction + // below writes the `kCardDirty` (byte) value into the `object`'s card + // (located at `card + object >> kCardShift`). + // + // This dual use of the value in register `card` (1. to calculate the location + // of the card to mark; and 2. to load the `kCardDirty` value) saves a load + // (no need to explicitly load `kCardDirty` as an immediate value). __ Strb(card, MemOperand(card, temp.X())); if (value_can_be_null) { __ Bind(&done); @@ -4539,7 +4266,7 @@ vixl::aarch64::Label* CodeGeneratorARM64::NewStringBssEntryPatch( } void CodeGeneratorARM64::EmitBakerReadBarrierCbnz(uint32_t custom_data) { - ExactAssemblyScope guard(GetVIXLAssembler(), 1 * vixl::aarch64::kInstructionSize); + DCHECK(!__ AllowMacroInstructions()); // In ExactAssemblyScope. if (Runtime::Current()->UseJitCompilation()) { auto it = jit_baker_read_barrier_slow_paths_.FindOrAdd(custom_data); vixl::aarch64::Label* slow_path_entry = &it->second.label; @@ -6085,7 +5812,7 @@ void CodeGeneratorARM64::GenerateGcRootFieldLoad( __ bind(fixup_label); } static_assert(BAKER_MARK_INTROSPECTION_GC_ROOT_LDR_OFFSET == -8, - "GC root LDR must be 2 instruction (8B) before the return address label."); + "GC root LDR must be 2 instructions (8B) before the return address label."); __ ldr(root_reg, MemOperand(obj.X(), offset)); EmitBakerReadBarrierCbnz(custom_data); __ bind(&return_address); @@ -6115,6 +5842,25 @@ void CodeGeneratorARM64::GenerateGcRootFieldLoad( MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } +void CodeGeneratorARM64::GenerateUnsafeCasOldValueMovWithBakerReadBarrier( + vixl::aarch64::Register marked, + vixl::aarch64::Register old_value) { + DCHECK(kEmitCompilerReadBarrier); + DCHECK(kUseBakerReadBarrier); + + // Similar to the Baker RB path in GenerateGcRootFieldLoad(), with a MOV instead of LDR. + uint32_t custom_data = EncodeBakerReadBarrierGcRootData(marked.GetCode()); + + ExactAssemblyScope guard(GetVIXLAssembler(), 3 * vixl::aarch64::kInstructionSize); + vixl::aarch64::Label return_address; + __ adr(lr, &return_address); + static_assert(BAKER_MARK_INTROSPECTION_GC_ROOT_LDR_OFFSET == -8, + "GC root LDR must be 2 instructions (8B) before the return address label."); + __ mov(marked, old_value); + EmitBakerReadBarrierCbnz(custom_data); + __ bind(&return_address); +} + void CodeGeneratorARM64::GenerateFieldLoadWithBakerReadBarrier(HInstruction* instruction, Location ref, vixl::aarch64::Register obj, @@ -6278,139 +6024,6 @@ void CodeGeneratorARM64::GenerateArrayLoadWithBakerReadBarrier(Location ref, MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__, /* temp_loc */ LocationFrom(ip1)); } -void CodeGeneratorARM64::UpdateReferenceFieldWithBakerReadBarrier(HInstruction* instruction, - Location ref, - Register obj, - Location field_offset, - Register temp, - bool needs_null_check, - bool use_load_acquire) { - DCHECK(kEmitCompilerReadBarrier); - DCHECK(kUseBakerReadBarrier); - // If we are emitting an array load, we should not be using a - // Load Acquire instruction. In other words: - // `instruction->IsArrayGet()` => `!use_load_acquire`. - DCHECK(!instruction->IsArrayGet() || !use_load_acquire); - - // Query `art::Thread::Current()->GetIsGcMarking()` (stored in the - // Marking Register) to decide whether we need to enter the slow - // path to update the reference field within `obj`. Then, in the - // slow path, check the gray bit in the lock word of the reference's - // holder (`obj`) to decide whether to mark `ref` and update the - // field or not. - // - // if (mr) { // Thread::Current()->GetIsGcMarking() - // // Slow path. - // uint32_t rb_state = Lockword(obj->monitor_).ReadBarrierState(); - // lfence; // Load fence or artificial data dependency to prevent load-load reordering - // HeapReference<mirror::Object> ref = *(obj + field_offset); // Reference load. - // bool is_gray = (rb_state == ReadBarrier::GrayState()); - // if (is_gray) { - // old_ref = ref; - // entrypoint = Thread::Current()->pReadBarrierMarkReg ## root.reg() - // ref = entrypoint(ref); // ref = ReadBarrier::Mark(ref); // Runtime entry point call. - // compareAndSwapObject(obj, field_offset, old_ref, ref); - // } - // } - - // Slow path updating the object reference at address `obj + field_offset` - // when the GC is marking. The entrypoint will be loaded by the slow path code. - SlowPathCodeARM64* slow_path = - new (GetScopedAllocator()) LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARM64( - instruction, - ref, - obj, - /* offset */ 0u, - /* index */ field_offset, - /* scale_factor */ 0u /* "times 1" */, - needs_null_check, - use_load_acquire, - temp); - AddSlowPath(slow_path); - - __ Cbnz(mr, slow_path->GetEntryLabel()); - // Fast path: the GC is not marking: nothing to do (the field is - // up-to-date, and we don't need to load the reference). - __ Bind(slow_path->GetExitLabel()); - MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); -} - -void CodeGeneratorARM64::GenerateRawReferenceLoad(HInstruction* instruction, - Location ref, - Register obj, - uint32_t offset, - Location index, - size_t scale_factor, - bool needs_null_check, - bool use_load_acquire) { - DCHECK(obj.IsW()); - DataType::Type type = DataType::Type::kReference; - Register ref_reg = RegisterFrom(ref, type); - - // If needed, vixl::EmissionCheckScope guards are used to ensure - // that no pools are emitted between the load (macro) instruction - // and MaybeRecordImplicitNullCheck. - - if (index.IsValid()) { - // Load types involving an "index": ArrayGet, - // UnsafeGetObject/UnsafeGetObjectVolatile and UnsafeCASObject - // intrinsics. - if (use_load_acquire) { - // UnsafeGetObjectVolatile intrinsic case. - // Register `index` is not an index in an object array, but an - // offset to an object reference field within object `obj`. - DCHECK(instruction->IsInvoke()) << instruction->DebugName(); - DCHECK(instruction->GetLocations()->Intrinsified()); - DCHECK(instruction->AsInvoke()->GetIntrinsic() == Intrinsics::kUnsafeGetObjectVolatile) - << instruction->AsInvoke()->GetIntrinsic(); - DCHECK_EQ(offset, 0u); - DCHECK_EQ(scale_factor, 0u); - DCHECK_EQ(needs_null_check, false); - // /* HeapReference<mirror::Object> */ ref = *(obj + index) - MemOperand field = HeapOperand(obj, XRegisterFrom(index)); - LoadAcquire(instruction, ref_reg, field, /* needs_null_check */ false); - } else { - // ArrayGet and UnsafeGetObject and UnsafeCASObject intrinsics cases. - // /* HeapReference<mirror::Object> */ ref = *(obj + offset + (index << scale_factor)) - if (index.IsConstant()) { - uint32_t computed_offset = offset + (Int64FromLocation(index) << scale_factor); - EmissionCheckScope guard(GetVIXLAssembler(), kMaxMacroInstructionSizeInBytes); - Load(type, ref_reg, HeapOperand(obj, computed_offset)); - if (needs_null_check) { - MaybeRecordImplicitNullCheck(instruction); - } - } else { - UseScratchRegisterScope temps(GetVIXLAssembler()); - Register temp = temps.AcquireW(); - __ Add(temp, obj, offset); - { - EmissionCheckScope guard(GetVIXLAssembler(), kMaxMacroInstructionSizeInBytes); - Load(type, ref_reg, HeapOperand(temp, XRegisterFrom(index), LSL, scale_factor)); - if (needs_null_check) { - MaybeRecordImplicitNullCheck(instruction); - } - } - } - } - } else { - // /* HeapReference<mirror::Object> */ ref = *(obj + offset) - MemOperand field = HeapOperand(obj, offset); - if (use_load_acquire) { - // Implicit null checks are handled by CodeGeneratorARM64::LoadAcquire. - LoadAcquire(instruction, ref_reg, field, needs_null_check); - } else { - EmissionCheckScope guard(GetVIXLAssembler(), kMaxMacroInstructionSizeInBytes); - Load(type, ref_reg, field); - if (needs_null_check) { - MaybeRecordImplicitNullCheck(instruction); - } - } - } - - // Object* ref = ref_addr->AsMirrorPtr() - GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); -} - void CodeGeneratorARM64::MaybeGenerateMarkingRegisterCheck(int code, Location temp_loc) { // The following condition is a compile-time one, so it does not have a run-time cost. if (kEmitCompilerReadBarrier && kUseBakerReadBarrier && kIsDebugBuild) { @@ -6549,7 +6162,7 @@ static void EmitGrayCheckAndFastPath(arm64::Arm64Assembler& assembler, // Load the lock word containing the rb_state. __ Ldr(ip0.W(), lock_word); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); __ Tbnz(ip0.W(), LockWord::kReadBarrierStateShift, slow_path); static_assert( diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h index f3f5079f0a..4f6a44fe4d 100644 --- a/compiler/optimizing/code_generator_arm64.h +++ b/compiler/optimizing/code_generator_arm64.h @@ -671,6 +671,9 @@ class CodeGeneratorARM64 : public CodeGenerator { uint32_t offset, vixl::aarch64::Label* fixup_label, ReadBarrierOption read_barrier_option); + // Generate MOV for the `old_value` in UnsafeCASObject and mark it with Baker read barrier. + void GenerateUnsafeCasOldValueMovWithBakerReadBarrier(vixl::aarch64::Register marked, + vixl::aarch64::Register old_value); // Fast path implementation of ReadBarrier::Barrier for a heap // reference field load when Baker's read barriers are used. // Overload suitable for Unsafe.getObject/-Volatile() intrinsic. @@ -698,36 +701,6 @@ class CodeGeneratorARM64 : public CodeGenerator { vixl::aarch64::Register temp, bool needs_null_check); - // Generate code checking whether the the reference field at the - // address `obj + field_offset`, held by object `obj`, needs to be - // marked, and if so, marking it and updating the field within `obj` - // with the marked value. - // - // This routine is used for the implementation of the - // UnsafeCASObject intrinsic with Baker read barriers. - // - // This method has a structure similar to - // GenerateReferenceLoadWithBakerReadBarrier, but note that argument - // `ref` is only as a temporary here, and thus its value should not - // be used afterwards. - void UpdateReferenceFieldWithBakerReadBarrier(HInstruction* instruction, - Location ref, - vixl::aarch64::Register obj, - Location field_offset, - vixl::aarch64::Register temp, - bool needs_null_check, - bool use_load_acquire); - - // Generate a heap reference load (with no read barrier). - void GenerateRawReferenceLoad(HInstruction* instruction, - Location ref, - vixl::aarch64::Register obj, - uint32_t offset, - Location index, - size_t scale_factor, - bool needs_null_check, - bool use_load_acquire); - // Emit code checking the status of the Marking Register, and // aborting the program if MR does not match the value stored in the // art::Thread object. Code is only emitted in debug mode and if diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index ce93afc01c..d811e0711b 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -899,7 +899,7 @@ class LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARMVIXL // Given the numeric representation, it's enough to check the low bit of the // rb_state. We do that by shifting the bit out of the lock word with LSRS // which can be a 16-bit instruction unlike the TST immediate. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); __ Lsrs(temp1_, temp1_, LockWord::kReadBarrierStateShift + 1); __ B(cc, GetExitLabel()); // Carry flag is the last bit shifted out by LSRS. @@ -6844,9 +6844,25 @@ void CodeGeneratorARMVIXL::MarkGCCard(vixl32::Register temp, if (can_be_null) { __ CompareAndBranchIfZero(value, &is_null); } + // Load the address of the card table into `card`. GetAssembler()->LoadFromOffset( kLoadWord, card, tr, Thread::CardTableOffset<kArmPointerSize>().Int32Value()); + // Calculate the offset (in the card table) of the card corresponding to + // `object`. __ Lsr(temp, object, Operand::From(gc::accounting::CardTable::kCardShift)); + // Write the `art::gc::accounting::CardTable::kCardDirty` value into the + // `object`'s card. + // + // Register `card` contains the address of the card table. Note that the card + // table's base is biased during its creation so that it always starts at an + // address whose least-significant byte is equal to `kCardDirty` (see + // art::gc::accounting::CardTable::Create). Therefore the STRB instruction + // below writes the `kCardDirty` (byte) value into the `object`'s card + // (located at `card + object >> kCardShift`). + // + // This dual use of the value in register `card` (1. to calculate the location + // of the card to mark; and 2. to load the `kCardDirty` value) saves a load + // (no need to explicitly load `kCardDirty` as an immediate value). __ Strb(card, MemOperand(card, temp)); if (can_be_null) { __ Bind(&is_null); @@ -9645,7 +9661,7 @@ static void EmitGrayCheckAndFastPath(ArmVIXLAssembler& assembler, // Load the lock word containing the rb_state. __ Ldr(ip, lock_word); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); __ Tst(ip, Operand(LockWord::kReadBarrierStateMaskShifted)); __ B(ne, slow_path, /* is_far_target */ false); diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index 0ed5756b53..aed334b024 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -1868,12 +1868,27 @@ void CodeGeneratorMIPS::MarkGCCard(Register object, if (value_can_be_null) { __ Beqz(value, &done); } + // Load the address of the card table into `card`. __ LoadFromOffset(kLoadWord, card, TR, Thread::CardTableOffset<kMipsPointerSize>().Int32Value()); + // Calculate the address of the card corresponding to `object`. __ Srl(temp, object, gc::accounting::CardTable::kCardShift); __ Addu(temp, card, temp); + // Write the `art::gc::accounting::CardTable::kCardDirty` value into the + // `object`'s card. + // + // Register `card` contains the address of the card table. Note that the card + // table's base is biased during its creation so that it always starts at an + // address whose least-significant byte is equal to `kCardDirty` (see + // art::gc::accounting::CardTable::Create). Therefore the SB instruction + // below writes the `kCardDirty` (byte) value into the `object`'s card + // (located at `card + object >> kCardShift`). + // + // This dual use of the value in register `card` (1. to calculate the location + // of the card to mark; and 2. to load the `kCardDirty` value) saves a load + // (no need to explicitly load `kCardDirty` as an immediate value). __ Sb(card, temp, 0); if (value_can_be_null) { __ Bind(&done); @@ -7439,7 +7454,7 @@ void CodeGeneratorMIPS::GenerateReferenceLoadWithBakerReadBarrier(HInstruction* // Given the numeric representation, it's enough to check the low bit of the // rb_state. We do that by shifting the bit into the sign bit (31) and // performing a branch on less than zero. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); static_assert(LockWord::kReadBarrierStateSize == 1, "Expecting 1-bit read barrier state size"); __ Sll(temp_reg, temp_reg, 31 - LockWord::kReadBarrierStateShift); diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 2b6928eee2..72318e98b0 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -1490,12 +1490,27 @@ void CodeGeneratorMIPS64::MarkGCCard(GpuRegister object, if (value_can_be_null) { __ Beqzc(value, &done); } + // Load the address of the card table into `card`. __ LoadFromOffset(kLoadDoubleword, card, TR, Thread::CardTableOffset<kMips64PointerSize>().Int32Value()); + // Calculate the address of the card corresponding to `object`. __ Dsrl(temp, object, gc::accounting::CardTable::kCardShift); __ Daddu(temp, card, temp); + // Write the `art::gc::accounting::CardTable::kCardDirty` value into the + // `object`'s card. + // + // Register `card` contains the address of the card table. Note that the card + // table's base is biased during its creation so that it always starts at an + // address whose least-significant byte is equal to `kCardDirty` (see + // art::gc::accounting::CardTable::Create). Therefore the SB instruction + // below writes the `kCardDirty` (byte) value into the `object`'s card + // (located at `card + object >> kCardShift`). + // + // This dual use of the value in register `card` (1. to calculate the location + // of the card to mark; and 2. to load the `kCardDirty` value) saves a load + // (no need to explicitly load `kCardDirty` as an immediate value). __ Sb(card, temp, 0); if (value_can_be_null) { __ Bind(&done); @@ -5555,7 +5570,7 @@ void CodeGeneratorMIPS64::GenerateReferenceLoadWithBakerReadBarrier(HInstruction // Given the numeric representation, it's enough to check the low bit of the // rb_state. We do that by shifting the bit into the sign bit (31) and // performing a branch on less than zero. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); static_assert(LockWord::kReadBarrierStateSize == 1, "Expecting 1-bit read barrier state size"); __ Sll(temp_reg, temp_reg, 31 - LockWord::kReadBarrierStateShift); diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index a835aed6b0..df00ec7d30 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -5104,9 +5104,25 @@ void CodeGeneratorX86::MarkGCCard(Register temp, __ testl(value, value); __ j(kEqual, &is_null); } + // Load the address of the card table into `card`. __ fs()->movl(card, Address::Absolute(Thread::CardTableOffset<kX86PointerSize>().Int32Value())); + // Calculate the offset (in the card table) of the card corresponding to + // `object`. __ movl(temp, object); __ shrl(temp, Immediate(gc::accounting::CardTable::kCardShift)); + // Write the `art::gc::accounting::CardTable::kCardDirty` value into the + // `object`'s card. + // + // Register `card` contains the address of the card table. Note that the card + // table's base is biased during its creation so that it always starts at an + // address whose least-significant byte is equal to `kCardDirty` (see + // art::gc::accounting::CardTable::Create). Therefore the MOVB instruction + // below writes the `kCardDirty` (byte) value into the `object`'s card + // (located at `card + object >> kCardShift`). + // + // This dual use of the value in register `card` (1. to calculate the location + // of the card to mark; and 2. to load the `kCardDirty` value) saves a load + // (no need to explicitly load `kCardDirty` as an immediate value). __ movb(Address(temp, card, TIMES_1, 0), X86ManagedRegister::FromCpuRegister(card).AsByteRegister()); if (value_can_be_null) { @@ -7741,7 +7757,7 @@ void CodeGeneratorX86::GenerateReferenceLoadWithBakerReadBarrier(HInstruction* i uint32_t monitor_offset = mirror::Object::MonitorOffset().Int32Value(); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); constexpr uint32_t gray_byte_position = LockWord::kReadBarrierStateShift / kBitsPerByte; constexpr uint32_t gray_bit_position = LockWord::kReadBarrierStateShift % kBitsPerByte; diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index dee891b8de..ae2a000d07 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -5436,10 +5436,26 @@ void CodeGeneratorX86_64::MarkGCCard(CpuRegister temp, __ testl(value, value); __ j(kEqual, &is_null); } + // Load the address of the card table into `card`. __ gs()->movq(card, Address::Absolute(Thread::CardTableOffset<kX86_64PointerSize>().Int32Value(), /* no_rip */ true)); + // Calculate the offset (in the card table) of the card corresponding to + // `object`. __ movq(temp, object); __ shrq(temp, Immediate(gc::accounting::CardTable::kCardShift)); + // Write the `art::gc::accounting::CardTable::kCardDirty` value into the + // `object`'s card. + // + // Register `card` contains the address of the card table. Note that the card + // table's base is biased during its creation so that it always starts at an + // address whose least-significant byte is equal to `kCardDirty` (see + // art::gc::accounting::CardTable::Create). Therefore the MOVB instruction + // below writes the `kCardDirty` (byte) value into the `object`'s card + // (located at `card + object >> kCardShift`). + // + // This dual use of the value in register `card` (1. to calculate the location + // of the card to mark; and 2. to load the `kCardDirty` value) saves a load + // (no need to explicitly load `kCardDirty` as an immediate value). __ movb(Address(temp, card, TIMES_1, 0), card); if (value_can_be_null) { __ Bind(&is_null); @@ -7034,7 +7050,7 @@ void CodeGeneratorX86_64::GenerateReferenceLoadWithBakerReadBarrier(HInstruction uint32_t monitor_offset = mirror::Object::MonitorOffset().Int32Value(); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); constexpr uint32_t gray_byte_position = LockWord::kReadBarrierStateShift / kBitsPerByte; constexpr uint32_t gray_bit_position = LockWord::kReadBarrierStateShift % kBitsPerByte; diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index e95cc6e79e..a657b5818f 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -984,106 +984,155 @@ static void CreateIntIntIntIntIntToInt(ArenaAllocator* allocator, ? LocationSummary::kCallOnSlowPath : LocationSummary::kNoCall, kIntrinsified); + if (can_call) { + locations->SetCustomSlowPathCallerSaves(RegisterSet::Empty()); // No caller-save registers. + } locations->SetInAt(0, Location::NoLocation()); // Unused receiver. locations->SetInAt(1, Location::RequiresRegister()); locations->SetInAt(2, Location::RequiresRegister()); locations->SetInAt(3, Location::RequiresRegister()); locations->SetInAt(4, Location::RequiresRegister()); - // If heap poisoning is enabled, we don't want the unpoisoning - // operations to potentially clobber the output. Likewise when - // emitting a (Baker) read barrier, which may call. - Location::OutputOverlap overlaps = - ((kPoisonHeapReferences && type == DataType::Type::kReference) || can_call) - ? Location::kOutputOverlap - : Location::kNoOutputOverlap; - locations->SetOut(Location::RequiresRegister(), overlaps); + locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap); if (type == DataType::Type::kReference && kEmitCompilerReadBarrier && kUseBakerReadBarrier) { - // Temporary register for (Baker) read barrier. + // We need two non-scratch temporary registers for (Baker) read barrier. + locations->AddTemp(Location::RequiresRegister()); locations->AddTemp(Location::RequiresRegister()); } } +class BakerReadBarrierCasSlowPathARM64 : public SlowPathCodeARM64 { + public: + explicit BakerReadBarrierCasSlowPathARM64(HInvoke* invoke) + : SlowPathCodeARM64(invoke) {} + + const char* GetDescription() const OVERRIDE { return "BakerReadBarrierCasSlowPathARM64"; } + + void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { + CodeGeneratorARM64* arm64_codegen = down_cast<CodeGeneratorARM64*>(codegen); + Arm64Assembler* assembler = arm64_codegen->GetAssembler(); + MacroAssembler* masm = assembler->GetVIXLAssembler(); + __ Bind(GetEntryLabel()); + + // Get the locations. + LocationSummary* locations = instruction_->GetLocations(); + Register base = WRegisterFrom(locations->InAt(1)); // Object pointer. + Register offset = XRegisterFrom(locations->InAt(2)); // Long offset. + Register expected = WRegisterFrom(locations->InAt(3)); // Expected. + Register value = WRegisterFrom(locations->InAt(4)); // Value. + + Register old_value = WRegisterFrom(locations->GetTemp(0)); // The old value from main path. + Register marked = WRegisterFrom(locations->GetTemp(1)); // The marked old value. + + // Mark the `old_value` from the main path and compare with `expected`. This clobbers the + // `tmp_ptr` scratch register but we do not want to allocate another non-scratch temporary. + arm64_codegen->GenerateUnsafeCasOldValueMovWithBakerReadBarrier(marked, old_value); + __ Cmp(marked, expected); + __ B(GetExitLabel(), ne); // If taken, Z=false indicates failure. + + // The `old_value` we have read did not match `expected` (which is always a to-space reference) + // but after the read barrier in GenerateUnsafeCasOldValueMovWithBakerReadBarrier() the marked + // to-space value matched, so the `old_value` must be a from-space reference to the same + // object. Do the same CAS loop as the main path but check for both `expected` and the unmarked + // old value representing the to-space and from-space references for the same object. + + UseScratchRegisterScope temps(masm); + Register tmp_ptr = temps.AcquireX(); + Register tmp = temps.AcquireSameSizeAs(value); + + // Recalculate the `tmp_ptr` clobbered above. + __ Add(tmp_ptr, base.X(), Operand(offset)); + + // do { + // tmp_value = [tmp_ptr]; + // } while ((tmp_value == expected || tmp == old_value) && failure([tmp_ptr] <- r_new_value)); + // result = (tmp_value == expected || tmp == old_value); + + vixl::aarch64::Label loop_head; + __ Bind(&loop_head); + __ Ldaxr(tmp, MemOperand(tmp_ptr)); + assembler->MaybeUnpoisonHeapReference(tmp); + __ Cmp(tmp, expected); + __ Ccmp(tmp, old_value, ZFlag, ne); + __ B(GetExitLabel(), ne); // If taken, Z=false indicates failure. + assembler->MaybePoisonHeapReference(value); + __ Stlxr(tmp.W(), value, MemOperand(tmp_ptr)); + assembler->MaybeUnpoisonHeapReference(value); + __ Cbnz(tmp.W(), &loop_head); + + // Z=true from the above CMP+CCMP indicates success. + __ B(GetExitLabel()); + } +}; + static void GenCas(HInvoke* invoke, DataType::Type type, CodeGeneratorARM64* codegen) { - MacroAssembler* masm = codegen->GetVIXLAssembler(); + Arm64Assembler* assembler = codegen->GetAssembler(); + MacroAssembler* masm = assembler->GetVIXLAssembler(); LocationSummary* locations = invoke->GetLocations(); - Location out_loc = locations->Out(); - Register out = WRegisterFrom(out_loc); // Boolean result. - - Register base = WRegisterFrom(locations->InAt(1)); // Object pointer. - Location offset_loc = locations->InAt(2); - Register offset = XRegisterFrom(offset_loc); // Long offset. - Register expected = RegisterFrom(locations->InAt(3), type); // Expected. - Register value = RegisterFrom(locations->InAt(4), type); // Value. + Register out = WRegisterFrom(locations->Out()); // Boolean result. + Register base = WRegisterFrom(locations->InAt(1)); // Object pointer. + Register offset = XRegisterFrom(locations->InAt(2)); // Long offset. + Register expected = RegisterFrom(locations->InAt(3), type); // Expected. + Register value = RegisterFrom(locations->InAt(4), type); // Value. // This needs to be before the temp registers, as MarkGCCard also uses VIXL temps. if (type == DataType::Type::kReference) { // Mark card for object assuming new value is stored. bool value_can_be_null = true; // TODO: Worth finding out this information? codegen->MarkGCCard(base, value, value_can_be_null); - - // The only read barrier implementation supporting the - // UnsafeCASObject intrinsic is the Baker-style read barriers. - DCHECK(!kEmitCompilerReadBarrier || kUseBakerReadBarrier); - - if (kEmitCompilerReadBarrier && kUseBakerReadBarrier) { - Register temp = WRegisterFrom(locations->GetTemp(0)); - // Need to make sure the reference stored in the field is a to-space - // one before attempting the CAS or the CAS could fail incorrectly. - codegen->UpdateReferenceFieldWithBakerReadBarrier( - invoke, - out_loc, // Unused, used only as a "temporary" within the read barrier. - base, - /* field_offset */ offset_loc, - temp, - /* needs_null_check */ false, - /* use_load_acquire */ false); - } } UseScratchRegisterScope temps(masm); Register tmp_ptr = temps.AcquireX(); // Pointer to actual memory. - Register tmp_value = temps.AcquireSameSizeAs(value); // Value in memory. + Register old_value; // Value in memory. - Register tmp_32 = tmp_value.W(); + vixl::aarch64::Label exit_loop_label; + vixl::aarch64::Label* exit_loop = &exit_loop_label; + vixl::aarch64::Label* failure = &exit_loop_label; - __ Add(tmp_ptr, base.X(), Operand(offset)); + if (kEmitCompilerReadBarrier && type == DataType::Type::kReference) { + // The only read barrier implementation supporting the + // UnsafeCASObject intrinsic is the Baker-style read barriers. + DCHECK(kUseBakerReadBarrier); - if (kPoisonHeapReferences && type == DataType::Type::kReference) { - codegen->GetAssembler()->PoisonHeapReference(expected); - if (value.Is(expected)) { - // Do not poison `value`, as it is the same register as - // `expected`, which has just been poisoned. - } else { - codegen->GetAssembler()->PoisonHeapReference(value); - } + BakerReadBarrierCasSlowPathARM64* slow_path = + new (codegen->GetScopedAllocator()) BakerReadBarrierCasSlowPathARM64(invoke); + codegen->AddSlowPath(slow_path); + exit_loop = slow_path->GetExitLabel(); + failure = slow_path->GetEntryLabel(); + // We need to store the `old_value` in a non-scratch register to make sure + // the Baker read barrier in the slow path does not clobber it. + old_value = WRegisterFrom(locations->GetTemp(0)); + } else { + old_value = temps.AcquireSameSizeAs(value); } + __ Add(tmp_ptr, base.X(), Operand(offset)); + // do { - // tmp_value = [tmp_ptr] - expected; - // } while (tmp_value == 0 && failure([tmp_ptr] <- r_new_value)); - // result = tmp_value != 0; + // tmp_value = [tmp_ptr]; + // } while (tmp_value == expected && failure([tmp_ptr] <- r_new_value)); + // result = tmp_value == expected; - vixl::aarch64::Label loop_head, exit_loop; + vixl::aarch64::Label loop_head; __ Bind(&loop_head); - __ Ldaxr(tmp_value, MemOperand(tmp_ptr)); - __ Cmp(tmp_value, expected); - __ B(&exit_loop, ne); - __ Stlxr(tmp_32, value, MemOperand(tmp_ptr)); - __ Cbnz(tmp_32, &loop_head); - __ Bind(&exit_loop); - __ Cset(out, eq); - - if (kPoisonHeapReferences && type == DataType::Type::kReference) { - codegen->GetAssembler()->UnpoisonHeapReference(expected); - if (value.Is(expected)) { - // Do not unpoison `value`, as it is the same register as - // `expected`, which has just been unpoisoned. - } else { - codegen->GetAssembler()->UnpoisonHeapReference(value); - } + __ Ldaxr(old_value, MemOperand(tmp_ptr)); + if (type == DataType::Type::kReference) { + assembler->MaybeUnpoisonHeapReference(old_value); } + __ Cmp(old_value, expected); + __ B(failure, ne); + if (type == DataType::Type::kReference) { + assembler->MaybePoisonHeapReference(value); + } + __ Stlxr(old_value.W(), value, MemOperand(tmp_ptr)); // Reuse `old_value` for STLXR result. + if (type == DataType::Type::kReference) { + assembler->MaybeUnpoisonHeapReference(value); + } + __ Cbnz(old_value.W(), &loop_head); + __ Bind(exit_loop); + __ Cset(out, eq); } void IntrinsicLocationsBuilderARM64::VisitUnsafeCASInt(HInvoke* invoke) { @@ -2690,7 +2739,7 @@ void IntrinsicCodeGeneratorARM64::VisitSystemArrayCopy(HInvoke* invoke) { codegen_->AddSlowPath(read_barrier_slow_path); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); __ Tbnz(tmp, LockWord::kReadBarrierStateShift, read_barrier_slow_path->GetEntryLabel()); diff --git a/compiler/optimizing/intrinsics_arm_vixl.cc b/compiler/optimizing/intrinsics_arm_vixl.cc index b92075053e..53b0aa2560 100644 --- a/compiler/optimizing/intrinsics_arm_vixl.cc +++ b/compiler/optimizing/intrinsics_arm_vixl.cc @@ -2205,7 +2205,7 @@ void IntrinsicCodeGeneratorARMVIXL::VisitSystemArrayCopy(HInvoke* invoke) { // Given the numeric representation, it's enough to check the low bit of the // rb_state. We do that by shifting the bit out of the lock word with LSRS // which can be a 16-bit instruction unlike the TST immediate. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); __ Lsrs(temp2, temp2, LockWord::kReadBarrierStateShift + 1); // Carry flag is the last bit shifted out by LSRS. diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc index 98cea35af1..5c7be54037 100644 --- a/compiler/optimizing/intrinsics_x86.cc +++ b/compiler/optimizing/intrinsics_x86.cc @@ -2781,7 +2781,7 @@ void IntrinsicCodeGeneratorX86::VisitSystemArrayCopy(HInvoke* invoke) { __ j(kEqual, &done); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); constexpr uint32_t gray_byte_position = LockWord::kReadBarrierStateShift / kBitsPerByte; constexpr uint32_t gray_bit_position = LockWord::kReadBarrierStateShift % kBitsPerByte; diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc index ac6eab0834..b5afe931ff 100644 --- a/compiler/optimizing/intrinsics_x86_64.cc +++ b/compiler/optimizing/intrinsics_x86_64.cc @@ -1143,7 +1143,7 @@ void IntrinsicCodeGeneratorX86_64::VisitSystemArrayCopy(HInvoke* invoke) { __ j(kEqual, &done); // Given the numeric representation, it's enough to check the low bit of the rb_state. - static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); + static_assert(ReadBarrier::NonGrayState() == 0, "Expecting non-gray to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); constexpr uint32_t gray_byte_position = LockWord::kReadBarrierStateShift / kBitsPerByte; constexpr uint32_t gray_bit_position = LockWord::kReadBarrierStateShift % kBitsPerByte; diff --git a/libdexfile/dex/dex_file-inl.h b/libdexfile/dex/dex_file-inl.h index 09668594dd..c512361586 100644 --- a/libdexfile/dex/dex_file-inl.h +++ b/libdexfile/dex/dex_file-inl.h @@ -204,26 +204,6 @@ inline bool Signature::operator==(const Signature& rhs) const { return true; } -inline -InvokeType ClassDataItemIterator::GetMethodInvokeType(const DexFile::ClassDef& class_def) const { - if (HasNextDirectMethod()) { - if ((GetRawMemberAccessFlags() & kAccStatic) != 0) { - return kStatic; - } else { - return kDirect; - } - } else { - DCHECK_EQ(GetRawMemberAccessFlags() & kAccStatic, 0U); - if ((class_def.access_flags_ & kAccInterface) != 0) { - return kInterface; - } else if ((GetRawMemberAccessFlags() & kAccConstructor) != 0) { - return kSuper; - } else { - return kVirtual; - } - } -} - template<typename NewLocalCallback, typename IndexToStringData, typename TypeIndexToStringData> bool DexFile::DecodeDebugLocalInfo(const uint8_t* stream, const std::string& location, @@ -518,18 +498,6 @@ inline const uint8_t* DexFile::GetCatchHandlerData(const DexInstructionIterator& return handler_data + offset; } -template <typename Visitor> -inline void DexFile::ClassDef::VisitMethods(const DexFile* dex_file, const Visitor& visitor) const { - const uint8_t* class_data = dex_file->GetClassData(*this); - if (class_data != nullptr) { - ClassDataItemIterator it(*dex_file, class_data); - it.SkipAllFields(); - for (; it.HasNext(); it.Next()) { - visitor(it); - } - } -} - inline IterationRange<ClassIterator> DexFile::GetClasses() const { return { ClassIterator(*this, 0u), ClassIterator(*this, NumClassDefs()) }; } diff --git a/libdexfile/dex/dex_file.cc b/libdexfile/dex/dex_file.cc index f1f896058c..a2198b7c98 100644 --- a/libdexfile/dex/dex_file.cc +++ b/libdexfile/dex/dex_file.cc @@ -31,6 +31,7 @@ #include "base/enums.h" #include "base/leb128.h" #include "base/stl_util.h" +#include "class_accessor-inl.h" #include "descriptors_names.h" #include "dex_file-inl.h" #include "standard_dex_file.h" @@ -219,21 +220,12 @@ const DexFile::ClassDef* DexFile::FindClassDef(dex::TypeIndex type_idx) const { uint32_t DexFile::FindCodeItemOffset(const DexFile::ClassDef& class_def, uint32_t method_idx) const { - const uint8_t* class_data = GetClassData(class_def); - CHECK(class_data != nullptr); - ClassDataItemIterator it(*this, class_data); - it.SkipAllFields(); - while (it.HasNextDirectMethod()) { - if (it.GetMemberIndex() == method_idx) { - return it.GetMethodCodeItemOffset(); + ClassAccessor accessor(*this, class_def); + CHECK(accessor.HasClassData()); + for (const ClassAccessor::Method& method : accessor.GetMethods()) { + if (method.GetIndex() == method_idx) { + return method.GetCodeItemOffset(); } - it.Next(); - } - while (it.HasNextVirtualMethod()) { - if (it.GetMemberIndex() == method_idx) { - return it.GetMethodCodeItemOffset(); - } - it.Next(); } LOG(FATAL) << "Unable to find method " << method_idx; UNREACHABLE(); @@ -684,31 +676,6 @@ std::ostream& operator<<(std::ostream& os, const Signature& sig) { return os << sig.ToString(); } -// Decodes the header section from the class data bytes. -void ClassDataItemIterator::ReadClassDataHeader() { - CHECK(ptr_pos_ != nullptr); - header_.static_fields_size_ = DecodeUnsignedLeb128(&ptr_pos_); - header_.instance_fields_size_ = DecodeUnsignedLeb128(&ptr_pos_); - header_.direct_methods_size_ = DecodeUnsignedLeb128(&ptr_pos_); - header_.virtual_methods_size_ = DecodeUnsignedLeb128(&ptr_pos_); -} - -void ClassDataItemIterator::ReadClassDataField() { - field_.field_idx_delta_ = DecodeUnsignedLeb128(&ptr_pos_); - field_.access_flags_ = DecodeUnsignedLeb128(&ptr_pos_); - // The user of the iterator is responsible for checking if there - // are unordered or duplicate indexes. -} - -void ClassDataItemIterator::ReadClassDataMethod() { - method_.method_idx_delta_ = DecodeUnsignedLeb128(&ptr_pos_); - method_.access_flags_ = DecodeUnsignedLeb128(&ptr_pos_); - method_.code_off_ = DecodeUnsignedLeb128(&ptr_pos_); - if (last_idx_ != 0 && method_.method_idx_delta_ == 0) { - LOG(WARNING) << "Duplicate method in " << dex_file_.GetLocation(); - } -} - EncodedArrayValueIterator::EncodedArrayValueIterator(const DexFile& dex_file, const uint8_t* array_data) : dex_file_(dex_file), diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h index 25cd2f4ddc..98787d1dd0 100644 --- a/libdexfile/dex/dex_file.h +++ b/libdexfile/dex/dex_file.h @@ -238,9 +238,6 @@ class DexFile { } } - template <typename Visitor> - void VisitMethods(const DexFile* dex_file, const Visitor& visitor) const; - private: DISALLOW_COPY_AND_ASSIGN(ClassDef); }; @@ -1174,211 +1171,6 @@ class Signature : public ValueObject { }; std::ostream& operator<<(std::ostream& os, const Signature& sig); -// Iterate and decode class_data_item -class ClassDataItemIterator { - public: - ClassDataItemIterator(const DexFile& dex_file, const uint8_t* raw_class_data_item) - : dex_file_(dex_file), pos_(0), ptr_pos_(raw_class_data_item), last_idx_(0) { - ReadClassDataHeader(); - if (EndOfInstanceFieldsPos() > 0) { - ReadClassDataField(); - } else if (EndOfVirtualMethodsPos() > 0) { - ReadClassDataMethod(); - } - } - uint32_t NumStaticFields() const { - return header_.static_fields_size_; - } - uint32_t NumInstanceFields() const { - return header_.instance_fields_size_; - } - uint32_t NumDirectMethods() const { - return header_.direct_methods_size_; - } - uint32_t NumVirtualMethods() const { - return header_.virtual_methods_size_; - } - bool IsAtMethod() const { - return pos_ >= EndOfInstanceFieldsPos(); - } - bool IsAtVirtualMethod() const { - return pos_ >= EndOfDirectMethodsPos(); - } - bool HasNextStaticField() const { - return pos_ < EndOfStaticFieldsPos(); - } - bool HasNextInstanceField() const { - return pos_ >= EndOfStaticFieldsPos() && pos_ < EndOfInstanceFieldsPos(); - } - bool HasNextDirectMethod() const { - return pos_ >= EndOfInstanceFieldsPos() && pos_ < EndOfDirectMethodsPos(); - } - bool HasNextVirtualMethod() const { - return pos_ >= EndOfDirectMethodsPos() && pos_ < EndOfVirtualMethodsPos(); - } - bool HasNextMethod() const { - const bool result = pos_ >= EndOfInstanceFieldsPos() && pos_ < EndOfVirtualMethodsPos(); - DCHECK_EQ(result, HasNextDirectMethod() || HasNextVirtualMethod()); - return result; - } - void SkipStaticFields() { - while (HasNextStaticField()) { - Next(); - } - } - void SkipInstanceFields() { - while (HasNextInstanceField()) { - Next(); - } - } - void SkipAllFields() { - SkipStaticFields(); - SkipInstanceFields(); - } - void SkipDirectMethods() { - while (HasNextDirectMethod()) { - Next(); - } - } - void SkipVirtualMethods() { - while (HasNextVirtualMethod()) { - Next(); - } - } - bool HasNext() const { - return pos_ < EndOfVirtualMethodsPos(); - } - inline void Next() { - pos_++; - if (pos_ < EndOfStaticFieldsPos()) { - last_idx_ = GetMemberIndex(); - ReadClassDataField(); - } else if (pos_ == EndOfStaticFieldsPos() && NumInstanceFields() > 0) { - last_idx_ = 0; // transition to next array, reset last index - ReadClassDataField(); - } else if (pos_ < EndOfInstanceFieldsPos()) { - last_idx_ = GetMemberIndex(); - ReadClassDataField(); - } else if (pos_ == EndOfInstanceFieldsPos() && NumDirectMethods() > 0) { - last_idx_ = 0; // transition to next array, reset last index - ReadClassDataMethod(); - } else if (pos_ < EndOfDirectMethodsPos()) { - last_idx_ = GetMemberIndex(); - ReadClassDataMethod(); - } else if (pos_ == EndOfDirectMethodsPos() && NumVirtualMethods() > 0) { - last_idx_ = 0; // transition to next array, reset last index - ReadClassDataMethod(); - } else if (pos_ < EndOfVirtualMethodsPos()) { - last_idx_ = GetMemberIndex(); - ReadClassDataMethod(); - } else { - DCHECK(!HasNext()); - } - } - uint32_t GetMemberIndex() const { - if (pos_ < EndOfInstanceFieldsPos()) { - return last_idx_ + field_.field_idx_delta_; - } else { - DCHECK_LT(pos_, EndOfVirtualMethodsPos()); - return last_idx_ + method_.method_idx_delta_; - } - } - uint32_t GetRawMemberAccessFlags() const { - if (pos_ < EndOfInstanceFieldsPos()) { - return field_.access_flags_; - } else { - DCHECK_LT(pos_, EndOfVirtualMethodsPos()); - return method_.access_flags_; - } - } - uint32_t GetFieldAccessFlags() const { - return GetMemberAccessFlags() & kAccValidFieldFlags; - } - uint32_t GetMethodAccessFlags() const { - return GetMemberAccessFlags() & kAccValidMethodFlags; - } - uint32_t GetMemberAccessFlags() const { - return HiddenApiAccessFlags::RemoveFromDex(GetRawMemberAccessFlags()); - } - HiddenApiAccessFlags::ApiList DecodeHiddenAccessFlags() const { - return HiddenApiAccessFlags::DecodeFromDex(GetRawMemberAccessFlags()); - } - bool MemberIsNative() const { - return GetRawMemberAccessFlags() & kAccNative; - } - bool MemberIsFinal() const { - return GetRawMemberAccessFlags() & kAccFinal; - } - ALWAYS_INLINE InvokeType GetMethodInvokeType(const DexFile::ClassDef& class_def) const; - const DexFile::CodeItem* GetMethodCodeItem() const { - return dex_file_.GetCodeItem(method_.code_off_); - } - uint32_t GetMethodCodeItemOffset() const { - return method_.code_off_; - } - const uint8_t* DataPointer() const { - return ptr_pos_; - } - const uint8_t* EndDataPointer() const { - CHECK(!HasNext()); - return ptr_pos_; - } - - private: - // A dex file's class_data_item is leb128 encoded, this structure holds a decoded form of the - // header for a class_data_item - struct ClassDataHeader { - uint32_t static_fields_size_; // the number of static fields - uint32_t instance_fields_size_; // the number of instance fields - uint32_t direct_methods_size_; // the number of direct methods - uint32_t virtual_methods_size_; // the number of virtual methods - } header_; - - // Read and decode header from a class_data_item stream into header - void ReadClassDataHeader(); - - uint32_t EndOfStaticFieldsPos() const { - return header_.static_fields_size_; - } - uint32_t EndOfInstanceFieldsPos() const { - return EndOfStaticFieldsPos() + header_.instance_fields_size_; - } - uint32_t EndOfDirectMethodsPos() const { - return EndOfInstanceFieldsPos() + header_.direct_methods_size_; - } - uint32_t EndOfVirtualMethodsPos() const { - return EndOfDirectMethodsPos() + header_.virtual_methods_size_; - } - - // A decoded version of the field of a class_data_item - struct ClassDataField { - uint32_t field_idx_delta_; // delta of index into the field_ids array for FieldId - uint32_t access_flags_; // access flags for the field - ClassDataField() : field_idx_delta_(0), access_flags_(0) {} - }; - ClassDataField field_; - - // Read and decode a field from a class_data_item stream into field - void ReadClassDataField(); - - // A decoded version of the method of a class_data_item - struct ClassDataMethod { - uint32_t method_idx_delta_; // delta of index into the method_ids array for MethodId - uint32_t access_flags_; - uint32_t code_off_; - ClassDataMethod() : method_idx_delta_(0), access_flags_(0), code_off_(0) {} - }; - ClassDataMethod method_; - - // Read and decode a method from a class_data_item stream into method - void ReadClassDataMethod(); - - const DexFile& dex_file_; - size_t pos_; // integral number of items passed - const uint8_t* ptr_pos_; // pointer into stream of class_data_item - uint32_t last_idx_; // last read field or method index to apply delta to -}; - class EncodedArrayValueIterator { public: EncodedArrayValueIterator(const DexFile& dex_file, const uint8_t* array_data); diff --git a/libdexfile/dex/modifiers.h b/libdexfile/dex/modifiers.h index be82fff65c..38f8455b64 100644 --- a/libdexfile/dex/modifiers.h +++ b/libdexfile/dex/modifiers.h @@ -42,9 +42,8 @@ static constexpr uint32_t kAccEnum = 0x4000; // class, field, ic (1.5) static constexpr uint32_t kAccJavaFlagsMask = 0xffff; // bits set from Java sources (low 16) -// The following flags are used to insert hidden API access flags into boot -// class path dex files. They are decoded by DexFile::ClassDataItemIterator and -// removed from the access flags before used by the runtime. +// The following flags are used to insert hidden API access flags into boot class path dex files. +// They are decoded by ClassAccessor and removed from the access flags before used by the runtime. static constexpr uint32_t kAccDexHiddenBit = 0x00000020; // field, method (not native) static constexpr uint32_t kAccDexHiddenBitNative = 0x00000200; // method (native) diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc index 2f24d7ea3d..d20c756522 100644 --- a/openjdkjvmti/deopt_manager.cc +++ b/openjdkjvmti/deopt_manager.cc @@ -40,6 +40,8 @@ #include "dex/dex_file_annotations.h" #include "dex/modifiers.h" #include "events-inl.h" +#include "gc/heap.h" +#include "gc/scoped_gc_critical_section.h" #include "jit/jit.h" #include "jni/jni_internal.h" #include "mirror/class-inl.h" @@ -266,29 +268,35 @@ void DeoptManager::WaitForDeoptimizationToFinish(art::Thread* self) { deoptimization_status_lock_.ExclusiveUnlock(self); } +// Users should make sure that only gc-critical-section safe code is used while a +// ScopedDeoptimizationContext exists. class ScopedDeoptimizationContext : public art::ValueObject { public: ScopedDeoptimizationContext(art::Thread* self, DeoptManager* deopt) RELEASE(deopt->deoptimization_status_lock_) ACQUIRE(art::Locks::mutator_lock_) ACQUIRE(art::Roles::uninterruptible_) - : self_(self), deopt_(deopt), uninterruptible_cause_(nullptr) { + : self_(self), + deopt_(deopt), + critical_section_(self_, "JVMTI Deoptimizing methods"), + uninterruptible_cause_(nullptr) { deopt_->WaitForDeoptimizationToFinishLocked(self_); DCHECK(!deopt->performing_deoptimization_) << "Already performing deoptimization on another thread!"; // Use performing_deoptimization_ to keep track of the lock. deopt_->performing_deoptimization_ = true; deopt_->deoptimization_status_lock_.Unlock(self_); + uninterruptible_cause_ = critical_section_.Enter(art::gc::kGcCauseInstrumentation, + art::gc::kCollectorTypeCriticalSection); art::Runtime::Current()->GetThreadList()->SuspendAll("JMVTI Deoptimizing methods", /*long_suspend*/ false); - uninterruptible_cause_ = self_->StartAssertNoThreadSuspension("JVMTI deoptimizing methods"); } ~ScopedDeoptimizationContext() RELEASE(art::Locks::mutator_lock_) RELEASE(art::Roles::uninterruptible_) { // Can be suspended again. - self_->EndAssertNoThreadSuspension(uninterruptible_cause_); + critical_section_.Exit(uninterruptible_cause_); // Release the mutator lock. art::Runtime::Current()->GetThreadList()->ResumeAll(); // Let other threads know it's fine to proceed. @@ -300,6 +308,7 @@ class ScopedDeoptimizationContext : public art::ValueObject { private: art::Thread* self_; DeoptManager* deopt_; + art::gc::GCCriticalSection critical_section_; const char* uninterruptible_cause_; }; diff --git a/runtime/arch/arm64/quick_entrypoints_arm64.S b/runtime/arch/arm64/quick_entrypoints_arm64.S index 8b77453f30..40a8dbc008 100644 --- a/runtime/arch/arm64/quick_entrypoints_arm64.S +++ b/runtime/arch/arm64/quick_entrypoints_arm64.S @@ -2785,6 +2785,8 @@ READ_BARRIER_MARK_REG art_quick_read_barrier_mark_reg29, w29, x29 * the root register to IP0 and jumps to the customized entrypoint, * art_quick_read_barrier_mark_introspection_gc_roots. The thunk also * performs all the fast-path checks, so we need just the slow path. + * The UnsafeCASObject intrinsic is also using the GC root entrypoint with + * MOV instead of LDR, the destination register is in the same bits. * * The code structure is * art_quick_read_barrier_mark_introspection: diff --git a/runtime/gc/accounting/card_table.cc b/runtime/gc/accounting/card_table.cc index c7f936fb11..22104a30fe 100644 --- a/runtime/gc/accounting/card_table.cc +++ b/runtime/gc/accounting/card_table.cc @@ -41,10 +41,10 @@ constexpr uint8_t CardTable::kCardDirty; * non-null values to heap addresses should go through an entry in * WriteBarrier, and from there to here. * - * The heap is divided into "cards" of GC_CARD_SIZE bytes, as - * determined by GC_CARD_SHIFT. The card table contains one byte of + * The heap is divided into "cards" of `kCardSize` bytes, as + * determined by `kCardShift`. The card table contains one byte of * data per card, to be used by the GC. The value of the byte will be - * one of GC_CARD_CLEAN or GC_CARD_DIRTY. + * one of `kCardClean` or `kCardDirty`. * * After any store of a non-null object pointer into a heap object, * code is obliged to mark the card dirty. The setters in @@ -53,9 +53,9 @@ constexpr uint8_t CardTable::kCardDirty; * * The card table's base [the "biased card table"] gets set to a * rather strange value. In order to keep the JIT from having to - * fabricate or load GC_DIRTY_CARD to store into the card table, + * fabricate or load `kCardDirty` to store into the card table, * biased base is within the mmap allocation at a point where its low - * byte is equal to GC_DIRTY_CARD. See CardTable::Create for details. + * byte is equal to `kCardDirty`. See CardTable::Create for details. */ CardTable* CardTable::Create(const uint8_t* heap_begin, size_t heap_capacity) { @@ -75,8 +75,8 @@ CardTable* CardTable::Create(const uint8_t* heap_begin, size_t heap_capacity) { uint8_t* cardtable_begin = mem_map->Begin(); CHECK(cardtable_begin != nullptr); - // We allocated up to a bytes worth of extra space to allow biased_begin's byte value to equal - // kCardDirty, compute a offset value to make this the case + // We allocated up to a bytes worth of extra space to allow `biased_begin`'s byte value to equal + // `kCardDirty`, compute a offset value to make this the case size_t offset = 0; uint8_t* biased_begin = reinterpret_cast<uint8_t*>(reinterpret_cast<uintptr_t>(cardtable_begin) - (reinterpret_cast<uintptr_t>(heap_begin) >> kCardShift)); diff --git a/runtime/gc/accounting/card_table.h b/runtime/gc/accounting/card_table.h index f3548f7ce5..b8520b7dc0 100644 --- a/runtime/gc/accounting/card_table.h +++ b/runtime/gc/accounting/card_table.h @@ -56,7 +56,7 @@ class CardTable { static CardTable* Create(const uint8_t* heap_begin, size_t heap_capacity); ~CardTable(); - // Set the card associated with the given address to GC_CARD_DIRTY. + // Set the card associated with the given address to `kCardDirty`. ALWAYS_INLINE void MarkCard(const void *addr) { *CardFromAddr(addr) = kCardDirty; } @@ -84,8 +84,8 @@ class CardTable { } } - // Returns a value that when added to a heap address >> GC_CARD_SHIFT will address the appropriate - // card table byte. For convenience this value is cached in every Thread + // Returns a value that when added to a heap address >> `kCardShift` will address the appropriate + // card table byte. For convenience this value is cached in every Thread. uint8_t* GetBiasedBegin() const { return biased_begin_; } @@ -148,7 +148,7 @@ class CardTable { // Value used to compute card table addresses from object addresses, see GetBiasedBegin uint8_t* const biased_begin_; // Card table doesn't begin at the beginning of the mem_map_, instead it is displaced by offset - // to allow the byte value of biased_begin_ to equal GC_CARD_DIRTY + // to allow the byte value of `biased_begin_` to equal `kCardDirty`. const size_t offset_; DISALLOW_IMPLICIT_CONSTRUCTORS(CardTable); diff --git a/runtime/gc/collector/concurrent_copying-inl.h b/runtime/gc/collector/concurrent_copying-inl.h index 36fefbdbc3..cde5dc7d50 100644 --- a/runtime/gc/collector/concurrent_copying-inl.h +++ b/runtime/gc/collector/concurrent_copying-inl.h @@ -35,12 +35,13 @@ inline mirror::Object* ConcurrentCopying::MarkUnevacFromSpaceRegion( Thread* const self, mirror::Object* ref, accounting::ContinuousSpaceBitmap* bitmap) { - // For the Baker-style RB, in a rare case, we could incorrectly change the object from white - // to gray even though the object has already been marked through. This happens if a mutator - // thread gets preempted before the AtomicSetReadBarrierState below, GC marks through the - // object (changes it from white to gray and back to white), and the thread runs and - // incorrectly changes it from white to gray. If this happens, the object will get added to the - // mark stack again and get changed back to white after it is processed. + // For the Baker-style RB, in a rare case, we could incorrectly change the object from non-gray + // (black) to gray even though the object has already been marked through. This happens if a + // mutator thread gets preempted before the AtomicSetReadBarrierState below, GC marks through the + // object (changes it from non-gray (white) to gray and back to non-gray (black)), and the thread + // runs and incorrectly changes it from non-gray (black) to gray. If this happens, the object + // will get added to the mark stack again and get changed back to non-gray (black) after it is + // processed. if (kUseBakerReadBarrier) { // Test the bitmap first to avoid graying an object that has already been marked through most // of the time. @@ -55,7 +56,7 @@ inline mirror::Object* ConcurrentCopying::MarkUnevacFromSpaceRegion( // we can avoid an expensive CAS. // For the baker case, an object is marked if either the mark bit marked or the bitmap bit is // set. - success = ref->AtomicSetReadBarrierState(/* expected_rb_state */ ReadBarrier::WhiteState(), + success = ref->AtomicSetReadBarrierState(/* expected_rb_state */ ReadBarrier::NonGrayState(), /* rb_state */ ReadBarrier::GrayState()); } else { success = !bitmap->AtomicTestAndSet(ref); @@ -91,8 +92,9 @@ inline mirror::Object* ConcurrentCopying::MarkImmuneSpace(Thread* const self, return ref; } // This may or may not succeed, which is ok because the object may already be gray. - bool success = ref->AtomicSetReadBarrierState(/* expected_rb_state */ ReadBarrier::WhiteState(), - /* rb_state */ ReadBarrier::GrayState()); + bool success = + ref->AtomicSetReadBarrierState(/* expected_rb_state */ ReadBarrier::NonGrayState(), + /* rb_state */ ReadBarrier::GrayState()); if (success) { MutexLock mu(self, immune_gray_stack_lock_); immune_gray_stack_.push_back(ref); @@ -204,8 +206,8 @@ inline mirror::Object* ConcurrentCopying::GetFwdPtr(mirror::Object* from_ref) { } inline bool ConcurrentCopying::IsMarkedInUnevacFromSpace(mirror::Object* from_ref) { - // Use load acquire on the read barrier pointer to ensure that we never see a white read barrier - // state with an unmarked bit due to reordering. + // Use load-acquire on the read barrier pointer to ensure that we never see a black (non-gray) + // read barrier state with an unmarked bit due to reordering. DCHECK(region_space_->IsInUnevacFromSpace(from_ref)); if (kUseBakerReadBarrier && from_ref->GetReadBarrierStateAcquire() == ReadBarrier::GrayState()) { return true; diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc index b03f67152b..07abbfc684 100644 --- a/runtime/gc/collector/concurrent_copying.cc +++ b/runtime/gc/collector/concurrent_copying.cc @@ -646,10 +646,10 @@ class ConcurrentCopying::GrayImmuneObjectVisitor { explicit GrayImmuneObjectVisitor(Thread* self) : self_(self) {} ALWAYS_INLINE void operator()(mirror::Object* obj) const REQUIRES_SHARED(Locks::mutator_lock_) { - if (kUseBakerReadBarrier && obj->GetReadBarrierState() == ReadBarrier::WhiteState()) { + if (kUseBakerReadBarrier && obj->GetReadBarrierState() == ReadBarrier::NonGrayState()) { if (kConcurrent) { Locks::mutator_lock_->AssertSharedHeld(self_); - obj->AtomicSetReadBarrierState(ReadBarrier::WhiteState(), ReadBarrier::GrayState()); + obj->AtomicSetReadBarrierState(ReadBarrier::NonGrayState(), ReadBarrier::GrayState()); // Mod union table VisitObjects may visit the same object multiple times so we can't check // the result of the atomic set. } else { @@ -765,9 +765,9 @@ class ConcurrentCopying::ImmuneSpaceScanObjVisitor { // Only need to scan gray objects. if (obj->GetReadBarrierState() == ReadBarrier::GrayState()) { collector_->ScanImmuneObject(obj); - // Done scanning the object, go back to white. + // Done scanning the object, go back to black (non-gray). bool success = obj->AtomicSetReadBarrierState(ReadBarrier::GrayState(), - ReadBarrier::WhiteState()); + ReadBarrier::NonGrayState()); CHECK(success) << Runtime::Current()->GetHeap()->GetVerification()->DumpObjectInfo(obj, "failed CAS"); } @@ -824,21 +824,23 @@ void ConcurrentCopying::MarkingPhase() { // This release fence makes the field updates in the above loop visible before allowing mutator // getting access to immune objects without graying it first. updated_all_immune_objects_.store(true, std::memory_order_release); - // Now whiten immune objects concurrently accessed and grayed by mutators. We can't do this in - // the above loop because we would incorrectly disable the read barrier by whitening an object - // which may point to an unscanned, white object, breaking the to-space invariant. + // Now "un-gray" (conceptually blacken) immune objects concurrently accessed and grayed by + // mutators. We can't do this in the above loop because we would incorrectly disable the read + // barrier by un-graying (conceptually blackening) an object which may point to an unscanned, + // white object, breaking the to-space invariant (a mutator shall never observe a from-space + // (white) object). // - // Make sure no mutators are in the middle of marking an immune object before whitening immune - // objects. + // Make sure no mutators are in the middle of marking an immune object before un-graying + // (blackening) immune objects. IssueEmptyCheckpoint(); MutexLock mu(Thread::Current(), immune_gray_stack_lock_); if (kVerboseMode) { LOG(INFO) << "immune gray stack size=" << immune_gray_stack_.size(); } for (mirror::Object* obj : immune_gray_stack_) { - DCHECK(obj->GetReadBarrierState() == ReadBarrier::GrayState()); + DCHECK_EQ(obj->GetReadBarrierState(), ReadBarrier::GrayState()); bool success = obj->AtomicSetReadBarrierState(ReadBarrier::GrayState(), - ReadBarrier::WhiteState()); + ReadBarrier::NonGrayState()); DCHECK(success); } immune_gray_stack_.clear(); @@ -1038,16 +1040,17 @@ void ConcurrentCopying::PushOntoFalseGrayStack(Thread* const self, mirror::Objec void ConcurrentCopying::ProcessFalseGrayStack() { CHECK(kUseBakerReadBarrier); - // Change the objects on the false gray stack from gray to white. + // Change the objects on the false gray stack from gray to non-gray (conceptually black). MutexLock mu(Thread::Current(), mark_stack_lock_); for (mirror::Object* obj : false_gray_stack_) { DCHECK(IsMarked(obj)); - // The object could be white here if a thread got preempted after a success at the - // AtomicSetReadBarrierState in Mark(), GC started marking through it (but not finished so - // still gray), and the thread ran to register it onto the false gray stack. + // The object could be non-gray (conceptually black) here if a thread got preempted after a + // success at the AtomicSetReadBarrierState in MarkNonMoving(), GC started marking through it + // (but not finished so still gray), the thread ran to register it onto the false gray stack, + // and then GC eventually marked it black (non-gray) after it finished scanning it. if (obj->GetReadBarrierState() == ReadBarrier::GrayState()) { bool success = obj->AtomicSetReadBarrierState(ReadBarrier::GrayState(), - ReadBarrier::WhiteState()); + ReadBarrier::NonGrayState()); DCHECK(success); } } @@ -1166,9 +1169,8 @@ class ConcurrentCopying::VerifyNoFromSpaceRefsVisitor : public SingleRootVisitor } collector_->AssertToSpaceInvariant(holder, offset, ref); if (kUseBakerReadBarrier) { - CHECK_EQ(ref->GetReadBarrierState(), ReadBarrier::WhiteState()) - << "Ref " << ref << " " << ref->PrettyTypeOf() - << " has non-white rb_state "; + CHECK_EQ(ref->GetReadBarrierState(), ReadBarrier::NonGrayState()) + << "Ref " << ref << " " << ref->PrettyTypeOf() << " has gray rb_state"; } } @@ -1243,8 +1245,8 @@ void ConcurrentCopying::VerifyNoFromSpaceReferences() { visitor, visitor); if (kUseBakerReadBarrier) { - CHECK_EQ(obj->GetReadBarrierState(), ReadBarrier::WhiteState()) - << "obj=" << obj << " non-white rb_state " << obj->GetReadBarrierState(); + CHECK_EQ(obj->GetReadBarrierState(), ReadBarrier::NonGrayState()) + << "obj=" << obj << " has gray rb_state " << obj->GetReadBarrierState(); } }; // Roots. @@ -1542,18 +1544,18 @@ inline void ConcurrentCopying::ProcessMarkStackRef(mirror::Object* to_ref) { (referent = to_ref->AsReference()->GetReferent<kWithoutReadBarrier>()) != nullptr && !IsInToSpace(referent)))) { // Leave this reference gray in the queue so that GetReferent() will trigger a read barrier. We - // will change it to white later in ReferenceQueue::DequeuePendingReference(). + // will change it to non-gray later in ReferenceQueue::DisableReadBarrierForReference. DCHECK(to_ref->AsReference()->GetPendingNext() != nullptr) << "Left unenqueued ref gray " << to_ref; } else { - // We may occasionally leave a reference white in the queue if its referent happens to be + // We may occasionally leave a reference non-gray in the queue if its referent happens to be // concurrently marked after the Scan() call above has enqueued the Reference, in which case the - // above IsInToSpace() evaluates to true and we change the color from gray to white here in this - // else block. + // above IsInToSpace() evaluates to true and we change the color from gray to non-gray here in + // this else block. if (kUseBakerReadBarrier) { bool success = to_ref->AtomicSetReadBarrierState<std::memory_order_release>( ReadBarrier::GrayState(), - ReadBarrier::WhiteState()); + ReadBarrier::NonGrayState()); DCHECK(success) << "Must succeed as we won the race."; } } @@ -2617,7 +2619,7 @@ mirror::Object* ConcurrentCopying::MarkNonMoving(Thread* const self, } else { // Not marked. if (IsOnAllocStack(ref)) { - // If it's on the allocation stack, it's considered marked. Keep it white. + // If it's on the allocation stack, it's considered marked. Keep it white (non-gray). // Objects on the allocation stack need not be marked. if (!is_los) { DCHECK(!mark_bitmap->Test(ref)); @@ -2625,7 +2627,7 @@ mirror::Object* ConcurrentCopying::MarkNonMoving(Thread* const self, DCHECK(!los_bitmap->Test(ref)); } if (kUseBakerReadBarrier) { - DCHECK_EQ(ref->GetReadBarrierState(), ReadBarrier::WhiteState()); + DCHECK_EQ(ref->GetReadBarrierState(), ReadBarrier::NonGrayState()); } } else { // For the baker-style RB, we need to handle 'false-gray' cases. See the @@ -2642,22 +2644,24 @@ mirror::Object* ConcurrentCopying::MarkNonMoving(Thread* const self, // AtomicSetReadBarrierState since it will fault if the address is not valid. heap_->GetVerification()->LogHeapCorruption(holder, offset, ref, /* fatal */ true); } - // Not marked or on the allocation stack. Try to mark it. + // Not marked nor on the allocation stack. Try to mark it. // This may or may not succeed, which is ok. bool cas_success = false; if (kUseBakerReadBarrier) { - cas_success = ref->AtomicSetReadBarrierState(ReadBarrier::WhiteState(), + cas_success = ref->AtomicSetReadBarrierState(ReadBarrier::NonGrayState(), ReadBarrier::GrayState()); } if (!is_los && mark_bitmap->AtomicTestAndSet(ref)) { // Already marked. - if (kUseBakerReadBarrier && cas_success && + if (kUseBakerReadBarrier && + cas_success && ref->GetReadBarrierState() == ReadBarrier::GrayState()) { PushOntoFalseGrayStack(self, ref); } } else if (is_los && los_bitmap->AtomicTestAndSet(ref)) { // Already marked in LOS. - if (kUseBakerReadBarrier && cas_success && + if (kUseBakerReadBarrier && + cas_success && ref->GetReadBarrierState() == ReadBarrier::GrayState()) { PushOntoFalseGrayStack(self, ref); } diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h index d01437299f..5c34c56e09 100644 --- a/runtime/gc/heap.h +++ b/runtime/gc/heap.h @@ -1425,6 +1425,7 @@ class Heap { friend class collector::ConcurrentCopying; friend class collector::MarkSweep; friend class collector::SemiSpace; + friend class GCCriticalSection; friend class ReferenceQueue; friend class ScopedGCCriticalSection; friend class VerifyReferenceCardVisitor; diff --git a/runtime/gc/reference_queue.cc b/runtime/gc/reference_queue.cc index 321d22a592..e25e279ea6 100644 --- a/runtime/gc/reference_queue.cc +++ b/runtime/gc/reference_queue.cc @@ -76,19 +76,19 @@ void ReferenceQueue::DisableReadBarrierForReference(ObjPtr<mirror::Reference> re Heap* heap = Runtime::Current()->GetHeap(); if (kUseBakerOrBrooksReadBarrier && heap->CurrentCollectorType() == kCollectorTypeCC && heap->ConcurrentCopyingCollector()->IsActive()) { - // Change the gray ptr we left in ConcurrentCopying::ProcessMarkStackRef() to white. + // Change the gray ptr we left in ConcurrentCopying::ProcessMarkStackRef() to non-gray. // We check IsActive() above because we don't want to do this when the zygote compaction // collector (SemiSpace) is running. CHECK(ref != nullptr); collector::ConcurrentCopying* concurrent_copying = heap->ConcurrentCopyingCollector(); uint32_t rb_state = ref->GetReadBarrierState(); if (rb_state == ReadBarrier::GrayState()) { - ref->AtomicSetReadBarrierState(ReadBarrier::GrayState(), ReadBarrier::WhiteState()); - CHECK_EQ(ref->GetReadBarrierState(), ReadBarrier::WhiteState()); + ref->AtomicSetReadBarrierState(ReadBarrier::GrayState(), ReadBarrier::NonGrayState()); + CHECK_EQ(ref->GetReadBarrierState(), ReadBarrier::NonGrayState()); } else { - // In ConcurrentCopying::ProcessMarkStackRef() we may leave a white reference in the queue and - // find it here, which is OK. - CHECK_EQ(rb_state, ReadBarrier::WhiteState()) << "ref=" << ref << " rb_state=" << rb_state; + // In ConcurrentCopying::ProcessMarkStackRef() we may leave a non-gray reference in the queue + // and find it here, which is OK. + CHECK_EQ(rb_state, ReadBarrier::NonGrayState()) << "ref=" << ref << " rb_state=" << rb_state; ObjPtr<mirror::Object> referent = ref->GetReferent<kWithoutReadBarrier>(); // The referent could be null if it's cleared by a mutator (Reference.clear()). if (referent != nullptr) { diff --git a/runtime/gc/scoped_gc_critical_section.cc b/runtime/gc/scoped_gc_critical_section.cc index 2976dd0252..7a0a6e8736 100644 --- a/runtime/gc/scoped_gc_critical_section.cc +++ b/runtime/gc/scoped_gc_critical_section.cc @@ -24,21 +24,39 @@ namespace art { namespace gc { -ScopedGCCriticalSection::ScopedGCCriticalSection(Thread* self, - GcCause cause, - CollectorType collector_type) - : self_(self) { - Runtime::Current()->GetHeap()->StartGC(self, cause, collector_type); - if (self != nullptr) { - old_cause_ = self->StartAssertNoThreadSuspension("ScopedGCCriticalSection"); +const char* GCCriticalSection::Enter(GcCause cause, CollectorType type) { + Runtime::Current()->GetHeap()->StartGC(self_, cause, type); + if (self_ != nullptr) { + return self_->StartAssertNoThreadSuspension(section_name_); + } else { + // Workaround to avoid having to mark the whole function as NO_THREAD_SAFETY_ANALYSIS. + auto kludge = []() ACQUIRE(Roles::uninterruptible_) NO_THREAD_SAFETY_ANALYSIS {}; + kludge(); + return nullptr; } } -ScopedGCCriticalSection::~ScopedGCCriticalSection() { + +void GCCriticalSection::Exit(const char* old_cause) { if (self_ != nullptr) { - self_->EndAssertNoThreadSuspension(old_cause_); + self_->EndAssertNoThreadSuspension(old_cause); + } else { + // Workaround to avoid having to mark the whole function as NO_THREAD_SAFETY_ANALYSIS. + auto kludge = []() RELEASE(Roles::uninterruptible_) NO_THREAD_SAFETY_ANALYSIS {}; + kludge(); } Runtime::Current()->GetHeap()->FinishGC(self_, collector::kGcTypeNone); } +ScopedGCCriticalSection::ScopedGCCriticalSection(Thread* self, + GcCause cause, + CollectorType collector_type) + : critical_section_(self, "ScopedGCCriticalSection") { + old_no_suspend_reason_ = critical_section_.Enter(cause, collector_type); +} + +ScopedGCCriticalSection::~ScopedGCCriticalSection() { + critical_section_.Exit(old_no_suspend_reason_); +} + } // namespace gc } // namespace art diff --git a/runtime/gc/scoped_gc_critical_section.h b/runtime/gc/scoped_gc_critical_section.h index 1271ff7af3..864bf878f6 100644 --- a/runtime/gc/scoped_gc_critical_section.h +++ b/runtime/gc/scoped_gc_critical_section.h @@ -27,6 +27,24 @@ class Thread; namespace gc { +// The use of ScopedGCCriticalSection should be preferred whenever possible. +class GCCriticalSection { + public: + GCCriticalSection(Thread* self, const char* name) + : self_(self), section_name_(name) {} + ~GCCriticalSection() {} + + // Starts a GCCriticalSection. Returns the previous no-suspension reason. + const char* Enter(GcCause cause, CollectorType type) ACQUIRE(Roles::uninterruptible_); + + // Ends a GCCriticalSection. Takes the old no-suspension reason. + void Exit(const char* old_reason) RELEASE(Roles::uninterruptible_); + + private: + Thread* const self_; + const char* section_name_; +}; + // Wait until the GC is finished and then prevent the GC from starting until the destructor. Used // to prevent deadlocks in places where we call ClassLinker::VisitClass with all the threads // suspended. @@ -37,8 +55,8 @@ class ScopedGCCriticalSection { ~ScopedGCCriticalSection() RELEASE(Roles::uninterruptible_); private: - Thread* const self_; - const char* old_cause_; + GCCriticalSection critical_section_; + const char* old_no_suspend_reason_; }; } // namespace gc diff --git a/runtime/lock_word.h b/runtime/lock_word.h index ce7fe34b22..ac7890c12f 100644 --- a/runtime/lock_word.h +++ b/runtime/lock_word.h @@ -210,7 +210,7 @@ class LockWord { void SetReadBarrierState(uint32_t rb_state) { DCHECK_EQ(rb_state & ~kReadBarrierStateMask, 0U); - DCHECK(rb_state == ReadBarrier::WhiteState() || + DCHECK(rb_state == ReadBarrier::NonGrayState() || rb_state == ReadBarrier::GrayState()) << rb_state; DCHECK_NE(static_cast<uint32_t>(GetState()), static_cast<uint32_t>(kForwardingAddress)); // Clear and or the bits. @@ -288,7 +288,7 @@ class LockWord { if (!kUseReadBarrier) { DCHECK_EQ(rb_state, 0U); } else { - DCHECK(rb_state == ReadBarrier::WhiteState() || + DCHECK(rb_state == ReadBarrier::NonGrayState() || rb_state == ReadBarrier::GrayState()) << rb_state; } } diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h index 47f0a298de..bd89907ae2 100644 --- a/runtime/mirror/object-inl.h +++ b/runtime/mirror/object-inl.h @@ -119,7 +119,7 @@ inline void Object::SetReadBarrierState(uint32_t rb_state) { inline void Object::AssertReadBarrierState() const { CHECK(kUseBakerReadBarrier); Object* obj = const_cast<Object*>(this); - DCHECK_EQ(obj->GetReadBarrierState(), ReadBarrier::WhiteState()) + DCHECK_EQ(obj->GetReadBarrierState(), ReadBarrier::NonGrayState()) << "Bad Baker pointer: obj=" << obj << " rb_state" << obj->GetReadBarrierState(); } diff --git a/runtime/mirror/object-readbarrier-inl.h b/runtime/mirror/object-readbarrier-inl.h index df50d0613a..cc375bd796 100644 --- a/runtime/mirror/object-readbarrier-inl.h +++ b/runtime/mirror/object-readbarrier-inl.h @@ -168,10 +168,11 @@ inline bool Object::AtomicSetReadBarrierState(uint32_t expected_rb_state, uint32 expected_lw.SetReadBarrierState(expected_rb_state); new_lw = lw; new_lw.SetReadBarrierState(rb_state); - // ConcurrentCopying::ProcessMarkStackRef uses this with kCasRelease == true. - // If kCasRelease == true, use a CAS release so that when GC updates all the fields of - // an object and then changes the object from gray to black, the field updates (stores) will be - // visible (won't be reordered after this CAS.) + // ConcurrentCopying::ProcessMarkStackRef uses this with + // `kMemoryOrder` == `std::memory_order_release`. + // If `kMemoryOrder` == `std::memory_order_release`, use a CAS release so that when GC updates + // all the fields of an object and then changes the object from gray to black (non-gray), the + // field updates (stores) will be visible (won't be reordered after this CAS.) } while (!CasLockWord(expected_lw, new_lw, CASMode::kWeak, kMemoryOrder)); return true; } diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h index 2801928240..47aded3f0c 100644 --- a/runtime/mirror/object.h +++ b/runtime/mirror/object.h @@ -117,7 +117,7 @@ class MANAGED LOCKABLE Object { ALWAYS_INLINE bool AtomicSetMarkBit(uint32_t expected_mark_bit, uint32_t mark_bit) REQUIRES_SHARED(Locks::mutator_lock_); - // Assert that the read barrier state is in the default (white) state. + // Assert that the read barrier state is in the default (white, i.e. non-gray) state. ALWAYS_INLINE void AssertReadBarrierState() const REQUIRES_SHARED(Locks::mutator_lock_); // The verifier treats all interfaces as java.lang.Object and relies on runtime checks in diff --git a/runtime/read_barrier-inl.h b/runtime/read_barrier-inl.h index 640fa7e393..672303a51b 100644 --- a/runtime/read_barrier-inl.h +++ b/runtime/read_barrier-inl.h @@ -255,13 +255,13 @@ inline mirror::Object* ReadBarrier::Mark(mirror::Object* obj) { } inline bool ReadBarrier::IsGray(mirror::Object* obj, uintptr_t* fake_address_dependency) { - return obj->GetReadBarrierState(fake_address_dependency) == gray_state_; + return obj->GetReadBarrierState(fake_address_dependency) == kGrayState; } inline bool ReadBarrier::IsGray(mirror::Object* obj) { // Use a load-acquire to load the read barrier bit to avoid reordering with the subsequent load. // GetReadBarrierStateAcquire() has load-acquire semantics. - return obj->GetReadBarrierStateAcquire() == gray_state_; + return obj->GetReadBarrierStateAcquire() == kGrayState; } } // namespace art diff --git a/runtime/read_barrier.h b/runtime/read_barrier.h index e8df2ad4ce..0741da663b 100644 --- a/runtime/read_barrier.h +++ b/runtime/read_barrier.h @@ -102,11 +102,11 @@ class ReadBarrier { // ALWAYS_INLINE on this caused a performance regression b/26744236. static mirror::Object* Mark(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_); - static constexpr uint32_t WhiteState() { - return white_state_; + static constexpr uint32_t NonGrayState() { + return kNonGrayState; } static constexpr uint32_t GrayState() { - return gray_state_; + return kGrayState; } // fake_address_dependency will be zero which should be bitwise-or'ed with the address of the @@ -122,12 +122,13 @@ class ReadBarrier { REQUIRES_SHARED(Locks::mutator_lock_); static bool IsValidReadBarrierState(uint32_t rb_state) { - return rb_state == white_state_ || rb_state == gray_state_; + return rb_state == kNonGrayState || rb_state == kGrayState; } - static constexpr uint32_t white_state_ = 0x0; // Not marked. - static constexpr uint32_t gray_state_ = 0x1; // Marked, but not marked through. On mark stack. - static constexpr uint32_t rb_state_mask_ = 0x1; // The low bits for white|gray. + private: + static constexpr uint32_t kNonGrayState = 0x0; // White (not marked) or black (marked through). + static constexpr uint32_t kGrayState = 0x1; // Marked, but not marked through. On mark stack. + static constexpr uint32_t kRBStateMask = 0x1; // The low bits for non-gray|gray. }; } // namespace art |