[compiler] Add StackOrder to CallInterfaceDescriptor

This CL is a step towards reversing JS stack arguments for TurboFan.

It does the following:
1. Add StackOrder to CallInterfaceDescriptor
2. Reverse arguments in TF backend for JS calls.
3. Cleanup TFJ builtins interface descriptors, since calls for these builtins already reverse the arguments, we don't need to reverse the interface descriptor anymore.

Change-Id: Ie840b1757bf023aa381a7fa01cbe66e7cf90778f
Bug: v8:10201
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2213440Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67971}

src/builtins/builtins-descriptors.h

@@ -13,34 +13,7 @@
 namespace v8 {
 namespace internal {
 
-#define REVERSE_0(a) a,
-#define REVERSE_1(a, b) b, a,
-#define REVERSE_2(a, b, c) c, b, a,
-#define REVERSE_3(a, b, c, d) d, c, b, a,
-#define REVERSE_4(a, b, c, d, e) e, d, c, b, a,
-#define REVERSE_5(a, b, c, d, e, f) f, e, d, c, b, a,
-#define REVERSE_6(a, b, c, d, e, f, g) g, f, e, d, c, b, a,
-#define REVERSE_7(a, b, c, d, e, f, g, h) h, g, f, e, d, c, b, a,
-#define REVERSE_8(a, b, c, d, e, f, g, h, i) i, h, g, f, e, d, c, b, a,
-#define REVERSE_kDontAdaptArgumentsSentinel(...)
-#define REVERSE(N, ...) REVERSE_##N(__VA_ARGS__)
-
 // Define interface descriptors for builtins with JS linkage.
-#ifdef V8_REVERSE_JSARGS
-#define DEFINE_TFJ_INTERFACE_DESCRIPTOR(Name, Argc, ...)                \
-  struct Builtin_##Name##_InterfaceDescriptor {                         \
-    enum ParameterIndices {                                             \
-      kJSTarget = compiler::CodeAssembler::kTargetParameterIndex,       \
-      REVERSE_##Argc(__VA_ARGS__) kJSNewTarget,                         \
-      kJSActualArgumentsCount,                                          \
-      kContext,                                                         \
-      kParameterCount,                                                  \
-    };                                                                  \
-    static_assert((Argc) == static_cast<uint16_t>(kParameterCount - 4), \
-                  "Inconsistent set of arguments");                     \
-    static_assert(kJSTarget == -1, "Unexpected kJSTarget index value"); \
-  };
-#else
 #define DEFINE_TFJ_INTERFACE_DESCRIPTOR(Name, Argc, ...)                \
   struct Builtin_##Name##_InterfaceDescriptor {                         \
     enum ParameterIndices {                                             \
@@ -55,7 +28,6 @@ namespace internal {
                   "Inconsistent set of arguments");                     \
     static_assert(kJSTarget == -1, "Unexpected kJSTarget index value"); \
   };
-#endif
 
 // Define interface descriptors for builtins with StubCall linkage.
 #define DEFINE_TFC_INTERFACE_DESCRIPTOR(Name, InterfaceDescriptor) \

src/codegen/code-stub-assembler.cc

@@ -13025,17 +13025,9 @@ TNode<Object> CodeStubAssembler::CallApiCallback(
     TNode<Object> context, TNode<RawPtrT> callback, TNode<IntPtrT> argc,
     TNode<Object> data, TNode<Object> holder, TNode<Object> receiver,
     TNode<Object> value) {
-  // CallApiCallback receives the first four arguments in registers
-  // (callback, argc, data and holder). The last arguments are in the stack in
-  // JS ordering. See ApiCallbackDescriptor.
   Callable callable = CodeFactory::CallApiCallback(isolate());
-#ifdef V8_REVERSE_JSARGS
-  return CallStub(callable, context, callback, argc, data, holder, value,
-                  receiver);
-#else
   return CallStub(callable, context, callback, argc, data, holder, receiver,
                   value);
-#endif
 }
 
 TNode<Object> CodeStubAssembler::CallRuntimeNewArray(

src/codegen/interface-descriptors.cc

@@ -30,10 +30,12 @@ void CallInterfaceDescriptorData::InitializePlatformSpecific(
 
 void CallInterfaceDescriptorData::InitializePlatformIndependent(
     Flags flags, int return_count, int parameter_count,
-    const MachineType* machine_types, int machine_types_length) {
+    const MachineType* machine_types, int machine_types_length,
+    StackArgumentOrder stack_order) {
   DCHECK(IsInitializedPlatformSpecific());
 
   flags_ = flags;
+  stack_order_ = stack_order;
   return_count_ = return_count;
   param_count_ = parameter_count;
   const int types_length = return_count_ + param_count_;

src/codegen/interface-descriptors.h

@@ -105,6 +105,16 @@ namespace internal {
   BUILTIN_LIST_TFS(V)                    \
   TORQUE_BUILTIN_LIST_TFC(V)
 
+enum class StackArgumentOrder {
+  kDefault,  // Arguments in the stack are pushed in the default/stub order (the
+             // first argument is pushed first).
+  kJS,  // Arguments in the stack are pushed in the same order as the one used
+        // by JS-to-JS function calls. This should be used if calling a
+        // JSFunction or if the builtin is expected to be called directly from a
+        // JSFunction. When V8_REVERSE_JSARGS is set, this order is reversed
+        // compared to kDefault.
+};
+
 class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
  public:
   enum Flag {
@@ -140,7 +150,8 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
   void InitializePlatformIndependent(Flags flags, int return_count,
                                      int parameter_count,
                                      const MachineType* machine_types,
-                                     int machine_types_length);
+                                     int machine_types_length,
+                                     StackArgumentOrder stack_order);
 
   void Reset();
 
@@ -163,6 +174,7 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
     DCHECK_LT(index, param_count_);
     return machine_types_[return_count_ + index];
   }
+  StackArgumentOrder stack_order() const { return stack_order_; }
 
   void RestrictAllocatableRegisters(const Register* registers, int num) {
     DCHECK_EQ(allocatable_registers_, 0);
@@ -197,6 +209,7 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
   int return_count_ = -1;
   int param_count_ = -1;
   Flags flags_ = kNoFlags;
+  StackArgumentOrder stack_order_ = StackArgumentOrder::kDefault;
 
   // Specifying the set of registers that could be used by the register
   // allocator. Currently, it's only used by RecordWrite code stub.
@@ -293,6 +306,10 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptor {
     return data()->allocatable_registers();
   }
 
+  StackArgumentOrder GetStackArgumentOrder() const {
+    return data()->stack_order();
+  }
+
   static const Register ContextRegister();
 
   const char* DebugName() const;
@@ -312,9 +329,9 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptor {
       CallInterfaceDescriptorData* data) {
     // Default descriptor configuration: one result, all parameters are passed
     // in registers and all parameters have MachineType::AnyTagged() type.
-    data->InitializePlatformIndependent(CallInterfaceDescriptorData::kNoFlags,
-                                        1, data->register_param_count(),
-                                        nullptr, 0);
+    data->InitializePlatformIndependent(
+        CallInterfaceDescriptorData::kNoFlags, 1, data->register_param_count(),
+        nullptr, 0, StackArgumentOrder::kDefault);
   }
 
   // Initializes |data| using the platform dependent default set of registers.
@@ -400,7 +417,8 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
   void InitializePlatformIndependent(CallInterfaceDescriptorData* data)        \
       override {                                                               \
     data->InitializePlatformIndependent(Flags(kDescriptorFlags), kReturnCount, \
-                                        kParameterCount, nullptr, 0);          \
+                                        kParameterCount, nullptr, 0,           \
+                                        kStackArgumentOrder);                  \
   }                                                                            \
   name(CallDescriptors::Key key) : base(key) {}                                \
                                                                                \
@@ -418,9 +436,11 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
                                                                             \
  public:
 
-#define DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(flags, return_count, ...) \
+#define DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(flags, stack_order,       \
+                                               return_count, ...)        \
   static constexpr int kDescriptorFlags = flags;                         \
   static constexpr int kReturnCount = return_count;                      \
+  static constexpr StackArgumentOrder kStackArgumentOrder = stack_order; \
   enum ParameterIndices {                                                \
     __dummy = -1, /* to be able to pass zero arguments */                \
     ##__VA_ARGS__,                                                       \
@@ -429,35 +449,41 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
     kContext = kParameterCount /* implicit parameter */                  \
   };
 
-#define DEFINE_RESULT_AND_PARAMETERS(return_count, ...) \
-  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(               \
-      CallInterfaceDescriptorData::kNoFlags, return_count, ##__VA_ARGS__)
+#define DEFINE_RESULT_AND_PARAMETERS(return_count, ...)                    \
+  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(                                  \
+      CallInterfaceDescriptorData::kNoFlags, StackArgumentOrder::kDefault, \
+      return_count, ##__VA_ARGS__)
 
 // This is valid only for builtins that use EntryFrame, which does not scan
 // stack arguments on GC.
-#define DEFINE_PARAMETERS_ENTRY(...)                      \
-  static constexpr int kDescriptorFlags =                 \
-      CallInterfaceDescriptorData::kNoContext |           \
-      CallInterfaceDescriptorData::kNoStackScan;          \
-  static constexpr int kReturnCount = 1;                  \
-  enum ParameterIndices {                                 \
-    __dummy = -1, /* to be able to pass zero arguments */ \
-    ##__VA_ARGS__,                                        \
-                                                          \
-    kParameterCount                                       \
+#define DEFINE_PARAMETERS_ENTRY(...)                        \
+  static constexpr int kDescriptorFlags =                   \
+      CallInterfaceDescriptorData::kNoContext |             \
+      CallInterfaceDescriptorData::kNoStackScan;            \
+  static constexpr StackArgumentOrder kStackArgumentOrder = \
+      StackArgumentOrder::kDefault;                         \
+  static constexpr int kReturnCount = 1;                    \
+  enum ParameterIndices {                                   \
+    __dummy = -1, /* to be able to pass zero arguments */   \
+    ##__VA_ARGS__,                                          \
+                                                            \
+    kParameterCount                                         \
   };
 
-#define DEFINE_PARAMETERS(...)            \
-  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
-      CallInterfaceDescriptorData::kNoFlags, 1, ##__VA_ARGS__)
+#define DEFINE_PARAMETERS(...)                                                \
+  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(                                     \
+      CallInterfaceDescriptorData::kNoFlags, StackArgumentOrder::kDefault, 1, \
+      ##__VA_ARGS__)
 
-#define DEFINE_PARAMETERS_NO_CONTEXT(...) \
-  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
-      CallInterfaceDescriptorData::kNoContext, 1, ##__VA_ARGS__)
+#define DEFINE_PARAMETERS_NO_CONTEXT(...)                                    \
+  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(                                    \
+      CallInterfaceDescriptorData::kNoContext, StackArgumentOrder::kDefault, \
+      1, ##__VA_ARGS__)
 
-#define DEFINE_PARAMETERS_VARARGS(...)    \
-  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
-      CallInterfaceDescriptorData::kAllowVarArgs, 1, ##__VA_ARGS__)
+#define DEFINE_PARAMETERS_VARARGS(...)                                        \
+  DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(                                     \
+      CallInterfaceDescriptorData::kAllowVarArgs, StackArgumentOrder::kJS, 1, \
+      ##__VA_ARGS__)
 
 #define DEFINE_RESULT_AND_PARAMETER_TYPES_WITH_FLAG(flag, ...)                \
   void InitializePlatformIndependent(CallInterfaceDescriptorData* data)       \
@@ -468,7 +494,7 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
         "Parameter names definition is not consistent with parameter types"); \
     data->InitializePlatformIndependent(                                      \
         Flags(flag | kDescriptorFlags), kReturnCount, kParameterCount,        \
-        machine_types, arraysize(machine_types));                             \
+        machine_types, arraysize(machine_types), kStackArgumentOrder);        \
   }
 
 #define DEFINE_RESULT_AND_PARAMETER_TYPES(...) \
@@ -479,18 +505,20 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
   DEFINE_RESULT_AND_PARAMETER_TYPES(MachineType::AnyTagged() /* result */, \
                                     ##__VA_ARGS__)
 
-#define DEFINE_JS_PARAMETERS(...)                       \
-  static constexpr int kDescriptorFlags =               \
-      CallInterfaceDescriptorData::kAllowVarArgs;       \
-  static constexpr int kReturnCount = 1;                \
-  enum ParameterIndices {                               \
-    kTarget,                                            \
-    kNewTarget,                                         \
-    kActualArgumentsCount,                              \
-    ##__VA_ARGS__,                                      \
-                                                        \
-    kParameterCount,                                    \
-    kContext = kParameterCount /* implicit parameter */ \
+#define DEFINE_JS_PARAMETERS(...)                           \
+  static constexpr int kDescriptorFlags =                   \
+      CallInterfaceDescriptorData::kAllowVarArgs;           \
+  static constexpr int kReturnCount = 1;                    \
+  static constexpr StackArgumentOrder kStackArgumentOrder = \
+      StackArgumentOrder::kJS;                              \
+  enum ParameterIndices {                                   \
+    kTarget,                                                \
+    kNewTarget,                                             \
+    kActualArgumentsCount,                                  \
+    ##__VA_ARGS__,                                          \
+                                                            \
+    kParameterCount,                                        \
+    kContext = kParameterCount /* implicit parameter */     \
   };
 
 #define DEFINE_JS_PARAMETER_TYPES(...)                                         \
@@ -552,7 +580,8 @@ class TorqueInterfaceDescriptor : public CallInterfaceDescriptor {
     DCHECK_EQ(kReturnCount + kParameterCount, machine_types.size());
     data->InitializePlatformIndependent(Flags(kDescriptorFlags), kReturnCount,
                                         kParameterCount, machine_types.data(),
-                                        static_cast<int>(machine_types.size()));
+                                        static_cast<int>(machine_types.size()),
+                                        StackArgumentOrder::kDefault);
   }
 };
 

src/compiler/backend/instruction-selector.cc

@@ -1043,7 +1043,8 @@ void InstructionSelector::InitializeCallBuffer(Node* call, CallBuffer* buffer,
     InstructionOperand op = g.UseLocation(*iter, location);
     UnallocatedOperand unallocated = UnallocatedOperand::cast(op);
     if (unallocated.HasFixedSlotPolicy() && !call_tail) {
-      int stack_index = -unallocated.fixed_slot_index() - 1;
+      int stack_index = buffer->descriptor->GetStackIndexFromSlot(
+          unallocated.fixed_slot_index());
       // This can insert empty slots before stack_index and will insert enough
       // slots after stack_index to store the parameter.
       if (static_cast<size_t>(stack_index) >= buffer->pushed_nodes.size()) {

src/compiler/code-assembler.cc

@@ -1027,11 +1027,7 @@ Node* CodeAssembler::CallJSStubImpl(const CallInterfaceDescriptor& descriptor,
     inputs.Add(new_target);
   }
   inputs.Add(arity);
-#ifdef V8_REVERSE_JSARGS
-  for (auto arg : base::Reversed(args)) inputs.Add(arg);
-#else
   for (auto arg : args) inputs.Add(arg);
-#endif
   if (descriptor.HasContextParameter()) {
     inputs.Add(context);
   }

src/compiler/linkage.cc

@@ -325,7 +325,11 @@ CallDescriptor* Linkage::GetJSCallDescriptor(Zone* zone, bool is_osr,
 
   // All parameters to JS calls go on the stack.
   for (int i = 0; i < js_parameter_count; i++) {
+#ifdef V8_REVERSE_JSARGS
+    int spill_slot_index = -i - 1;
+#else
     int spill_slot_index = i - js_parameter_count;
+#endif
     locations.AddParam(LinkageLocation::ForCallerFrameSlot(
         spill_slot_index, MachineType::AnyTagged()));
   }
@@ -358,7 +362,8 @@ CallDescriptor* Linkage::GetJSCallDescriptor(Zone* zone, bool is_osr,
       kNoCalleeSaved,                   // callee-saved
       kNoCalleeSaved,                   // callee-saved fp
       flags,                            // flags
-      "js-call");
+      "js-call",                        // debug name
+      StackArgumentOrder::kJS);         // stack order
 }
 
 // TODO(turbofan): cache call descriptors for code stub calls.
@@ -458,6 +463,7 @@ CallDescriptor* Linkage::GetStubCallDescriptor(
       kNoCalleeSaved,                        // callee-saved fp
       CallDescriptor::kCanUseRoots | flags,  // flags
       descriptor.DebugName(),                // debug name
+      descriptor.GetStackArgumentOrder(),    // stack order
       descriptor.allocatable_registers());
 }
 

src/compiler/linkage.h

@@ -237,6 +237,7 @@ class V8_EXPORT_PRIVATE CallDescriptor final
                  RegList callee_saved_registers,
                  RegList callee_saved_fp_registers, Flags flags,
                  const char* debug_name = "",
+                 StackArgumentOrder stack_order = StackArgumentOrder::kDefault,
                  const RegList allocatable_registers = 0,
                  size_t stack_return_count = 0)
       : kind_(kind),
@@ -250,6 +251,7 @@ class V8_EXPORT_PRIVATE CallDescriptor final
         callee_saved_fp_registers_(callee_saved_fp_registers),
         allocatable_registers_(allocatable_registers),
         flags_(flags),
+        stack_order_(stack_order),
         debug_name_(debug_name) {}
 
   // Returns the kind of this call.
@@ -292,6 +294,19 @@ class V8_EXPORT_PRIVATE CallDescriptor final
     return stack_param_count_;
   }
 
+  int GetStackIndexFromSlot(int slot_index) const {
+#ifdef V8_REVERSE_JSARGS
+    switch (GetStackArgumentOrder()) {
+      case StackArgumentOrder::kDefault:
+        return -slot_index - 1;
+      case StackArgumentOrder::kJS:
+        return slot_index + static_cast<int>(StackParameterCount());
+    }
+#else
+    return -slot_index - 1;
+#endif
+  }
+
   // The total number of inputs to this call, which includes the target,
   // receiver, context, etc.
   // TODO(titzer): this should input the framestate input too.
@@ -338,6 +353,8 @@ class V8_EXPORT_PRIVATE CallDescriptor final
     return location_sig_->GetParam(index).GetType();
   }
 
+  StackArgumentOrder GetStackArgumentOrder() const { return stack_order_; }
+
   // Operator properties describe how this call can be optimized, if at all.
   Operator::Properties properties() const { return properties_; }
 
@@ -391,6 +408,7 @@ class V8_EXPORT_PRIVATE CallDescriptor final
   // register allocator to use.
   const RegList allocatable_registers_;
   const Flags flags_;
+  const StackArgumentOrder stack_order_;
   const char* const debug_name_;
   const CFunctionInfo* c_function_info_ = nullptr;
 

src/compiler/wasm-compiler.cc

@@ -7012,18 +7012,19 @@ CallDescriptor* GetWasmCallDescriptor(
 
   CallDescriptor::Flags flags =
       use_retpoline ? CallDescriptor::kRetpoline : CallDescriptor::kNoFlags;
-  return new (zone) CallDescriptor(             // --
-      descriptor_kind,                          // kind
-      target_type,                              // target MachineType
-      target_loc,                               // target location
-      locations.Build(),                        // location_sig
-      parameter_slots,                          // stack_parameter_count
-      compiler::Operator::kNoProperties,        // properties
-      kCalleeSaveRegisters,                     // callee-saved registers
-      kCalleeSaveFPRegisters,                   // callee-saved fp regs
-      flags,                                    // flags
-      "wasm-call",                              // debug name
-      0,                                        // allocatable registers
+  return new (zone) CallDescriptor(       // --
+      descriptor_kind,                    // kind
+      target_type,                        // target MachineType
+      target_loc,                         // target location
+      locations.Build(),                  // location_sig
+      parameter_slots,                    // stack_parameter_count
+      compiler::Operator::kNoProperties,  // properties
+      kCalleeSaveRegisters,               // callee-saved registers
+      kCalleeSaveFPRegisters,             // callee-saved fp regs
+      flags,                              // flags
+      "wasm-call",                        // debug name
+      StackArgumentOrder::kDefault,       // order of the arguments in the stack
+      0,                                  // allocatable registers
       rets.NumStackSlots() - parameter_slots);  // stack_return_count
 }
 
@@ -7101,6 +7102,7 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith(
       call_descriptor->CalleeSavedFPRegisters(),       // callee-saved fp regs
       call_descriptor->flags(),                        // flags
       call_descriptor->debug_name(),                   // debug name
+      call_descriptor->GetStackArgumentOrder(),        // stack order
       call_descriptor->AllocatableRegisters(),         // allocatable registers
       rets.NumStackSlots() - params.NumStackSlots());  // stack_return_count
 }