Do not use HArm64IntermediateAddress with read barriers.
This ARM64 instruction simplification does not yet work
correctly with the read barrier compiler instrumentation.
Bug: 26601270
Bug: 12687968
Change-Id: I0c3c5d0043ebd936e00984740efbae8b3025c7ca
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 5e905fc..6ed2c5a 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -604,30 +604,13 @@
DCHECK(!instruction_->IsInvoke() ||
(instruction_->IsInvokeStaticOrDirect() &&
instruction_->GetLocations()->Intrinsified()));
+ // The read barrier instrumentation does not support the
+ // HArm64IntermediateAddress instruction yet.
+ DCHECK(!(instruction_->IsArrayGet() &&
+ instruction_->AsArrayGet()->GetArray()->IsArm64IntermediateAddress()));
__ Bind(GetEntryLabel());
- // Note: In the case of a HArrayGet instruction, when the base
- // address is a HArm64IntermediateAddress instruction, it does not
- // point to the array object itself, but to an offset within this
- // object. However, the read barrier entry point needs the array
- // object address to be passed as first argument. So we
- // temporarily set back `obj_` to that address, and restore its
- // initial value later.
- if (instruction_->IsArrayGet() &&
- instruction_->AsArrayGet()->GetArray()->IsArm64IntermediateAddress()) {
- if (kIsDebugBuild) {
- HArm64IntermediateAddress* intermediate_address =
- instruction_->AsArrayGet()->GetArray()->AsArm64IntermediateAddress();
- uint32_t intermediate_address_offset =
- intermediate_address->GetOffset()->AsIntConstant()->GetValueAsUint64();
- DCHECK_EQ(intermediate_address_offset, offset_);
- DCHECK_EQ(mirror::Array::DataOffset(Primitive::ComponentSize(type)).Uint32Value(), offset_);
- }
- Register obj_reg = RegisterFrom(obj_, Primitive::kPrimInt);
- __ Sub(obj_reg, obj_reg, offset_);
- }
-
SaveLiveRegisters(codegen, locations);
// We may have to change the index's value, but as `index_` is a
@@ -728,22 +711,6 @@
RestoreLiveRegisters(codegen, locations);
- // Restore the value of `obj_` when it corresponds to a
- // HArm64IntermediateAddress instruction.
- if (instruction_->IsArrayGet() &&
- instruction_->AsArrayGet()->GetArray()->IsArm64IntermediateAddress()) {
- if (kIsDebugBuild) {
- HArm64IntermediateAddress* intermediate_address =
- instruction_->AsArrayGet()->GetArray()->AsArm64IntermediateAddress();
- uint32_t intermediate_address_offset =
- intermediate_address->GetOffset()->AsIntConstant()->GetValueAsUint64();
- DCHECK_EQ(intermediate_address_offset, offset_);
- DCHECK_EQ(mirror::Array::DataOffset(Primitive::ComponentSize(type)).Uint32Value(), offset_);
- }
- Register obj_reg = RegisterFrom(obj_, Primitive::kPrimInt);
- __ Add(obj_reg, obj_reg, offset_);
- }
-
__ B(GetExitLabel());
}
@@ -1970,6 +1937,9 @@
}
void LocationsBuilderARM64::VisitArm64IntermediateAddress(HArm64IntermediateAddress* instruction) {
+ // The read barrier instrumentation does not support the
+ // HArm64IntermediateAddress instruction yet.
+ DCHECK(!kEmitCompilerReadBarrier);
LocationSummary* locations =
new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
locations->SetInAt(0, Location::RequiresRegister());
@@ -1979,6 +1949,9 @@
void InstructionCodeGeneratorARM64::VisitArm64IntermediateAddress(
HArm64IntermediateAddress* instruction) {
+ // The read barrier instrumentation does not support the
+ // HArm64IntermediateAddress instruction yet.
+ DCHECK(!kEmitCompilerReadBarrier);
__ Add(OutputRegister(instruction),
InputRegisterAt(instruction, 0),
Operand(InputOperandAt(instruction, 1)));
@@ -2067,6 +2040,9 @@
} else {
Register temp = temps.AcquireSameSizeAs(obj);
if (instruction->GetArray()->IsArm64IntermediateAddress()) {
+ // The read barrier instrumentation does not support the
+ // HArm64IntermediateAddress instruction yet.
+ DCHECK(!kEmitCompilerReadBarrier);
// We do not need to compute the intermediate address from the array: the
// input instruction has done it already. See the comment in
// `InstructionSimplifierArm64::TryExtractArrayAccessAddress()`.
@@ -2093,11 +2069,6 @@
if (index.IsConstant()) {
codegen_->MaybeGenerateReadBarrier(instruction, out, out, obj_loc, offset);
} else {
- // Note: when `obj_loc` is a HArm64IntermediateAddress, it does
- // not contain the base address of the array object, which is
- // needed by the read barrier entry point. So the read barrier
- // slow path will temporarily set back `obj_loc` to the right
- // address (see ReadBarrierForHeapReferenceSlowPathARM64::EmitNativeCode).
codegen_->MaybeGenerateReadBarrier(instruction, out, out, obj_loc, offset, index);
}
}
@@ -2161,6 +2132,9 @@
UseScratchRegisterScope temps(masm);
Register temp = temps.AcquireSameSizeAs(array);
if (instruction->GetArray()->IsArm64IntermediateAddress()) {
+ // The read barrier instrumentation does not support the
+ // HArm64IntermediateAddress instruction yet.
+ DCHECK(!kEmitCompilerReadBarrier);
// We do not need to compute the intermediate address from the array: the
// input instruction has done it already. See the comment in
// `InstructionSimplifierArm64::TryExtractArrayAccessAddress()`.
diff --git a/compiler/optimizing/instruction_simplifier_arm64.cc b/compiler/optimizing/instruction_simplifier_arm64.cc
index 6bbc751..4bcfc54 100644
--- a/compiler/optimizing/instruction_simplifier_arm64.cc
+++ b/compiler/optimizing/instruction_simplifier_arm64.cc
@@ -30,6 +30,15 @@
HInstruction* array,
HInstruction* index,
int access_size) {
+ if (kEmitCompilerReadBarrier) {
+ // The read barrier instrumentation does not support the
+ // HArm64IntermediateAddress instruction yet.
+ //
+ // TODO: Handle this case properly in the ARM64 code generator and
+ // re-enable this optimization; otherwise, remove this TODO.
+ // b/26601270
+ return;
+ }
if (index->IsConstant() ||
(index->IsBoundsCheck() && index->AsBoundsCheck()->GetIndex()->IsConstant())) {
// When the index is a constant all the addressing can be fitted in the
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index 7ec3067..2195b40 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -546,10 +546,13 @@
# Tests that should fail in the read barrier configuration with the Optimizing compiler.
# 484: Baker's fast path based read barrier compiler instrumentation generates code containing
# more parallel moves on x86, thus some Checker assertions may fail.
+# 527: On ARM64, the read barrier instrumentation does not support the HArm64IntermediateAddress
+# instruction yet (b/26601270).
# 537: Expects an array copy to be intrinsified on x86-64, but calling-on-slowpath intrinsics are
# not yet handled in the read barrier configuration.
TEST_ART_BROKEN_OPTIMIZING_READ_BARRIER_RUN_TESTS := \
484-checker-register-hints \
+ 527-checker-array-access-split \
537-checker-arraycopy
# Tests that should fail in the read barrier configuration with JIT.