[sparkplug][x64] Reduce CallRecordWrite instructions

Directly use the correct registers for calling the RecordWrite stubs
in sparkplug. To keep changes to existing builtins minimal there are
certain register requirements which are now made explicit in
WriteBarrierDescriptor::Verify.


Bug: v8:11420
Change-Id: I3a0c500fbe26f82ee2243a61dbf574fd31656982
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2910313Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74998}

src/baseline/baseline-compiler.cc

@@ -713,6 +713,18 @@ void BaselineCompiler::VisitLdaImmutableCurrentContextSlot() {
 }
 
 void BaselineCompiler::VisitStaContextSlot() {
+  // TODO(cbruni): enable on all platforms
+#if V8_TARGET_ARCH_X64
+  Register value = WriteBarrierDescriptor::ValueRegister();
+  Register context = WriteBarrierDescriptor::ObjectRegister();
+  DCHECK(!AreAliased(value, context, kInterpreterAccumulatorRegister));
+  __ Move(value, kInterpreterAccumulatorRegister);
+  LoadRegister(context, 0);
+  int depth = Uint(2);
+  for (; depth > 0; --depth) {
+    __ LoadTaggedPointerField(context, context, Context::kPreviousOffset);
+  }
+#else
   BaselineAssembler::ScratchRegisterScope scratch_scope(&basm_);
   Register context = scratch_scope.AcquireScratch();
   LoadRegister(context, 0);
@@ -722,17 +734,27 @@ void BaselineCompiler::VisitStaContextSlot() {
   }
   Register value = scratch_scope.AcquireScratch();
   __ Move(value, kInterpreterAccumulatorRegister);
+#endif  // V8_TARGET_ARCH_X64
   __ StoreTaggedFieldWithWriteBarrier(
       context, Context::OffsetOfElementAt(iterator().GetIndexOperand(1)),
       value);
 }
 
 void BaselineCompiler::VisitStaCurrentContextSlot() {
+// TODO(cbruni): enable on all platforms
+#if V8_TARGET_ARCH_X64
+  Register value = WriteBarrierDescriptor::ValueRegister();
+  Register context = WriteBarrierDescriptor::ObjectRegister();
+  DCHECK(!AreAliased(value, context, kInterpreterAccumulatorRegister));
+  __ Move(value, kInterpreterAccumulatorRegister);
+  __ LoadContext(context);
+#else
   BaselineAssembler::ScratchRegisterScope scratch_scope(&basm_);
   Register context = scratch_scope.AcquireScratch();
   __ LoadContext(context);
   Register value = scratch_scope.AcquireScratch();
   __ Move(value, kInterpreterAccumulatorRegister);
+#endif  // V8_TARGET_ARCH_X64
   __ StoreTaggedFieldWithWriteBarrier(
       context, Context::OffsetOfElementAt(Index(0)), value);
 }
@@ -857,6 +879,34 @@ void BaselineCompiler::VisitLdaModuleVariable() {
 }
 
 void BaselineCompiler::VisitStaModuleVariable() {
+// TODO(cbruni): enable on all platforms
+#if V8_TARGET_ARCH_X64
+  int cell_index = Int(0);
+  if (V8_UNLIKELY(cell_index < 0)) {
+    // Not supported (probably never).
+    CallRuntime(Runtime::kAbort,
+                Smi::FromInt(static_cast<int>(
+                    AbortReason::kUnsupportedModuleOperation)));
+    __ Trap();
+  }
+  Register value = WriteBarrierDescriptor::ValueRegister();
+  Register scratch = WriteBarrierDescriptor::ObjectRegister();
+  DCHECK(!AreAliased(value, scratch, kInterpreterAccumulatorRegister));
+  __ Move(value, kInterpreterAccumulatorRegister);
+  __ LoadContext(scratch);
+  int depth = Uint(1);
+  for (; depth > 0; --depth) {
+    __ LoadTaggedPointerField(scratch, scratch, Context::kPreviousOffset);
+  }
+  __ LoadTaggedPointerField(scratch, scratch, Context::kExtensionOffset);
+  __ LoadTaggedPointerField(scratch, scratch,
+                            SourceTextModule::kRegularExportsOffset);
+
+  // The actual array index is (cell_index - 1).
+  cell_index -= 1;
+  __ LoadFixedArrayElement(scratch, scratch, cell_index);
+  __ StoreTaggedFieldWithWriteBarrier(scratch, Cell::kValueOffset, value);
+#else   // V8_TARGET_ARCH_X64
   BaselineAssembler::ScratchRegisterScope scratch_scope(&basm_);
   Register scratch = scratch_scope.AcquireScratch();
   __ LoadContext(scratch);
@@ -882,6 +932,7 @@ void BaselineCompiler::VisitStaModuleVariable() {
                     AbortReason::kUnsupportedModuleOperation)));
     __ Trap();
   }
