Revert "Revert "x86/x86-64: Avoid temporary for read barrier field load.""
Fixed the fault handler recognizing the TEST instruction and
fault address within the lock word. Added tests to 439-npe.
Bug: 29966877
Bug: 12687968
Test: Tested with ART_USE_READ_BARRIER=true on host.
Test: Tested with ART_USE_READ_BARRIER=true ART_HEAP_POISONING=true on host.
This reverts commit ccf15bca330f9a23337b1a4b5850f7fcc6c1bf15.
Change-Id: I8990def5f719c9205bf6e5fdba32027fa82bec50
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 49d6c19..cf4a040 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -1934,10 +1934,9 @@
Register output = output_loc.AsRegister<Register>();
if (kEmitCompilerReadBarrier) {
if (kUseBakerReadBarrier) {
- Location temp = locations->GetTemp(0);
Address src(base, offset, ScaleFactor::TIMES_1, 0);
codegen->GenerateReferenceLoadWithBakerReadBarrier(
- invoke, output_loc, base, src, temp, /* needs_null_check */ false);
+ invoke, output_loc, base, src, /* needs_null_check */ false);
} else {
__ movl(output, Address(base, offset, ScaleFactor::TIMES_1, 0));
codegen->GenerateReadBarrierSlow(
@@ -2000,11 +1999,6 @@
locations->SetOut(Location::RequiresRegister(),
can_call ? Location::kOutputOverlap : Location::kNoOutputOverlap);
}
- if (type == Primitive::kPrimNot && kEmitCompilerReadBarrier && kUseBakerReadBarrier) {
- // We need a temporary register for the read barrier marking slow
- // path in InstructionCodeGeneratorX86::GenerateReferenceLoadWithBakerReadBarrier.
- locations->AddTemp(Location::RequiresRegister());
- }
}
void IntrinsicLocationsBuilderX86::VisitUnsafeGet(HInvoke* invoke) {
@@ -2933,11 +2927,11 @@
if (kEmitCompilerReadBarrier && kUseBakerReadBarrier) {
// /* HeapReference<Class> */ temp1 = src->klass_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp1_loc, src, class_offset, temp2_loc, /* needs_null_check */ false);
+ invoke, temp1_loc, src, class_offset, /* needs_null_check */ false);
// Bail out if the source is not a non primitive array.
// /* HeapReference<Class> */ temp1 = temp1->component_type_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp1_loc, temp1, component_offset, temp2_loc, /* needs_null_check */ false);
+ invoke, temp1_loc, temp1, component_offset, /* needs_null_check */ false);
__ testl(temp1, temp1);
__ j(kEqual, intrinsic_slow_path->GetEntryLabel());
// If heap poisoning is enabled, `temp1` has been unpoisoned
@@ -2970,7 +2964,7 @@
// /* HeapReference<Class> */ temp1 = dest->klass_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp1_loc, dest, class_offset, temp2_loc, /* needs_null_check */ false);
+ invoke, temp1_loc, dest, class_offset, /* needs_null_check */ false);
if (!optimizations.GetDestinationIsNonPrimitiveArray()) {
// Bail out if the destination is not a non primitive array.
@@ -2982,7 +2976,7 @@
// temporaries such a `temp1`.
// /* HeapReference<Class> */ temp2 = temp1->component_type_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp2_loc, temp1, component_offset, temp3_loc, /* needs_null_check */ false);
+ invoke, temp2_loc, temp1, component_offset, /* needs_null_check */ false);
__ testl(temp2, temp2);
__ j(kEqual, intrinsic_slow_path->GetEntryLabel());
// If heap poisoning is enabled, `temp2` has been unpoisoned
@@ -2995,7 +2989,7 @@
// read barrier emitted by GenerateFieldLoadWithBakerReadBarrier below.
// /* HeapReference<Class> */ temp2 = src->klass_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp2_loc, src, class_offset, temp3_loc, /* needs_null_check */ false);
+ invoke, temp2_loc, src, class_offset, /* needs_null_check */ false);
// Note: if heap poisoning is on, we are comparing two unpoisoned references here.
__ cmpl(temp1, temp2);
@@ -3004,7 +2998,7 @@
__ j(kEqual, &do_copy);
// /* HeapReference<Class> */ temp1 = temp1->component_type_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp1_loc, temp1, component_offset, temp2_loc, /* needs_null_check */ false);
+ invoke, temp1_loc, temp1, component_offset, /* needs_null_check */ false);
// 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
@@ -3058,10 +3052,10 @@
if (kEmitCompilerReadBarrier && kUseBakerReadBarrier) {
// /* HeapReference<Class> */ temp1 = src->klass_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp1_loc, src, class_offset, temp2_loc, /* needs_null_check */ false);
+ invoke, temp1_loc, src, class_offset, /* needs_null_check */ false);
// /* HeapReference<Class> */ temp1 = temp1->component_type_
codegen_->GenerateFieldLoadWithBakerReadBarrier(
- invoke, temp1_loc, temp1, component_offset, temp2_loc, /* needs_null_check */ false);
+ invoke, temp1_loc, temp1, component_offset, /* needs_null_check */ false);
__ testl(temp1, temp1);
__ j(kEqual, intrinsic_slow_path->GetEntryLabel());
// If heap poisoning is enabled, `temp1` has been unpoisoned
@@ -3139,11 +3133,18 @@
__ cmpl(temp1, temp3);
__ j(kEqual, &done);
- // /* int32_t */ monitor = src->monitor_
- __ movl(temp2, Address(src, monitor_offset));
- // /* LockWord */ lock_word = LockWord(monitor)
- static_assert(sizeof(LockWord) == sizeof(int32_t),
- "art::LockWord and int32_t have different sizes.");
+ // Given the numeric representation, it's enough to check the low bit of the rb_state.
+ static_assert(ReadBarrier::white_ptr_ == 0, "Expecting white to have value 0");
+ static_assert(ReadBarrier::gray_ptr_ == 1, "Expecting gray to have value 1");
+ static_assert(ReadBarrier::black_ptr_ == 2, "Expecting black to have value 2");
+ constexpr uint32_t gray_byte_position = LockWord::kReadBarrierStateShift / kBitsPerByte;
+ constexpr uint32_t gray_bit_position = LockWord::kReadBarrierStateShift % kBitsPerByte;
+ constexpr int32_t test_value = static_cast<int8_t>(1 << gray_bit_position);
+
+ // if (rb_state == ReadBarrier::gray_ptr_)
+ // goto slow_path;
+ // At this point, just do the "if" and make sure that flags are preserved until the branch.
+ __ testb(Address(src, monitor_offset + gray_byte_position), Immediate(test_value));
// Load fence to prevent load-load reordering.
// Note that this is a no-op, thanks to the x86 memory model.
@@ -3154,13 +3155,8 @@
new (GetAllocator()) ReadBarrierSystemArrayCopySlowPathX86(invoke);
codegen_->AddSlowPath(read_barrier_slow_path);
- // 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 SHR.
- static_assert(ReadBarrier::white_ptr_ == 0, "Expecting white to have value 0");
- static_assert(ReadBarrier::gray_ptr_ == 1, "Expecting gray to have value 1");
- static_assert(ReadBarrier::black_ptr_ == 2, "Expecting black to have value 2");
- __ shrl(temp2, Immediate(LockWord::kReadBarrierStateShift + 1));
- __ j(kCarrySet, read_barrier_slow_path->GetEntryLabel());
+ // We have done the "if" of the gray bit check above, now branch based on the flags.
+ __ j(kNotZero, read_barrier_slow_path->GetEntryLabel());
// Fast-path copy.