Commit 78dfde9d authored by Jakob Gruber's avatar Jakob Gruber Committed by Commit Bot

Revert "[arm64] Cleanup TODO around handling of x18"

This reverts commit 7a2651cb.

Reason for revert: https://ci.chromium.org/p/v8/builders/ci/V8%20Android%20Arm64%20-%20N5X/4126

Original change's description:
> [arm64] Cleanup TODO around handling of x18
> 
> Use `padreg` instead of x18 to maintain alignment in the CPURegList.
> 
> Also clean up some comments and tidy up RequiredStackSizeForCallerSaved
> and PushCallerSaved.
> 
> Change-Id: I80a780e5649e69a1746c43f37c2d1d875120c7a0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1581609
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Commit-Queue: Martyn Capewell <martyn.capewell@arm.com>
> Cr-Commit-Position: refs/heads/master@{#60987}

TBR=jgruber@chromium.org,martyn.capewell@arm.com,joey.gouly@arm.com

Change-Id: Id95ac26142717f6503d284d20ca03b9de33a9122
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1582403Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60999}
parent 6f7b8f62
......@@ -98,18 +98,6 @@ void CPURegList::RemoveCalleeSaved() {
}
}
void CPURegList::Align() {
// Use padreg, if necessary, to maintain stack alignment.
if (Count() % 2 != 0) {
if (IncludesAliasOf(padreg)) {
Remove(padreg);
} else {
Combine(padreg);
}
}
DCHECK_EQ(Count() % 2, 0);
}
CPURegList CPURegList::GetCalleeSaved(int size) {
return CPURegList(CPURegister::kRegister, size, 19, 29);
......
......@@ -55,6 +55,12 @@ void CopyRegListToFrame(MacroAssembler* masm, const Register& dst,
masm->Sub(dst, dst, dst_offset);
}
// TODO(jgruber): There's a hack here to explicitly skip restoration of the
// so-called 'arm64 platform register' x18. The register may be in use by the
// OS, thus we should not clobber it. Instead of this hack, it would be nicer
// not to add x18 to the list of saved registers in the first place. The
// complication here is that we require `reg_list.Count() % 2 == 0` in multiple
// spots.
void RestoreRegList(MacroAssembler* masm, const CPURegList& reg_list,
const Register& src_base, int src_offset) {
DCHECK_EQ(reg_list.Count() % 2, 0);
......@@ -68,8 +74,8 @@ void RestoreRegList(MacroAssembler* masm, const CPURegList& reg_list,
Register src = temps.AcquireX();
masm->Add(src, src_base, src_offset);
// No need to restore padreg.
restore_list.Remove(padreg);
// x18 is the platform register and is reserved for the use of platform ABIs.
restore_list.Remove(x18);
// Restore every register in restore_list from src.
while (!restore_list.IsEmpty()) {
......
......@@ -40,11 +40,25 @@ CPURegList TurboAssembler::DefaultFPTmpList() {
int TurboAssembler::RequiredStackSizeForCallerSaved(SaveFPRegsMode fp_mode,
Register exclusion) const {
int bytes = 0;
auto list = kCallerSaved;
list.Remove(exclusion);
list.Align();
// We only allow one exclusion register, so if the list is of even length
// before exclusions, it must still be afterwards, to maintain alignment.
// Therefore, we can ignore the exclusion register in the computation.
// However, we leave it in the argument list to mirror the prototype for
// Push/PopCallerSaved().
// X18 is excluded from caller-saved register list on ARM64 which makes
// caller-saved registers in odd number. padreg is used accordingly to
// maintain the alignment.
DCHECK_EQ(list.Count() % 2, 1);
if (exclusion.Is(no_reg)) {
bytes += kXRegSizeInBits / 8;
} else {
bytes -= kXRegSizeInBits / 8;
}
int bytes = list.Count() * kXRegSizeInBits / 8;
bytes += list.Count() * kXRegSizeInBits / 8;
if (fp_mode == kSaveFPRegs) {
DCHECK_EQ(kCallerSavedV.Count() % 2, 0);
......@@ -55,13 +69,20 @@ int TurboAssembler::RequiredStackSizeForCallerSaved(SaveFPRegsMode fp_mode,
int TurboAssembler::PushCallerSaved(SaveFPRegsMode fp_mode,
Register exclusion) {
int bytes = 0;
auto list = kCallerSaved;
list.Remove(exclusion);
list.Align();
PushCPURegList(list);
// X18 is excluded from caller-saved register list on ARM64, use padreg
// accordingly to maintain alignment.
if (!exclusion.Is(no_reg)) {
list.Remove(exclusion);
} else {
list.Combine(padreg);
}
int bytes = list.Count() * kXRegSizeInBits / 8;
DCHECK_EQ(list.Count() % 2, 0);
PushCPURegList(list);
bytes += list.Count() * kXRegSizeInBits / 8;
if (fp_mode == kSaveFPRegs) {
DCHECK_EQ(kCallerSavedV.Count() % 2, 0);
......@@ -80,9 +101,16 @@ int TurboAssembler::PopCallerSaved(SaveFPRegsMode fp_mode, Register exclusion) {
}
auto list = kCallerSaved;
list.Remove(exclusion);
list.Align();
// X18 is excluded from caller-saved register list on ARM64, use padreg
// accordingly to maintain alignment.
if (!exclusion.Is(no_reg)) {
list.Remove(exclusion);
} else {
list.Combine(padreg);
}
DCHECK_EQ(list.Count() % 2, 0);
PopCPURegList(list);
bytes += list.Count() * kXRegSizeInBits / 8;
......@@ -3350,8 +3378,11 @@ void MacroAssembler::Printf(const char * format,
TmpList()->set_list(0);
FPTmpList()->set_list(0);
// x18 is the platform register and is reserved for the use of platform ABIs.
// It is not part of the kCallerSaved list, but we add it here anyway to
// ensure `reg_list.Count() % 2 == 0` which is required in multiple spots.
CPURegList saved_registers = kCallerSaved;
saved_registers.Align();
saved_registers.Combine(x18.code());
// Preserve all caller-saved registers as well as NZCV.
// PushCPURegList asserts that the size of each list is a multiple of 16
......
......@@ -626,9 +626,6 @@ class V8_EXPORT_PRIVATE CPURegList {
// preparing registers for an AAPCS64 function call, for example.
void RemoveCalleeSaved();
// Align the list to 16 bytes.
void Align();
CPURegister PopLowestIndex();
CPURegister PopHighestIndex();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment