arm64: Clean up VarHandle intrinsics implementation.
Fix some typos and update some table lookup code that's
currently unused. Bring in a few things from arm (naming,
static assertion, pull an expression to a named variable).
Test: testrunner.py --target --64 --optimizing
Bug: 71781600
Change-Id: If2f2c4417942a272a8ad672c6b876e0569f8827c
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index efa65fc..8f4979f 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -1070,7 +1070,7 @@
uint32_t prev_insn = GetInsn(literal_offset - 4u);
const uint32_t root_reg = BakerReadBarrierFirstRegField::Decode(encoded_data);
// Usually LDR (immediate) with correct root_reg but
- // we may have a "MOV marked, old_value" for UnsafeCASObject.
+ // we may have a "MOV marked, old_value" for intrinsic CAS.
if ((prev_insn & 0xffe0ffff) != (0x2a0003e0 | root_reg)) { // MOV?
CHECK_EQ(prev_insn & 0xffc0001fu, 0xb9400000u | root_reg); // LDR?
}
@@ -6504,21 +6504,21 @@
MaybeGenerateMarkingRegisterCheck(/* code= */ __LINE__);
}
-void CodeGeneratorARM64::GenerateUnsafeCasOldValueMovWithBakerReadBarrier(
- vixl::aarch64::Register marked,
+void CodeGeneratorARM64::GenerateIntrinsicCasMoveWithBakerReadBarrier(
+ vixl::aarch64::Register marked_old_value,
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());
+ uint32_t custom_data = EncodeBakerReadBarrierGcRootData(marked_old_value.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);
+ __ mov(marked_old_value, old_value);
EmitBakerReadBarrierCbnz(custom_data);
__ bind(&return_address);
}
diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h
index ec48245..c7c11f5 100644
--- a/compiler/optimizing/code_generator_arm64.h
+++ b/compiler/optimizing/code_generator_arm64.h
@@ -808,9 +808,9 @@
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);
+ // Generate MOV for the `old_value` in intrinsic CAS and mark it with Baker read barrier.
+ void GenerateIntrinsicCasMoveWithBakerReadBarrier(vixl::aarch64::Register marked_old_value,
+ 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.
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
index f19e286..e7315ec 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -930,7 +930,7 @@
? LocationSummary::kCallOnSlowPath
: LocationSummary::kNoCall,
kIntrinsified);
- if (can_call) {
+ if (can_call && kUseBakerReadBarrier) {
locations->SetCustomSlowPathCallerSaves(RegisterSet::Empty()); // No caller-save registers.
}
locations->SetInAt(0, Location::NoLocation()); // Unused receiver.
@@ -1094,7 +1094,7 @@
if (expected2.IsValid()) {
__ Ccmp(old_value, expected2, ZFlag, ne);
}
- // If the comparison failed, the Z flag is cleared as we branch to the `failure` label.
+ // If the comparison failed, the Z flag is cleared as we branch to the `cmp_failure` label.
// If the comparison succeeded, the Z flag is set and remains set after the end of the
// code emitted here, unless we retry the whole operation.
__ B(cmp_failure, ne);
@@ -1166,8 +1166,7 @@
// Mark the `old_value_` from the main path and compare with `expected_`.
if (kUseBakerReadBarrier) {
DCHECK(mark_old_value_slow_path_ == nullptr);
- arm64_codegen->GenerateUnsafeCasOldValueMovWithBakerReadBarrier(old_value_temp_,
- old_value_);
+ arm64_codegen->GenerateIntrinsicCasMoveWithBakerReadBarrier(old_value_temp_, old_value_);
} else {
DCHECK(mark_old_value_slow_path_ != nullptr);
__ B(mark_old_value_slow_path_->GetEntryLabel());
@@ -1221,8 +1220,7 @@
__ Bind(&mark_old_value);
if (kUseBakerReadBarrier) {
DCHECK(update_old_value_slow_path_ == nullptr);
- arm64_codegen->GenerateUnsafeCasOldValueMovWithBakerReadBarrier(old_value_,
- old_value_temp_);
+ arm64_codegen->GenerateIntrinsicCasMoveWithBakerReadBarrier(old_value_, old_value_temp_);
} else {
// Note: We could redirect the `failure` above directly to the entry label and bind
// the exit label in the main path, but the main path would need to access the
@@ -1342,7 +1340,8 @@
} else {
// To preserve the old value across the non-Baker read barrier
// slow path, use a fixed callee-save register.
- locations->AddTemp(Location::RegisterLocation(CTZ(kArm64CalleeSaveRefSpills)));
+ constexpr int first_callee_save = CTZ(kArm64CalleeSaveRefSpills);
+ locations->AddTemp(Location::RegisterLocation(first_callee_save));
// To reduce the number of moves, request x0 as the second temporary.
DCHECK(InvokeRuntimeCallingConvention().GetReturnLocation(DataType::Type::kReference).Equals(
Location::RegisterLocation(x0.GetCode())));
@@ -1358,10 +1357,6 @@
GenUnsafeCas(invoke, DataType::Type::kInt64, codegen_);
}
void IntrinsicCodeGeneratorARM64::VisitUnsafeCASObject(HInvoke* invoke) {
- // The only read barrier implementation supporting the
- // UnsafeCASObject intrinsic is the Baker-style read barriers.
- DCHECK(!kEmitCompilerReadBarrier || kUseBakerReadBarrier);
-
GenUnsafeCas(invoke, DataType::Type::kReference, codegen_);
}
@@ -4082,7 +4077,8 @@
GetExpectedVarHandleCoordinatesCount(invoke) == 0u) { // For static fields.
// To preserve the offset value across the non-Baker read barrier slow path
// for loading the declaring class, use a fixed callee-save register.
- locations->AddTemp(Location::RegisterLocation(CTZ(kArm64CalleeSaveRefSpills)));
+ constexpr int first_callee_save = CTZ(kArm64CalleeSaveRefSpills);
+ locations->AddTemp(Location::RegisterLocation(first_callee_save));
} else {
locations->AddTemp(Location::RequiresRegister());
}
@@ -4303,12 +4299,22 @@
LocationSummary* locations = CreateVarHandleFieldLocations(invoke);
- if ((kEmitCompilerReadBarrier && !kUseBakerReadBarrier) &&
- GetExpectedVarHandleCoordinatesCount(invoke) == 0u) { // For static fields.
- // Not implemented for references, see above.
- // Note that we would need a callee-save register instead of the temporary
- // reserved in CreateVarHandleFieldLocations() for the class object.
- DCHECK_NE(value_type, DataType::Type::kReference);
+ if (kEmitCompilerReadBarrier && !kUseBakerReadBarrier) {
+ // We need callee-save registers for both the class object and offset instead of
+ // the temporaries reserved in CreateVarHandleFieldLocations().
+ static_assert(POPCOUNT(kArm64CalleeSaveRefSpills) >= 2u);
+ uint32_t first_callee_save = CTZ(kArm64CalleeSaveRefSpills);
+ uint32_t second_callee_save = CTZ(kArm64CalleeSaveRefSpills ^ (1u << first_callee_save));
+ if (GetExpectedVarHandleCoordinatesCount(invoke) == 0u) { // For static fields.
+ DCHECK_EQ(locations->GetTempCount(), 2u);
+ DCHECK(locations->GetTemp(0u).Equals(Location::RequiresRegister()));
+ DCHECK(locations->GetTemp(1u).Equals(Location::RegisterLocation(first_callee_save)));
+ locations->SetTempAt(0u, Location::RegisterLocation(second_callee_save));
+ } else {
+ DCHECK_EQ(locations->GetTempCount(), 1u);
+ DCHECK(locations->GetTemp(0u).Equals(Location::RequiresRegister()));
+ locations->SetTempAt(0u, Location::RegisterLocation(first_callee_save));
+ }
}
if (!return_success && DataType::IsFloatingPointType(value_type)) {
// Add a temporary for old value and exclusive store result if floating point
@@ -4640,7 +4646,7 @@
arg = MoveToTempIfFpRegister(arg, value_type, masm, &temps);
if (DataType::IsFloatingPointType(value_type) && !arg.IsZero()) {
// We need a temporary register but we have already used a scratch register for
- // the new value unless it is zero bit pattern (+0.0f or +0.0) and need anothe one
+ // the new value unless it is zero bit pattern (+0.0f or +0.0) and need another one
// in GenerateGetAndUpdate(). We have allocated a normal temporary to handle that.
Location temp = locations->GetTemp((expected_coordinates_count == 0u) ? 2u : 1u);
old_value =
@@ -4663,7 +4669,7 @@
}
} else if (kEmitCompilerReadBarrier && value_type == DataType::Type::kReference) {
if (kUseBakerReadBarrier) {
- codegen->GenerateUnsafeCasOldValueMovWithBakerReadBarrier(out.W(), old_value.W());
+ codegen->GenerateIntrinsicCasMoveWithBakerReadBarrier(out.W(), old_value.W());
} else {
codegen->GenerateReadBarrierSlow(
invoke,