Do type checks in ArraySet without read barriers.
This approach is valid in the case of Baker and non-Baker
read barriers.
Benchmarks (ARM64) score variations on Nexus 5X with CPU
cores clamped at 960000 Hz (aosp_bullhead-userdebug build,
medians of 10 runs for each suite):
- Ritzperf - average (lower is better): -0.44% (virtually unchanged)
- CaffeineMark - average (higher is better): -0.20% (virtually unchanged)
- DeltaBlue (lower is better): -4.08% (slightly better)
- Richards - average (lower is better): -0.57% (virtually unchanged)
- SciMark2 - average (higher is better): -0.52% (virtually unchanged)
Details about Ritzperf benchmarks with meaningful variations
(lower is better):
- GenericCalcActions.MemAllocTest: +3.02% (slightly worse)
Details about Richards benchmarks with meaningful variations
(lower is better):
- gibbons -5.01% (better)
Boot image code size variation on Nexus 5X
(aosp_bullhead-userdebug build):
- total ARM64 framework Oat files size change:
83127840 bytes -> 83082656 bytes (-45184 bytes, -0.05%)
- total ARM framework Oat files size change:
72571872 bytes -> 72522796 bytes (-49076 bytes, -0.07%)
Test: ART_USE_READ_BARRIER=true ART_HEAP_POISONING=true m test-art-host
Test: ART_USE_READ_BARRIER=true ART_HEAP_POISONING=true m test-art-target
Bug: 29516974
Bug: 12687968
Change-Id: I8fe130156ace87dd2e4a15d9f8b4111287e735b3
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index d8866a9..b9f2056 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -4619,7 +4619,6 @@
}
if (needs_write_barrier) {
// Temporary registers for the write barrier.
- // These registers may be used for Baker read barriers too.
locations->AddTemp(Location::RequiresRegister()); // Possibly used for ref. poisoning too.
locations->AddTemp(Location::RequiresRegister());
}
@@ -4737,127 +4736,42 @@
__ Bind(&non_zero);
}
- if (kEmitCompilerReadBarrier) {
- if (!kUseBakerReadBarrier) {
- // When (non-Baker) read barriers are enabled, the type
- // checking instrumentation requires two read barriers
- // generated by CodeGeneratorARM::GenerateReadBarrierSlow:
- //
- // __ Mov(temp2, temp1);
- // // /* HeapReference<Class> */ temp1 = temp1->component_type_
- // __ LoadFromOffset(kLoadWord, temp1, temp1, component_offset);
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp1_loc, temp1_loc, temp2_loc, component_offset);
- //
- // // /* HeapReference<Class> */ temp2 = value->klass_
- // __ LoadFromOffset(kLoadWord, temp2, value, class_offset);
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp2_loc, temp2_loc, value_loc, class_offset, temp1_loc);
- //
- // __ cmp(temp1, ShifterOperand(temp2));
- //
- // However, the second read barrier may trash `temp`, as it
- // is a temporary register, and as such would not be saved
- // along with live registers before calling the runtime (nor
- // restored afterwards). So in this case, we bail out and
- // delegate the work to the array set slow path.
- //
- // TODO: Extend the register allocator to support a new
- // "(locally) live temp" location so as to avoid always
- // going into the slow path when read barriers are enabled?
- //
- // There is no such problem with Baker read barriers (see below).
- __ b(slow_path->GetEntryLabel());
- } else {
- Register temp3 = IP;
- Location temp3_loc = Location::RegisterLocation(temp3);
+ // Note that when read barriers are enabled, the type checks
+ // are performed without read barriers. This is fine, even in
+ // the case where a class object is in the from-space after
+ // the flip, as a comparison involving such a type would not
+ // produce a false positive; it may of course produce a false
+ // negative, in which case we would take the ArraySet slow
+ // path.
- // Note: `temp3` (scratch register IP) cannot be used as
- // `ref` argument of GenerateFieldLoadWithBakerReadBarrier
- // calls below (see ReadBarrierMarkSlowPathARM for more
- // details).
+ // /* HeapReference<Class> */ temp1 = array->klass_
+ __ LoadFromOffset(kLoadWord, temp1, array, class_offset);
+ codegen_->MaybeRecordImplicitNullCheck(instruction);
+ __ MaybeUnpoisonHeapReference(temp1);
- // /* HeapReference<Class> */ temp1 = array->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(instruction,
- temp1_loc,
- array,
- class_offset,
- temp3_loc,
- /* needs_null_check */ true);
+ // /* HeapReference<Class> */ temp1 = temp1->component_type_
+ __ LoadFromOffset(kLoadWord, temp1, temp1, component_offset);
+ // /* HeapReference<Class> */ temp2 = value->klass_
+ __ LoadFromOffset(kLoadWord, temp2, value, class_offset);
+ // If heap poisoning is enabled, no need to unpoison `temp1`
+ // nor `temp2`, as we are comparing two poisoned references.
+ __ cmp(temp1, ShifterOperand(temp2));
- // /* HeapReference<Class> */ temp1 = temp1->component_type_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(instruction,
- temp1_loc,
- temp1,
- component_offset,
- temp3_loc,
- /* needs_null_check */ false);
- // Register `temp1` is not trashed by the read barrier
- // emitted by GenerateFieldLoadWithBakerReadBarrier below,
- // as that method produces a call to a ReadBarrierMarkRegX
- // entry point, which saves all potentially live registers,
- // including temporaries such a `temp1`.
- // /* HeapReference<Class> */ temp2 = value->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(instruction,
- temp2_loc,
- value,
- class_offset,
- temp3_loc,
- /* needs_null_check */ false);
- // If heap poisoning is enabled, `temp1` and `temp2` have
- // been unpoisoned by the the previous calls to
- // CodeGeneratorARM::GenerateFieldLoadWithBakerReadBarrier.
- __ cmp(temp1, ShifterOperand(temp2));
-
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- Label do_put;
- __ b(&do_put, EQ);
- // We do not need to emit a read barrier for the
- // following heap reference load, as `temp1` is only used
- // in a comparison with null below, and this reference
- // is not kept afterwards.
- // /* HeapReference<Class> */ temp1 = temp1->super_class_
- __ LoadFromOffset(kLoadWord, temp1, temp1, super_offset);
- // If heap poisoning is enabled, no need to unpoison
- // `temp`, as we are comparing against null below.
- __ CompareAndBranchIfNonZero(temp1, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ b(slow_path->GetEntryLabel(), NE);
- }
- }
- } else {
- // Non read barrier code.
-
- // /* HeapReference<Class> */ temp1 = array->klass_
- __ LoadFromOffset(kLoadWord, temp1, array, class_offset);
- codegen_->MaybeRecordImplicitNullCheck(instruction);
+ if (instruction->StaticTypeOfArrayIsObjectArray()) {
+ Label do_put;
+ __ b(&do_put, EQ);
+ // If heap poisoning is enabled, the `temp1` reference has
+ // not been unpoisoned yet; unpoison it now.
__ MaybeUnpoisonHeapReference(temp1);
- // /* HeapReference<Class> */ temp1 = temp1->component_type_
- __ LoadFromOffset(kLoadWord, temp1, temp1, component_offset);
- // /* HeapReference<Class> */ temp2 = value->klass_
- __ LoadFromOffset(kLoadWord, temp2, value, class_offset);
- // If heap poisoning is enabled, no need to unpoison `temp1`
- // nor `temp2`, as we are comparing two poisoned references.
- __ cmp(temp1, ShifterOperand(temp2));
-
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- Label do_put;
- __ b(&do_put, EQ);
- // If heap poisoning is enabled, the `temp1` reference has
- // not been unpoisoned yet; unpoison it now.
- __ MaybeUnpoisonHeapReference(temp1);
-
- // /* HeapReference<Class> */ temp1 = temp1->super_class_
- __ LoadFromOffset(kLoadWord, temp1, temp1, super_offset);
- // If heap poisoning is enabled, no need to unpoison
- // `temp1`, as we are comparing against null below.
- __ CompareAndBranchIfNonZero(temp1, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ b(slow_path->GetEntryLabel(), NE);
- }
+ // /* HeapReference<Class> */ temp1 = temp1->super_class_
+ __ LoadFromOffset(kLoadWord, temp1, temp1, super_offset);
+ // If heap poisoning is enabled, no need to unpoison
+ // `temp1`, as we are comparing against null below.
+ __ CompareAndBranchIfNonZero(temp1, slow_path->GetEntryLabel());
+ __ Bind(&do_put);
+ } else {
+ __ b(slow_path->GetEntryLabel(), NE);
}
}
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 17fc13c..426a39a 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -2171,11 +2171,6 @@
} else {
locations->SetInAt(2, Location::RequiresRegister());
}
- if (kEmitCompilerReadBarrier && kUseBakerReadBarrier && (value_type == Primitive::kPrimNot)) {
- // Additional temporary registers for a Baker read barrier.
- locations->AddTemp(Location::RequiresRegister());
- locations->AddTemp(Location::RequiresRegister());
- }
}
void InstructionCodeGeneratorARM64::VisitArraySet(HArraySet* instruction) {
@@ -2262,144 +2257,44 @@
__ Bind(&non_zero);
}
- if (kEmitCompilerReadBarrier) {
- if (!kUseBakerReadBarrier) {
- // When (non-Baker) read barriers are enabled, the type
- // checking instrumentation requires two read barriers
- // generated by CodeGeneratorARM64::GenerateReadBarrierSlow:
- //
- // __ Mov(temp2, temp);
- // // /* HeapReference<Class> */ temp = temp->component_type_
- // __ Ldr(temp, HeapOperand(temp, component_offset));
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp_loc, temp_loc, temp2_loc, component_offset);
- //
- // // /* HeapReference<Class> */ temp2 = value->klass_
- // __ Ldr(temp2, HeapOperand(Register(value), class_offset));
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp2_loc, temp2_loc, value_loc, class_offset, temp_loc);
- //
- // __ Cmp(temp, temp2);
- //
- // However, the second read barrier may trash `temp`, as it
- // is a temporary register, and as such would not be saved
- // along with live registers before calling the runtime (nor
- // restored afterwards). So in this case, we bail out and
- // delegate the work to the array set slow path.
- //
- // TODO: Extend the register allocator to support a new
- // "(locally) live temp" location so as to avoid always
- // going into the slow path when read barriers are enabled?
- //
- // There is no such problem with Baker read barriers (see below).
- __ B(slow_path->GetEntryLabel());
- } else {
- // Note that we cannot use `temps` (instance of VIXL's
- // UseScratchRegisterScope) to allocate `temp2` because
- // the Baker read barriers generated by
- // GenerateFieldLoadWithBakerReadBarrier below also use
- // that facility to allocate a temporary register, thus
- // making VIXL's scratch register pool empty.
- Location temp2_loc = locations->GetTemp(0);
- Register temp2 = WRegisterFrom(temp2_loc);
+ // Note that when Baker read barriers are enabled, the type
+ // checks are performed without read barriers. This is fine,
+ // even in the case where a class object is in the from-space
+ // after the flip, as a comparison involving such a type would
+ // not produce a false positive; it may of course produce a
+ // false negative, in which case we would take the ArraySet
+ // slow path.
- // Note: Because it is acquired from VIXL's scratch register
- // pool, `temp` might be IP0, and thus cannot be used as
- // `ref` argument of GenerateFieldLoadWithBakerReadBarrier
- // calls below (see ReadBarrierMarkSlowPathARM64 for more
- // details).
+ Register temp2 = temps.AcquireSameSizeAs(array);
+ // /* HeapReference<Class> */ temp = array->klass_
+ __ Ldr(temp, HeapOperand(array, class_offset));
+ codegen_->MaybeRecordImplicitNullCheck(instruction);
+ GetAssembler()->MaybeUnpoisonHeapReference(temp);
- // /* HeapReference<Class> */ temp2 = array->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(instruction,
- temp2_loc,
- array,
- class_offset,
- temp,
- /* needs_null_check */ true,
- /* use_load_acquire */ false);
+ // /* HeapReference<Class> */ temp = temp->component_type_
+ __ Ldr(temp, HeapOperand(temp, component_offset));
+ // /* HeapReference<Class> */ temp2 = value->klass_
+ __ Ldr(temp2, HeapOperand(Register(value), class_offset));
+ // If heap poisoning is enabled, no need to unpoison `temp`
+ // nor `temp2`, as we are comparing two poisoned references.
+ __ Cmp(temp, temp2);
+ temps.Release(temp2);
- // /* HeapReference<Class> */ temp2 = temp2->component_type_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(instruction,
- temp2_loc,
- temp2,
- component_offset,
- temp,
- /* needs_null_check */ false,
- /* use_load_acquire */ false);
- // For the same reason that we request `temp2` from the
- // register allocator above, we cannot get `temp3` from
- // VIXL's scratch register pool.
- Location temp3_loc = locations->GetTemp(1);
- Register temp3 = WRegisterFrom(temp3_loc);
- // Register `temp2` is not trashed by the read barrier
- // emitted by GenerateFieldLoadWithBakerReadBarrier below,
- // as that method produces a call to a ReadBarrierMarkRegX
- // entry point, which saves all potentially live registers,
- // including temporaries such a `temp2`.
- // /* HeapReference<Class> */ temp3 = register_value->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(instruction,
- temp3_loc,
- value.W(),
- class_offset,
- temp,
- /* needs_null_check */ false,
- /* use_load_acquire */ false);
- // If heap poisoning is enabled, `temp2` and `temp3` have
- // been unpoisoned by the the previous calls to
- // CodeGeneratorARM64::GenerateFieldLoadWithBakerReadBarrier.
- __ Cmp(temp2, temp3);
-
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- vixl::aarch64::Label do_put;
- __ B(eq, &do_put);
- // We do not need to emit a read barrier for the
- // following heap reference load, as `temp2` is only used
- // in a comparison with null below, and this reference
- // is not kept afterwards.
- // /* HeapReference<Class> */ temp = temp2->super_class_
- __ Ldr(temp, HeapOperand(temp2, super_offset));
- // If heap poisoning is enabled, no need to unpoison
- // `temp`, as we are comparing against null below.
- __ Cbnz(temp, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ B(ne, slow_path->GetEntryLabel());
- }
- }
- } else {
- // Non read barrier code.
-
- Register temp2 = temps.AcquireSameSizeAs(array);
- // /* HeapReference<Class> */ temp = array->klass_
- __ Ldr(temp, HeapOperand(array, class_offset));
- codegen_->MaybeRecordImplicitNullCheck(instruction);
+ if (instruction->StaticTypeOfArrayIsObjectArray()) {
+ vixl::aarch64::Label do_put;
+ __ B(eq, &do_put);
+ // If heap poisoning is enabled, the `temp` reference has
+ // not been unpoisoned yet; unpoison it now.
GetAssembler()->MaybeUnpoisonHeapReference(temp);
- // /* HeapReference<Class> */ temp = temp->component_type_
- __ Ldr(temp, HeapOperand(temp, component_offset));
- // /* HeapReference<Class> */ temp2 = value->klass_
- __ Ldr(temp2, HeapOperand(Register(value), class_offset));
- // If heap poisoning is enabled, no need to unpoison `temp`
- // nor `temp2`, as we are comparing two poisoned references.
- __ Cmp(temp, temp2);
- temps.Release(temp2);
-
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- vixl::aarch64::Label do_put;
- __ B(eq, &do_put);
- // If heap poisoning is enabled, the `temp` reference has
- // not been unpoisoned yet; unpoison it now.
- GetAssembler()->MaybeUnpoisonHeapReference(temp);
-
- // /* HeapReference<Class> */ temp = temp->super_class_
- __ Ldr(temp, HeapOperand(temp, super_offset));
- // If heap poisoning is enabled, no need to unpoison
- // `temp`, as we are comparing against null below.
- __ Cbnz(temp, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ B(ne, slow_path->GetEntryLabel());
- }
+ // /* HeapReference<Class> */ temp = temp->super_class_
+ __ Ldr(temp, HeapOperand(temp, super_offset));
+ // If heap poisoning is enabled, no need to unpoison
+ // `temp`, as we are comparing against null below.
+ __ Cbnz(temp, slow_path->GetEntryLabel());
+ __ Bind(&do_put);
+ } else {
+ __ B(ne, slow_path->GetEntryLabel());
}
}
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 675c5e0..1e44eef 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -5278,7 +5278,6 @@
}
if (needs_write_barrier) {
// Temporary registers for the write barrier.
- // These registers may be used for Baker read barriers too.
locations->AddTemp(Location::RequiresRegister()); // Possibly used for ref. poisoning too.
// Ensure the card is in a byte register.
locations->AddTemp(Location::RegisterLocation(ECX));
@@ -5368,105 +5367,40 @@
__ Bind(¬_null);
}
- if (kEmitCompilerReadBarrier) {
- if (!kUseBakerReadBarrier) {
- // When (non-Baker) read barriers are enabled, the type
- // checking instrumentation requires two read barriers
- // generated by CodeGeneratorX86::GenerateReadBarrierSlow:
- //
- // __ movl(temp2, temp);
- // // /* HeapReference<Class> */ temp = temp->component_type_
- // __ movl(temp, Address(temp, component_offset));
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp_loc, temp_loc, temp2_loc, component_offset);
- //
- // // /* HeapReference<Class> */ temp2 = register_value->klass_
- // __ movl(temp2, Address(register_value, class_offset));
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp2_loc, temp2_loc, value, class_offset, temp_loc);
- //
- // __ cmpl(temp, temp2);
- //
- // However, the second read barrier may trash `temp`, as it
- // is a temporary register, and as such would not be saved
- // along with live registers before calling the runtime (nor
- // restored afterwards). So in this case, we bail out and
- // delegate the work to the array set slow path.
- //
- // TODO: Extend the register allocator to support a new
- // "(locally) live temp" location so as to avoid always
- // going into the slow path when read barriers are enabled?
- //
- // There is no such problem with Baker read barriers (see below).
- __ jmp(slow_path->GetEntryLabel());
- } else {
- Location temp2_loc = locations->GetTemp(1);
- Register temp2 = temp2_loc.AsRegister<Register>();
- // /* HeapReference<Class> */ temp = array->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(
- instruction, temp_loc, array, class_offset, /* needs_null_check */ true);
+ // Note that when Baker read barriers are enabled, the type
+ // checks are performed without read barriers. This is fine,
+ // even in the case where a class object is in the from-space
+ // after the flip, as a comparison involving such a type would
+ // not produce a false positive; it may of course produce a
+ // false negative, in which case we would take the ArraySet
+ // slow path.
- // /* HeapReference<Class> */ temp = temp->component_type_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(
- instruction, temp_loc, temp, component_offset, /* needs_null_check */ false);
- // Register `temp` is not trashed by the read barrier
- // emitted by GenerateFieldLoadWithBakerReadBarrier below,
- // as that method produces a call to a ReadBarrierMarkRegX
- // entry point, which saves all potentially live registers,
- // including temporaries such a `temp`.
- // /* HeapReference<Class> */ temp2 = register_value->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(
- instruction, temp2_loc, register_value, class_offset, /* needs_null_check */ false);
- // If heap poisoning is enabled, `temp` and `temp2` have
- // been unpoisoned by the the previous calls to
- // CodeGeneratorX86::GenerateFieldLoadWithBakerReadBarrier.
- __ cmpl(temp, temp2);
+ // /* HeapReference<Class> */ temp = array->klass_
+ __ movl(temp, Address(array, class_offset));
+ codegen_->MaybeRecordImplicitNullCheck(instruction);
+ __ MaybeUnpoisonHeapReference(temp);
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- __ j(kEqual, &do_put);
- // We do not need to emit a read barrier for the
- // following heap reference load, as `temp` is only used
- // in a comparison with null below, and this reference
- // is not kept afterwards. Also, if heap poisoning is
- // enabled, there is no need to unpoison that heap
- // reference for the same reason (comparison with null).
- __ cmpl(Address(temp, super_offset), Immediate(0));
- __ j(kNotEqual, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ j(kNotEqual, slow_path->GetEntryLabel());
- }
- }
- } else {
- // Non read barrier code.
+ // /* HeapReference<Class> */ temp = temp->component_type_
+ __ movl(temp, Address(temp, component_offset));
+ // If heap poisoning is enabled, no need to unpoison `temp`
+ // nor the object reference in `register_value->klass`, as
+ // we are comparing two poisoned references.
+ __ cmpl(temp, Address(register_value, class_offset));
- // /* HeapReference<Class> */ temp = array->klass_
- __ movl(temp, Address(array, class_offset));
- codegen_->MaybeRecordImplicitNullCheck(instruction);
+ if (instruction->StaticTypeOfArrayIsObjectArray()) {
+ __ j(kEqual, &do_put);
+ // If heap poisoning is enabled, the `temp` reference has
+ // not been unpoisoned yet; unpoison it now.
__ MaybeUnpoisonHeapReference(temp);
- // /* HeapReference<Class> */ temp = temp->component_type_
- __ movl(temp, Address(temp, component_offset));
- // If heap poisoning is enabled, no need to unpoison `temp`
- // nor the object reference in `register_value->klass`, as
- // we are comparing two poisoned references.
- __ cmpl(temp, Address(register_value, class_offset));
-
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- __ j(kEqual, &do_put);
- // If heap poisoning is enabled, the `temp` reference has
- // not been unpoisoned yet; unpoison it now.
- __ MaybeUnpoisonHeapReference(temp);
-
- // If heap poisoning is enabled, no need to unpoison the
- // heap reference loaded below, as it is only used for a
- // comparison with null.
- __ cmpl(Address(temp, super_offset), Immediate(0));
- __ j(kNotEqual, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ j(kNotEqual, slow_path->GetEntryLabel());
- }
+ // If heap poisoning is enabled, no need to unpoison the
+ // heap reference loaded below, as it is only used for a
+ // comparison with null.
+ __ cmpl(Address(temp, super_offset), Immediate(0));
+ __ j(kNotEqual, slow_path->GetEntryLabel());
+ __ Bind(&do_put);
+ } else {
+ __ j(kNotEqual, slow_path->GetEntryLabel());
}
}
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index 87b6de3..12c4b2c 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -4759,7 +4759,6 @@
if (needs_write_barrier) {
// Temporary registers for the write barrier.
- // These registers may be used for Baker read barriers too.
locations->AddTemp(Location::RequiresRegister()); // Possibly used for ref. poisoning too.
locations->AddTemp(Location::RequiresRegister());
}
@@ -4849,105 +4848,40 @@
__ Bind(¬_null);
}
- if (kEmitCompilerReadBarrier) {
- if (!kUseBakerReadBarrier) {
- // When (non-Baker) read barriers are enabled, the type
- // checking instrumentation requires two read barriers
- // generated by CodeGeneratorX86_64::GenerateReadBarrierSlow:
- //
- // __ movl(temp2, temp);
- // // /* HeapReference<Class> */ temp = temp->component_type_
- // __ movl(temp, Address(temp, component_offset));
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp_loc, temp_loc, temp2_loc, component_offset);
- //
- // // /* HeapReference<Class> */ temp2 = register_value->klass_
- // __ movl(temp2, Address(register_value, class_offset));
- // codegen_->GenerateReadBarrierSlow(
- // instruction, temp2_loc, temp2_loc, value, class_offset, temp_loc);
- //
- // __ cmpl(temp, temp2);
- //
- // However, the second read barrier may trash `temp`, as it
- // is a temporary register, and as such would not be saved
- // along with live registers before calling the runtime (nor
- // restored afterwards). So in this case, we bail out and
- // delegate the work to the array set slow path.
- //
- // TODO: Extend the register allocator to support a new
- // "(locally) live temp" location so as to avoid always
- // going into the slow path when read barriers are enabled?
- //
- // There is no such problem with Baker read barriers (see below).
- __ jmp(slow_path->GetEntryLabel());
- } else {
- Location temp2_loc = locations->GetTemp(1);
- CpuRegister temp2 = temp2_loc.AsRegister<CpuRegister>();
- // /* HeapReference<Class> */ temp = array->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(
- instruction, temp_loc, array, class_offset, /* needs_null_check */ true);
+ // Note that when Baker read barriers are enabled, the type
+ // checks are performed without read barriers. This is fine,
+ // even in the case where a class object is in the from-space
+ // after the flip, as a comparison involving such a type would
+ // not produce a false positive; it may of course produce a
+ // false negative, in which case we would take the ArraySet
+ // slow path.
- // /* HeapReference<Class> */ temp = temp->component_type_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(
- instruction, temp_loc, temp, component_offset, /* needs_null_check */ false);
- // Register `temp` is not trashed by the read barrier
- // emitted by GenerateFieldLoadWithBakerReadBarrier below,
- // as that method produces a call to a ReadBarrierMarkRegX
- // entry point, which saves all potentially live registers,
- // including temporaries such a `temp`.
- // /* HeapReference<Class> */ temp2 = register_value->klass_
- codegen_->GenerateFieldLoadWithBakerReadBarrier(
- instruction, temp2_loc, register_value, class_offset, /* needs_null_check */ false);
- // If heap poisoning is enabled, `temp` and `temp2` have
- // been unpoisoned by the the previous calls to
- // CodeGeneratorX86_64::GenerateFieldLoadWithBakerReadBarrier.
- __ cmpl(temp, temp2);
+ // /* HeapReference<Class> */ temp = array->klass_
+ __ movl(temp, Address(array, class_offset));
+ codegen_->MaybeRecordImplicitNullCheck(instruction);
+ __ MaybeUnpoisonHeapReference(temp);
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- __ j(kEqual, &do_put);
- // We do not need to emit a read barrier for the
- // following heap reference load, as `temp` is only used
- // in a comparison with null below, and this reference
- // is not kept afterwards. Also, if heap poisoning is
- // enabled, there is no need to unpoison that heap
- // reference for the same reason (comparison with null).
- __ cmpl(Address(temp, super_offset), Immediate(0));
- __ j(kNotEqual, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ j(kNotEqual, slow_path->GetEntryLabel());
- }
- }
- } else {
- // Non read barrier code.
+ // /* HeapReference<Class> */ temp = temp->component_type_
+ __ movl(temp, Address(temp, component_offset));
+ // If heap poisoning is enabled, no need to unpoison `temp`
+ // nor the object reference in `register_value->klass`, as
+ // we are comparing two poisoned references.
+ __ cmpl(temp, Address(register_value, class_offset));
- // /* HeapReference<Class> */ temp = array->klass_
- __ movl(temp, Address(array, class_offset));
- codegen_->MaybeRecordImplicitNullCheck(instruction);
+ if (instruction->StaticTypeOfArrayIsObjectArray()) {
+ __ j(kEqual, &do_put);
+ // If heap poisoning is enabled, the `temp` reference has
+ // not been unpoisoned yet; unpoison it now.
__ MaybeUnpoisonHeapReference(temp);
- // /* HeapReference<Class> */ temp = temp->component_type_
- __ movl(temp, Address(temp, component_offset));
- // If heap poisoning is enabled, no need to unpoison `temp`
- // nor the object reference in `register_value->klass`, as
- // we are comparing two poisoned references.
- __ cmpl(temp, Address(register_value, class_offset));
-
- if (instruction->StaticTypeOfArrayIsObjectArray()) {
- __ j(kEqual, &do_put);
- // If heap poisoning is enabled, the `temp` reference has
- // not been unpoisoned yet; unpoison it now.
- __ MaybeUnpoisonHeapReference(temp);
-
- // If heap poisoning is enabled, no need to unpoison the
- // heap reference loaded below, as it is only used for a
- // comparison with null.
- __ cmpl(Address(temp, super_offset), Immediate(0));
- __ j(kNotEqual, slow_path->GetEntryLabel());
- __ Bind(&do_put);
- } else {
- __ j(kNotEqual, slow_path->GetEntryLabel());
- }
+ // If heap poisoning is enabled, no need to unpoison the
+ // heap reference loaded below, as it is only used for a
+ // comparison with null.
+ __ cmpl(Address(temp, super_offset), Immediate(0));
+ __ j(kNotEqual, slow_path->GetEntryLabel());
+ __ Bind(&do_put);
+ } else {
+ __ j(kNotEqual, slow_path->GetEntryLabel());
}
}
