Minor fixes and cleaning of arm64 static and direct calls code.
Fixes:
The proper way to avoid the MacroAssembler to generate code before or
after an instruction is to block the pools (usually via
`vixl::BlockPoolsScope`). Here we can use
`vixl::SingleEmissionCheckScope`, that checks we generate only one
instruction and also blocks the pools.
In practice the current code would have worked fine because VIXL would
not have generated anything after `Bl()` or `Ldr()`, but that was not
guaranteed.
Cleaning:
- `XRegisterFrom()` returns an X register. Calling `.X()` is not
required.
- Since we are sure (after the previous fixes) that nothing will be
emitted around the instructions we care about, update the code to
bind labels before the instructions for simplicity.
Change-Id: I42d49976721e380e66bcd7a5b345f1777009434a
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 7e248b4..d2378d5 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -2856,18 +2856,18 @@
switch (invoke->GetMethodLoadKind()) {
case HInvokeStaticOrDirect::MethodLoadKind::kStringInit:
// temp = thread->string_init_entrypoint
- __ Ldr(XRegisterFrom(temp).X(), MemOperand(tr, invoke->GetStringInitOffset()));
+ __ Ldr(XRegisterFrom(temp), MemOperand(tr, invoke->GetStringInitOffset()));
break;
case HInvokeStaticOrDirect::MethodLoadKind::kRecursive:
callee_method = invoke->GetLocations()->InAt(invoke->GetCurrentMethodInputIndex());
break;
case HInvokeStaticOrDirect::MethodLoadKind::kDirectAddress:
// Load method address from literal pool.
- __ Ldr(XRegisterFrom(temp).X(), DeduplicateUint64Literal(invoke->GetMethodAddress()));
+ __ Ldr(XRegisterFrom(temp), DeduplicateUint64Literal(invoke->GetMethodAddress()));
break;
case HInvokeStaticOrDirect::MethodLoadKind::kDirectAddressWithFixup:
// Load method address from literal pool with a link-time patch.
- __ Ldr(XRegisterFrom(temp).X(),
+ __ Ldr(XRegisterFrom(temp),
DeduplicateMethodAddressLiteral(invoke->GetTargetMethod()));
break;
case HInvokeStaticOrDirect::MethodLoadKind::kDexCachePcRelative: {
@@ -2877,16 +2877,19 @@
vixl::Label* pc_insn_label = &pc_relative_dex_cache_patches_.back().label;
{
vixl::SingleEmissionCheckScope guard(GetVIXLAssembler());
- __ adrp(XRegisterFrom(temp).X(), 0);
+ __ Bind(pc_insn_label);
+ __ adrp(XRegisterFrom(temp), 0);
}
- __ Bind(pc_insn_label); // Bind after ADRP.
pc_relative_dex_cache_patches_.back().pc_insn_label = pc_insn_label;
// Add LDR with its PC-relative DexCache access patch.
pc_relative_dex_cache_patches_.emplace_back(*invoke->GetTargetMethod().dex_file,
invoke->GetDexCacheArrayOffset());
- __ Ldr(XRegisterFrom(temp).X(), MemOperand(XRegisterFrom(temp).X(), 0));
- __ Bind(&pc_relative_dex_cache_patches_.back().label); // Bind after LDR.
- pc_relative_dex_cache_patches_.back().pc_insn_label = pc_insn_label;
+ {
+ vixl::SingleEmissionCheckScope guard(GetVIXLAssembler());
+ __ Bind(&pc_relative_dex_cache_patches_.back().label);
+ __ ldr(XRegisterFrom(temp), MemOperand(XRegisterFrom(temp), 0));
+ pc_relative_dex_cache_patches_.back().pc_insn_label = pc_insn_label;
+ }
break;
}
case HInvokeStaticOrDirect::MethodLoadKind::kDexCacheViaMethod: {
@@ -2920,8 +2923,9 @@
case HInvokeStaticOrDirect::CodePtrLocation::kCallPCRelative: {
relative_call_patches_.emplace_back(invoke->GetTargetMethod());
vixl::Label* label = &relative_call_patches_.back().label;
- __ Bl(label); // Arbitrarily branch to the instruction after BL, override at link time.
- __ Bind(label); // Bind after BL.
+ vixl::SingleEmissionCheckScope guard(GetVIXLAssembler());
+ __ Bind(label);
+ __ bl(0); // Branch and link to itself. This will be overriden at link time.
break;
}
case HInvokeStaticOrDirect::CodePtrLocation::kCallDirectWithFixup:
@@ -2934,7 +2938,7 @@
case HInvokeStaticOrDirect::CodePtrLocation::kCallArtMethod:
// LR = callee_method->entry_point_from_quick_compiled_code_;
__ Ldr(lr, MemOperand(
- XRegisterFrom(callee_method).X(),
+ XRegisterFrom(callee_method),
ArtMethod::EntryPointFromQuickCompiledCodeOffset(kArm64WordSize).Int32Value()));
// lr()
__ Blr(lr);
@@ -2990,14 +2994,14 @@
target_method.dex_method_index));
}
for (const MethodPatchInfo<vixl::Label>& info : relative_call_patches_) {
- linker_patches->push_back(LinkerPatch::RelativeCodePatch(info.label.location() - 4u,
+ linker_patches->push_back(LinkerPatch::RelativeCodePatch(info.label.location(),
info.target_method.dex_file,
info.target_method.dex_method_index));
}
for (const PcRelativeDexCacheAccessInfo& info : pc_relative_dex_cache_patches_) {
- linker_patches->push_back(LinkerPatch::DexCacheArrayPatch(info.label.location() - 4u,
+ linker_patches->push_back(LinkerPatch::DexCacheArrayPatch(info.label.location(),
&info.target_dex_file,
- info.pc_insn_label->location() - 4u,
+ info.pc_insn_label->location(),
info.element_offset));
}
}
diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h
index aa5ad38..cac8b51 100644
--- a/compiler/optimizing/code_generator_arm64.h
+++ b/compiler/optimizing/code_generator_arm64.h
@@ -422,8 +422,6 @@
const DexFile& target_dex_file;
uint32_t element_offset;
- // NOTE: Labels are bound to the end of the patched instruction because
- // we don't know if there will be a veneer or how big it will be.
vixl::Label label;
vixl::Label* pc_insn_label;
};