+#endif  // V8_TARGET_ARCH_X64
 }
 
 void BaselineCompiler::VisitStaNamedProperty() {

src/baseline/x64/baseline-assembler-x64-inl.h

@@ -318,12 +318,9 @@ void BaselineAssembler::StoreTaggedSignedField(Register target, int offset,
 }
 void BaselineAssembler::StoreTaggedFieldWithWriteBarrier(Register target,
                                                          int offset,
-
                                                          Register value) {
-  BaselineAssembler::ScratchRegisterScope scratch_scope(this);
-  Register scratch = scratch_scope.AcquireScratch();
-  DCHECK_NE(target, scratch);
-  DCHECK_NE(value, scratch);
+  Register scratch = WriteBarrierDescriptor::SlotAddressRegister();
+  DCHECK(!AreAliased(target, value, scratch));
   __ StoreTaggedField(FieldOperand(target, offset), value);
   __ RecordWriteField(target, offset, value, scratch, SaveFPRegsMode::kIgnore);
 }

src/builtins/x64/builtins-x64.cc

@@ -686,7 +686,10 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) {
   // Store input value into generator object.
   __ StoreTaggedField(
       FieldOperand(rdx, JSGeneratorObject::kInputOrDebugPosOffset), rax);
-  __ RecordWriteField(rdx, JSGeneratorObject::kInputOrDebugPosOffset, rax, rcx,
+  Register object = WriteBarrierDescriptor::ObjectRegister();
+  __ Move(object, rdx);
+  __ RecordWriteField(object, JSGeneratorObject::kInputOrDebugPosOffset, rax,
+                      WriteBarrierDescriptor::SlotAddressRegister(),
                       SaveFPRegsMode::kIgnore);
 
   Register decompr_scratch1 = COMPRESS_POINTERS_BOOL ? r8 : no_reg;
@@ -1086,7 +1089,8 @@ static void MaybeOptimizeCodeOrTailCallOptimizedCodeSlot(
   __ LoadAnyTaggedField(
       optimized_code_entry,
       FieldOperand(feedback_vector, FeedbackVector::kMaybeOptimizedCodeOffset));
-  TailCallOptimizedCodeSlot(masm, optimized_code_entry, closure, r8, r15,
+  TailCallOptimizedCodeSlot(masm, optimized_code_entry, closure, r9,
+                            WriteBarrierDescriptor::SlotAddressRegister(),
                             jump_mode);
 }
 
@@ -1327,9 +1331,9 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
                               FieldOperand(kInterpreterBytecodeArrayRegister,
                                            BaselineData::kBaselineCodeOffset));
     static_assert(kJavaScriptCallCodeStartRegister == rcx, "ABI mismatch");
-    ReplaceClosureCodeWithOptimizedCode(masm, rcx, closure,
-                                        kInterpreterBytecodeArrayRegister,
-                                        kInterpreterBytecodeOffsetRegister);
+    ReplaceClosureCodeWithOptimizedCode(
+        masm, rcx, closure, kInterpreterBytecodeArrayRegister,
+        WriteBarrierDescriptor::SlotAddressRegister());
     __ JumpCodeObject(rcx);
 
     __ bind(&install_baseline_code);
@@ -1856,7 +1860,8 @@ void Builtins::Generate_NotifyDeoptimized(MacroAssembler* masm) {
 void Builtins::Generate_TailCallOptimizedCodeSlot(MacroAssembler* masm) {
   Register optimized_code_entry = kJavaScriptCallCodeStartRegister;
   Register closure = kJSFunctionRegister;
-  TailCallOptimizedCodeSlot(masm, optimized_code_entry, closure, r8, r15,
+  TailCallOptimizedCodeSlot(masm, optimized_code_entry, closure, r9,
+                            WriteBarrierDescriptor::SlotAddressRegister(),
                             JumpMode::kJump);
 }
 

src/codegen/arm/interface-descriptors-arm-inl.h

@@ -19,6 +19,23 @@ constexpr auto CallInterfaceDescriptor::DefaultRegisterArray() {
   return registers;
 }
 
+#if DEBUG
+template <typename DerivedDescriptor>
+void StaticCallInterfaceDescriptor<DerivedDescriptor>::
+    VerifyArgumentRegisterCount(CallInterfaceDescriptorData* data, int argc) {
+  RegList allocatable_regs = data->allocatable_registers();
+  if (argc >= 1) DCHECK(allocatable_regs | r0.bit());
+  if (argc >= 2) DCHECK(allocatable_regs | r1.bit());
+  if (argc >= 3) DCHECK(allocatable_regs | r2.bit());
+  if (argc >= 4) DCHECK(allocatable_regs | r3.bit());
+  if (argc >= 5) DCHECK(allocatable_regs | r4.bit());
+  if (argc >= 6) DCHECK(allocatable_regs | r5.bit());
+  if (argc >= 7) DCHECK(allocatable_regs | r6.bit());
+  if (argc >= 8) DCHECK(allocatable_regs | r7.bit());
+  // Additional arguments are passed on the stack.
+}
+#endif  // DEBUG
+
 // static
 constexpr auto WriteBarrierDescriptor::registers() {
   STATIC_ASSERT(kReturnRegister0 == r0);

src/codegen/arm64/interface-descriptors-arm64-inl.h

@@ -20,6 +20,22 @@ constexpr auto CallInterfaceDescriptor::DefaultRegisterArray() {
   return registers;
 }
 
+#if DEBUG
+template <typename DerivedDescriptor>
+void StaticCallInterfaceDescriptor<DerivedDescriptor>::
+    VerifyArgumentRegisterCount(CallInterfaceDescriptorData* data, int argc) {
+  RegList allocatable_regs = data->allocatable_registers();
+  if (argc >= 1) DCHECK(allocatable_regs | x0.bit());
+  if (argc >= 2) DCHECK(allocatable_regs | x1.bit());
+  if (argc >= 3) DCHECK(allocatable_regs | x2.bit());
+  if (argc >= 4) DCHECK(allocatable_regs | x3.bit());
+  if (argc >= 5) DCHECK(allocatable_regs | x4.bit());
+  if (argc >= 6) DCHECK(allocatable_regs | x5.bit());
+  if (argc >= 7) DCHECK(allocatable_regs | x6.bit());
+  if (argc >= 8) DCHECK(allocatable_regs | x7.bit());
+}
+#endif  // DEBUG
+
 // static
 constexpr auto WriteBarrierDescriptor::registers() {
   STATIC_ASSERT(kReturnRegister0 == x0);

src/codegen/ia32/interface-descriptors-ia32-inl.h

@@ -18,6 +18,18 @@ constexpr auto CallInterfaceDescriptor::DefaultRegisterArray() {
   return registers;
 }
 
+#if DEBUG
+template <typename DerivedDescriptor>
+void StaticCallInterfaceDescriptor<DerivedDescriptor>::
+    VerifyArgumentRegisterCount(CallInterfaceDescriptorData* data,
+                                int nof_expected_args) {
+  RegList allocatable_regs = data->allocatable_registers();
+  if (nof_expected_args >= 1) DCHECK(allocatable_regs | esi.bit());
+  if (nof_expected_args >= 2) DCHECK(allocatable_regs | edi.bit());
+  // Additional arguments are passed on the stack.
+}
+#endif  // DEBUG
+
 // static
 constexpr auto WriteBarrierDescriptor::registers() {
   STATIC_ASSERT(esi == kContextRegister);

src/codegen/interface-descriptors-inl.h

@@ -84,8 +84,10 @@ void StaticCallInterfaceDescriptor<DerivedDescriptor>::Initialize(
 
   DCHECK(data->IsInitialized());
   DCHECK(this->CheckFloatingPointParameters(data));
+#if DEBUG
+  DerivedDescriptor::Verify(data);
+#endif
 }
-
 // static
 template <typename DerivedDescriptor>
 constexpr int

src/codegen/interface-descriptors.cc

@@ -125,5 +125,33 @@ bool CallInterfaceDescriptor::IsValidFloatParameterRegister(Register reg) {
 #endif
 }
 
+#if DEBUG
+template <typename DerivedDescriptor>
+void StaticCallInterfaceDescriptor<DerivedDescriptor>::Verify(
+    CallInterfaceDescriptorData* data) {}
+// static
+void WriteBarrierDescriptor::Verify(CallInterfaceDescriptorData* data) {
+  DCHECK(!AreAliased(ObjectRegister(), SlotAddressRegister(), ValueRegister()));
+// TODO(cbruni): enable on all platforms.
+#if V8_TARGET_ARCH_X64
+  // The default parameters should not clobber vital registers in order to
+  // reduce code size:
+  DCHECK(!AreAliased(ObjectRegister(), kContextRegister,
+                     kInterpreterAccumulatorRegister));
+  DCHECK(!AreAliased(SlotAddressRegister(), kContextRegister,
+                     kInterpreterAccumulatorRegister));
+  DCHECK(!AreAliased(ValueRegister(), kContextRegister,
+                     kInterpreterAccumulatorRegister));
+  // Coincidental: to make calling from various builtins easier.
+  DCHECK_EQ(ObjectRegister(), kJSFunctionRegister);
+#endif
+  // We need a certain set of registers by default:
+  RegList allocatable_regs = data->allocatable_registers();
+  DCHECK(allocatable_regs | kContextRegister.bit());
+  DCHECK(allocatable_regs | kReturnRegister0.bit());
+  VerifyArgumentRegisterCount(data, 4);
+}
+#endif  // DEBUG
+
 }  // namespace internal
 }  // namespace v8

src/codegen/interface-descriptors.h

@@ -473,10 +473,18 @@ class StaticCallInterfaceDescriptor : public CallInterfaceDescriptor {
   static constexpr inline Register GetRegisterParameter(int i) {
     return DerivedDescriptor::registers()[i];
   }
-
   explicit StaticCallInterfaceDescriptor(CallDescriptors::Key key)
       : CallInterfaceDescriptor(key) {}
 
+#if DEBUG
+  // Overwritten in DerivedDescriptor.
+  static void Verify(CallInterfaceDescriptorData* data);
+  // Verify that the CallInterfaceDescriptorData contains the default
+  // argument registers for {argc} arguments.
+  static inline void VerifyArgumentRegisterCount(
+      CallInterfaceDescriptorData* data, int nof_expected_args);
+#endif
+
  private:
   // {CallDescriptors} is allowed to call the private {Initialize} method.
   friend class CallDescriptors;
@@ -1025,6 +1033,9 @@ class WriteBarrierDescriptor final
   static constexpr inline Register ValueRegister();
   static constexpr inline RegList ComputeSavedRegisters(
       Register object, Register slot_address = no_reg);
+#if DEBUG
+  static void Verify(CallInterfaceDescriptorData* data);
+#endif
 };
 
 #ifdef V8_IS_TSAN

src/codegen/x64/interface-descriptors-x64-inl.h

@@ -18,10 +18,27 @@ constexpr auto CallInterfaceDescriptor::DefaultRegisterArray() {
   return registers;
 }
 
+#if DEBUG
+template <typename DerivedDescriptor>
+void StaticCallInterfaceDescriptor<DerivedDescriptor>::
+    VerifyArgumentRegisterCount(CallInterfaceDescriptorData* data,
+                                int nof_expected_args) {
+  RegList allocatable_regs = data->allocatable_registers();
+  if (nof_expected_args >= 1) DCHECK(allocatable_regs | arg_reg_1.bit());
+  if (nof_expected_args >= 2) DCHECK(allocatable_regs | arg_reg_2.bit());
+  if (nof_expected_args >= 3) DCHECK(allocatable_regs | arg_reg_3.bit());
+  if (nof_expected_args >= 4) DCHECK(allocatable_regs | arg_reg_4.bit());
+  // Additional arguments are passed on the stack.
+}
+#endif  // DEBUG
+
 // static
 constexpr auto WriteBarrierDescriptor::registers() {
-  return RegisterArray(arg_reg_1, arg_reg_2, arg_reg_3, arg_reg_4,
-                       kReturnRegister0);
+#if V8_TARGET_OS_WIN
+  return RegisterArray(rdi, r8, rcx, rax, r9, rdx, rsi);
+#else
+  return RegisterArray(rdi, rbx, rdx, rcx, rax, rsi);
+#endif  // V8_TARGET_OS_WIN
 }
 
 #ifdef V8_IS_TSAN

src/codegen/x64/macro-assembler-x64.cc

@@ -553,8 +553,7 @@ void MacroAssembler::RecordWrite(Register object, Register slot_address,
                 MemoryChunk::kPointersFromHereAreInterestingMask, zero, &done,
                 Label::kNear);
 
-  CallRecordWriteStubSaveRegisters(object, slot_address, remembered_set_action,
-                                   fp_mode);
+  CallRecordWriteStub(object, slot_address, remembered_set_action, fp_mode);
 
   bind(&done);