[arm64] Remove x18 from allocatable registers

schuay · Commit Bot · commit 9e9def0631cc · 2019-04-16T12:50:21.000Z
The arm64 ABI defines x18 as a platform register, and as such platforms may reserve it for their own purposes. This CL unconditionally removes x18 from the allocatable register list (previously it was only excluded from arm64 Windows). If, for some reason, we want to keep x18 allocatable on some platforms, we can explicitly enable it for specific platforms in the future. Bug: v8:8940,v8:9140 Change-Id: I28c4f6aad714e21a0a54bab6041c13a1b28fd467 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1564194 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#60870}
diff --git a/src/arm64/assembler-arm64.cc b/src/arm64/assembler-arm64.cc
@@ -109,14 +109,9 @@ CPURegList CPURegList::GetCalleeSavedV(int size) {
 
 
 CPURegList CPURegList::GetCallerSaved(int size) {
-#if defined(V8_OS_WIN)
-  // x18 is reserved as platform register on Windows arm64.
+  // x18 is the platform register and is reserved for the use of platform ABIs.
   // Registers x0-x17 and lr (x30) are caller-saved.
   CPURegList list = CPURegList(CPURegister::kRegister, size, 0, 17);
-#else
-  // Registers x0-x18 and lr (x30) are caller-saved.
-  CPURegList list = CPURegList(CPURegister::kRegister, size, 0, 18);
-#endif
   list.Combine(lr);
   return list;
 }
@@ -149,13 +144,7 @@ CPURegList CPURegList::GetSafepointSavedRegisters() {
   list.Remove(16);
   list.Remove(17);
 
-// Don't add x18 to safepoint list on Windows arm64 because it is reserved
-// as platform register.
-#if !defined(V8_OS_WIN)
-  // Add x18 to the safepoint list, as although it's not in kJSCallerSaved, it
-  // is a caller-saved register according to the procedure call standard.
-  list.Combine(18);
-#endif
+  // x18 is the platform register and is reserved for the use of platform ABIs.
 
   // Add the link register (x30) to the safepoint list.
   list.Combine(30);
diff --git a/src/arm64/deoptimizer-arm64.cc b/src/arm64/deoptimizer-arm64.cc
@@ -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,23 +74,19 @@ void RestoreRegList(MacroAssembler* masm, const CPURegList& reg_list,
   Register src = temps.AcquireX();
   masm->Add(src, src_base, src_offset);
 
-#if defined(V8_OS_WIN)
-  // x18 is reserved as platform register on Windows.
+  // x18 is the platform register and is reserved for the use of platform ABIs.
   restore_list.Remove(x18);
-#endif
 
   // Restore every register in restore_list from src.
   while (!restore_list.IsEmpty()) {
     CPURegister reg0 = restore_list.PopLowestIndex();
     CPURegister reg1 = restore_list.PopLowestIndex();
     int offset0 = reg0.code() * reg_size;
 
-#if defined(V8_OS_WIN)
     if (reg1 == NoCPUReg) {
       masm->Ldr(reg0, MemOperand(src, offset0));
       break;
     }
-#endif
 
     int offset1 = reg1.code() * reg_size;
 
diff --git a/src/arm64/macro-assembler-arm64.cc b/src/arm64/macro-assembler-arm64.cc
@@ -48,20 +48,15 @@ int TurboAssembler::RequiredStackSizeForCallerSaved(SaveFPRegsMode fp_mode,
   // However, we leave it in the argument list to mirror the prototype for
   // Push/PopCallerSaved().
 
-#if defined(V8_OS_WIN)
-  // X18 is excluded from caller-saved register list on Windows ARM64 which
-  // makes caller-saved registers in odd number. padreg is used accordingly
-  // to maintain the alignment.
+  // 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;
   }
-#else
-  DCHECK_EQ(list.Count() % 2, 0);
-  USE(exclusion);
-#endif
 
   bytes += list.Count() * kXRegSizeInBits / 8;
 
@@ -77,21 +72,13 @@ int TurboAssembler::PushCallerSaved(SaveFPRegsMode fp_mode,
   int bytes = 0;
   auto list = kCallerSaved;
 
-#if defined(V8_OS_WIN)
-  // X18 is excluded from caller-saved register list on Windows ARM64, use
-  // padreg accordingly to maintain alignment.
+  // 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);
   }
-#else
-  if (!exclusion.Is(no_reg)) {
-    // Replace the excluded register with padding to maintain alignment.
-    list.Remove(exclusion);
-    list.Combine(padreg);
-  }
-#endif
 
   DCHECK_EQ(list.Count() % 2, 0);
   PushCPURegList(list);
@@ -115,21 +102,13 @@ int TurboAssembler::PopCallerSaved(SaveFPRegsMode fp_mode, Register exclusion) {
 
   auto list = kCallerSaved;
 
-#if defined(V8_OS_WIN)
-  // X18 is excluded from caller-saved register list on Windows ARM64, use
-  // padreg accordingly to maintain alignment.
+  // 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);
   }
-#else
-  if (!exclusion.Is(no_reg)) {
-    // Replace the excluded register with padding to maintain alignment.
-    list.Remove(exclusion);
-    list.Combine(padreg);
-  }
-#endif
 
   DCHECK_EQ(list.Count() % 2, 0);
   PopCPURegList(list);
@@ -3408,14 +3387,20 @@ 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.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
   // bytes.
-  PushCPURegList(kCallerSaved);
+  PushCPURegList(saved_registers);
   PushCPURegList(kCallerSavedV);
 
   // We can use caller-saved registers as scratch values (except for argN).
-  CPURegList tmp_list = kCallerSaved;
+  CPURegList tmp_list = saved_registers;
   CPURegList fp_tmp_list = kCallerSavedV;
   tmp_list.Remove(arg0, arg1, arg2, arg3);
   fp_tmp_list.Remove(arg0, arg1, arg2, arg3);
@@ -3435,7 +3420,8 @@ void MacroAssembler::Printf(const char * format,
       // to PrintfNoPreserve as an argument.
       Register arg_sp = temps.AcquireX();
       Add(arg_sp, sp,
-          kCallerSaved.TotalSizeInBytes() + kCallerSavedV.TotalSizeInBytes());
+          saved_registers.TotalSizeInBytes() +
+              kCallerSavedV.TotalSizeInBytes());
       if (arg0_sp) arg0 = Register::Create(arg_sp.code(), arg0.SizeInBits());
       if (arg1_sp) arg1 = Register::Create(arg_sp.code(), arg1.SizeInBits());
       if (arg2_sp) arg2 = Register::Create(arg_sp.code(), arg2.SizeInBits());
@@ -3460,7 +3446,7 @@ void MacroAssembler::Printf(const char * format,
   }
 
   PopCPURegList(kCallerSavedV);
-  PopCPURegList(kCallerSaved);
+  PopCPURegList(saved_registers);
 
   TmpList()->set_list(old_tmp_list);
   FPTmpList()->set_list(old_fp_tmp_list);
diff --git a/src/arm64/register-arm64.h b/src/arm64/register-arm64.h
@@ -28,20 +28,13 @@ namespace internal {
   R(x16) R(x17) R(x18) R(x19) R(x20) R(x21) R(x22) R(x23) \
   R(x24) R(x25) R(x26) R(x27) R(x28) R(x29) R(x30) R(x31)
 
-#if defined(V8_OS_WIN)
-// x18 is reserved as platform register on Windows ARM64.
+// x18 is the platform register and is reserved for the use of platform ABIs.
+// It is known to be reserved by the OS at least on Windows and iOS.
 #define ALLOCATABLE_GENERAL_REGISTERS(R)                  \
   R(x0)  R(x1)  R(x2)  R(x3)  R(x4)  R(x5)  R(x6)  R(x7)  \
   R(x8)  R(x9)  R(x10) R(x11) R(x12) R(x13) R(x14) R(x15) \
          R(x19) R(x20) R(x21) R(x22) R(x23) R(x24) R(x25) \
   R(x27) R(x28)
-#else
-#define ALLOCATABLE_GENERAL_REGISTERS(R)                  \
-  R(x0)  R(x1)  R(x2)  R(x3)  R(x4)  R(x5)  R(x6)  R(x7)  \
-  R(x8)  R(x9)  R(x10) R(x11) R(x12) R(x13) R(x14) R(x15) \
-  R(x18) R(x19) R(x20) R(x21) R(x22) R(x23) R(x24) R(x25) \
-  R(x27) R(x28)
-#endif
 
 #define FLOAT_REGISTERS(V)                                \
   V(s0)  V(s1)  V(s2)  V(s3)  V(s4)  V(s5)  V(s6)  V(s7)  \
@@ -729,12 +722,7 @@ constexpr Register kJSFunctionRegister = x1;
 constexpr Register kContextRegister = cp;
 constexpr Register kAllocateSizeRegister = x1;
 
-#if defined(V8_OS_WIN)
-// x18 is reserved as platform register on Windows ARM64.
 constexpr Register kSpeculationPoisonRegister = x23;
-#else
-constexpr Register kSpeculationPoisonRegister = x18;
-#endif
 
 constexpr Register kInterpreterAccumulatorRegister = x0;
 constexpr Register kInterpreterBytecodeOffsetRegister = x19;
diff --git a/src/builtins/arm64/builtins-arm64.cc b/src/builtins/arm64/builtins-arm64.cc
@@ -1281,15 +1281,9 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ Mov(
       kInterpreterDispatchTableRegister,
       ExternalReference::interpreter_dispatch_table_address(masm->isolate()));
-#if defined(V8_OS_WIN)
   __ Ldrb(x23, MemOperand(kInterpreterBytecodeArrayRegister,
                           kInterpreterBytecodeOffsetRegister));
   __ Mov(x1, Operand(x23, LSL, kSystemPointerSizeLog2));
-#else
-  __ Ldrb(x18, MemOperand(kInterpreterBytecodeArrayRegister,
-                          kInterpreterBytecodeOffsetRegister));
-  __ Mov(x1, Operand(x18, LSL, kSystemPointerSizeLog2));
-#endif
   __ Ldr(kJavaScriptCallCodeStartRegister,
          MemOperand(kInterpreterDispatchTableRegister, x1));
   __ Call(kJavaScriptCallCodeStartRegister);
@@ -1534,15 +1528,9 @@ static void Generate_InterpreterEnterBytecode(MacroAssembler* masm) {
   __ SmiUntag(kInterpreterBytecodeOffsetRegister);
 
   // Dispatch to the target bytecode.
-#if defined(V8_OS_WIN)
   __ Ldrb(x23, MemOperand(kInterpreterBytecodeArrayRegister,
                           kInterpreterBytecodeOffsetRegister));
   __ Mov(x1, Operand(x23, LSL, kSystemPointerSizeLog2));
-#else
-  __ Ldrb(x18, MemOperand(kInterpreterBytecodeArrayRegister,
-                          kInterpreterBytecodeOffsetRegister));
-  __ Mov(x1, Operand(x18, LSL, kSystemPointerSizeLog2));
-#endif
   __ Ldr(kJavaScriptCallCodeStartRegister,
          MemOperand(kInterpreterDispatchTableRegister, x1));
   __ Jump(kJavaScriptCallCodeStartRegister);
diff --git a/src/wasm/baseline/liftoff-assembler-defs.h b/src/wasm/baseline/liftoff-assembler-defs.h
@@ -60,10 +60,11 @@ constexpr RegList kLiftoffAssemblerFpCacheRegs =
 
 #elif V8_TARGET_ARCH_ARM64
 
-// x16: ip0, x17: ip1, x26: root, x27: cp, x29: fp, x30: lr, x31: xzr.
+// x16: ip0, x17: ip1, x18: platform register, x26: root, x27: cp, x29: fp,
+// x30: lr, x31: xzr.
 constexpr RegList kLiftoffAssemblerGpCacheRegs =
     CPURegister::ListOf<x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12,
-                        x13, x14, x15, x18, x19, x20, x21, x22, x23, x24, x25,
+                        x13, x14, x15, x19, x20, x21, x22, x23, x24, x25,
                         x28>();
 
 // d15: fp_zero, d30-d31: macro-assembler scratch V Registers.
