Skip to content

Commit 5337e2a

Browse files
victorgomesCommit Bot
victorgomes
authored and
Commit Bot
committed
[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/+/2213440 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Victor Gomes <victorgomes@chromium.org> Cr-Commit-Position: refs/heads/master@{#67971}
1 parent a35d0e8 commit 5337e2a

9 files changed

Lines changed: 115 additions & 97 deletions

File tree

‎src/builtins/builtins-descriptors.h‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,34 +13,7 @@
1313
namespace v8 {
1414
namespace internal {
1515

16-
#define REVERSE_0(a) a,
17-
#define REVERSE_1(a, b) b, a,
18-
#define REVERSE_2(a, b, c) c, b, a,
19-
#define REVERSE_3(a, b, c, d) d, c, b, a,
20-
#define REVERSE_4(a, b, c, d, e) e, d, c, b, a,
21-
#define REVERSE_5(a, b, c, d, e, f) f, e, d, c, b, a,
22-
#define REVERSE_6(a, b, c, d, e, f, g) g, f, e, d, c, b, a,
23-
#define REVERSE_7(a, b, c, d, e, f, g, h) h, g, f, e, d, c, b, a,
24-
#define REVERSE_8(a, b, c, d, e, f, g, h, i) i, h, g, f, e, d, c, b, a,
25-
#define REVERSE_kDontAdaptArgumentsSentinel(...)
26-
#define REVERSE(N, ...) REVERSE_##N(__VA_ARGS__)
27-
2816
// Define interface descriptors for builtins with JS linkage.
29-
#ifdef V8_REVERSE_JSARGS
30-
#define DEFINE_TFJ_INTERFACE_DESCRIPTOR(Name, Argc, ...) \
31-
struct Builtin_##Name##_InterfaceDescriptor { \
32-
enum ParameterIndices { \
33-
kJSTarget = compiler::CodeAssembler::kTargetParameterIndex, \
34-
REVERSE_##Argc(__VA_ARGS__) kJSNewTarget, \
35-
kJSActualArgumentsCount, \
36-
kContext, \
37-
kParameterCount, \
38-
}; \
39-
static_assert((Argc) == static_cast<uint16_t>(kParameterCount - 4), \
40-
"Inconsistent set of arguments"); \
41-
static_assert(kJSTarget == -1, "Unexpected kJSTarget index value"); \
42-
};
43-
#else
4417
#define DEFINE_TFJ_INTERFACE_DESCRIPTOR(Name, Argc, ...) \
4518
struct Builtin_##Name##_InterfaceDescriptor { \
4619
enum ParameterIndices { \
@@ -55,7 +28,6 @@ namespace internal {
5528
"Inconsistent set of arguments"); \
5629
static_assert(kJSTarget == -1, "Unexpected kJSTarget index value"); \
5730
};
58-
#endif
5931

6032
// Define interface descriptors for builtins with StubCall linkage.
6133
#define DEFINE_TFC_INTERFACE_DESCRIPTOR(Name, InterfaceDescriptor) \

‎src/codegen/code-stub-assembler.cc‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13025,17 +13025,9 @@ TNode<Object> CodeStubAssembler::CallApiCallback(
1302513025
TNode<Object> context, TNode<RawPtrT> callback, TNode<IntPtrT> argc,
1302613026
TNode<Object> data, TNode<Object> holder, TNode<Object> receiver,
1302713027
TNode<Object> value) {
13028-
// CallApiCallback receives the first four arguments in registers
13029-
// (callback, argc, data and holder). The last arguments are in the stack in
13030-
// JS ordering. See ApiCallbackDescriptor.
1303113028
Callable callable = CodeFactory::CallApiCallback(isolate());
13032-
#ifdef V8_REVERSE_JSARGS
13033-
return CallStub(callable, context, callback, argc, data, holder, value,
13034-
receiver);
13035-
#else
1303613029
return CallStub(callable, context, callback, argc, data, holder, receiver,
1303713030
value);
13038-
#endif
1303913031
}
1304013032

1304113033
TNode<Object> CodeStubAssembler::CallRuntimeNewArray(

‎src/codegen/interface-descriptors.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ void CallInterfaceDescriptorData::InitializePlatformSpecific(
3030

3131
void CallInterfaceDescriptorData::InitializePlatformIndependent(
3232
Flags flags, int return_count, int parameter_count,
33-
const MachineType* machine_types, int machine_types_length) {
33+
const MachineType* machine_types, int machine_types_length,
34+
StackArgumentOrder stack_order) {
3435
DCHECK(IsInitializedPlatformSpecific());
3536

3637
flags_ = flags;
38+
stack_order_ = stack_order;
3739
return_count_ = return_count;
3840
param_count_ = parameter_count;
3941
const int types_length = return_count_ + param_count_;

‎src/codegen/interface-descriptors.h‎

Lines changed: 71 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,16 @@ namespace internal {
105105
BUILTIN_LIST_TFS(V) \
106106
TORQUE_BUILTIN_LIST_TFC(V)
107107

108+
enum class StackArgumentOrder {
109+
kDefault, // Arguments in the stack are pushed in the default/stub order (the
110+
// first argument is pushed first).
111+
kJS, // Arguments in the stack are pushed in the same order as the one used
112+
// by JS-to-JS function calls. This should be used if calling a
113+
// JSFunction or if the builtin is expected to be called directly from a
114+
// JSFunction. When V8_REVERSE_JSARGS is set, this order is reversed
115+
// compared to kDefault.
116+
};
117+
108118
class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
109119
public:
110120
enum Flag {
@@ -140,7 +150,8 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
140150
void InitializePlatformIndependent(Flags flags, int return_count,
141151
int parameter_count,
142152
const MachineType* machine_types,
143-
int machine_types_length);
153+
int machine_types_length,
154+
StackArgumentOrder stack_order);
144155

145156
void Reset();
146157

@@ -163,6 +174,7 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
163174
DCHECK_LT(index, param_count_);
164175
return machine_types_[return_count_ + index];
165176
}
177+
StackArgumentOrder stack_order() const { return stack_order_; }
166178

167179
void RestrictAllocatableRegisters(const Register* registers, int num) {
168180
DCHECK_EQ(allocatable_registers_, 0);
@@ -197,6 +209,7 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
197209
int return_count_ = -1;
198210
int param_count_ = -1;
199211
Flags flags_ = kNoFlags;
212+
StackArgumentOrder stack_order_ = StackArgumentOrder::kDefault;
200213

201214
// Specifying the set of registers that could be used by the register
202215
// allocator. Currently, it's only used by RecordWrite code stub.
@@ -293,6 +306,10 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptor {
293306
return data()->allocatable_registers();
294307
}
295308

309+
StackArgumentOrder GetStackArgumentOrder() const {
310+
return data()->stack_order();
311+
}
312+
296313
static const Register ContextRegister();
297314

298315
const char* DebugName() const;
@@ -312,9 +329,9 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptor {
312329
CallInterfaceDescriptorData* data) {
313330
// Default descriptor configuration: one result, all parameters are passed
314331
// in registers and all parameters have MachineType::AnyTagged() type.
315-
data->InitializePlatformIndependent(CallInterfaceDescriptorData::kNoFlags,
316-
1, data->register_param_count(),
317-
nullptr, 0);
332+
data->InitializePlatformIndependent(
333+
CallInterfaceDescriptorData::kNoFlags, 1, data->register_param_count(),
334+
nullptr, 0, StackArgumentOrder::kDefault);
318335
}
319336

320337
// Initializes |data| using the platform dependent default set of registers.
@@ -400,7 +417,8 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
400417
void InitializePlatformIndependent(CallInterfaceDescriptorData* data) \
401418
override { \
402419
data->InitializePlatformIndependent(Flags(kDescriptorFlags), kReturnCount, \
403-
kParameterCount, nullptr, 0); \
420+
kParameterCount, nullptr, 0, \
421+
kStackArgumentOrder); \
404422
} \
405423
name(CallDescriptors::Key key) : base(key) {} \
406424
\
@@ -418,9 +436,11 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
418436
\
419437
public:
420438

421-
#define DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(flags, return_count, ...) \
439+
#define DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(flags, stack_order, \
440+
return_count, ...) \
422441
static constexpr int kDescriptorFlags = flags; \
423442
static constexpr int kReturnCount = return_count; \
443+
static constexpr StackArgumentOrder kStackArgumentOrder = stack_order; \
424444
enum ParameterIndices { \
425445
__dummy = -1, /* to be able to pass zero arguments */ \
426446
##__VA_ARGS__, \
@@ -429,35 +449,41 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
429449
kContext = kParameterCount /* implicit parameter */ \
430450
};
431451

432-
#define DEFINE_RESULT_AND_PARAMETERS(return_count, ...) \
433-
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
434-
CallInterfaceDescriptorData::kNoFlags, return_count, ##__VA_ARGS__)
452+
#define DEFINE_RESULT_AND_PARAMETERS(return_count, ...) \
453+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
454+
CallInterfaceDescriptorData::kNoFlags, StackArgumentOrder::kDefault, \
455+
return_count, ##__VA_ARGS__)
435456

436457
// This is valid only for builtins that use EntryFrame, which does not scan
437458
// stack arguments on GC.
438-
#define DEFINE_PARAMETERS_ENTRY(...) \
439-
static constexpr int kDescriptorFlags = \
440-
CallInterfaceDescriptorData::kNoContext | \
441-
CallInterfaceDescriptorData::kNoStackScan; \
442-
static constexpr int kReturnCount = 1; \
443-
enum ParameterIndices { \
444-
__dummy = -1, /* to be able to pass zero arguments */ \
445-
##__VA_ARGS__, \
446-
\
447-
kParameterCount \
459+
#define DEFINE_PARAMETERS_ENTRY(...) \
460+
static constexpr int kDescriptorFlags = \
461+
CallInterfaceDescriptorData::kNoContext | \
462+
CallInterfaceDescriptorData::kNoStackScan; \
463+
static constexpr StackArgumentOrder kStackArgumentOrder = \
464+
StackArgumentOrder::kDefault; \
465+
static constexpr int kReturnCount = 1; \
466+
enum ParameterIndices { \
467+
__dummy = -1, /* to be able to pass zero arguments */ \
468+
##__VA_ARGS__, \
469+
\
470+
kParameterCount \
448471
};
449472

450-
#define DEFINE_PARAMETERS(...) \
451-
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
452-
CallInterfaceDescriptorData::kNoFlags, 1, ##__VA_ARGS__)
473+
#define DEFINE_PARAMETERS(...) \
474+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
475+
CallInterfaceDescriptorData::kNoFlags, StackArgumentOrder::kDefault, 1, \
476+
##__VA_ARGS__)
453477

454-
#define DEFINE_PARAMETERS_NO_CONTEXT(...) \
455-
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
456-
CallInterfaceDescriptorData::kNoContext, 1, ##__VA_ARGS__)
478+
#define DEFINE_PARAMETERS_NO_CONTEXT(...) \
479+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
480+
CallInterfaceDescriptorData::kNoContext, StackArgumentOrder::kDefault, \
481+
1, ##__VA_ARGS__)
457482

458-
#define DEFINE_PARAMETERS_VARARGS(...) \
459-
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
460-
CallInterfaceDescriptorData::kAllowVarArgs, 1, ##__VA_ARGS__)
483+
#define DEFINE_PARAMETERS_VARARGS(...) \
484+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
485+
CallInterfaceDescriptorData::kAllowVarArgs, StackArgumentOrder::kJS, 1, \
486+
##__VA_ARGS__)
461487

462488
#define DEFINE_RESULT_AND_PARAMETER_TYPES_WITH_FLAG(flag, ...) \
463489
void InitializePlatformIndependent(CallInterfaceDescriptorData* data) \
@@ -468,7 +494,7 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
468494
"Parameter names definition is not consistent with parameter types"); \
469495
data->InitializePlatformIndependent( \
470496
Flags(flag | kDescriptorFlags), kReturnCount, kParameterCount, \
471-
machine_types, arraysize(machine_types)); \
497+
machine_types, arraysize(machine_types), kStackArgumentOrder); \
472498
}
473499

474500
#define DEFINE_RESULT_AND_PARAMETER_TYPES(...) \
@@ -479,18 +505,20 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
479505
DEFINE_RESULT_AND_PARAMETER_TYPES(MachineType::AnyTagged() /* result */, \
480506
##__VA_ARGS__)
481507

482-
#define DEFINE_JS_PARAMETERS(...) \
483-
static constexpr int kDescriptorFlags = \
484-
CallInterfaceDescriptorData::kAllowVarArgs; \
485-
static constexpr int kReturnCount = 1; \
486-
enum ParameterIndices { \
487-
kTarget, \
488-
kNewTarget, \
489-
kActualArgumentsCount, \
490-
##__VA_ARGS__, \
491-
\
492-
kParameterCount, \
493-
kContext = kParameterCount /* implicit parameter */ \
508+
#define DEFINE_JS_PARAMETERS(...) \
509+
static constexpr int kDescriptorFlags = \
510+
CallInterfaceDescriptorData::kAllowVarArgs; \
511+
static constexpr int kReturnCount = 1; \
512+
static constexpr StackArgumentOrder kStackArgumentOrder = \
513+
StackArgumentOrder::kJS; \
514+
enum ParameterIndices { \
515+
kTarget, \
516+
kNewTarget, \
517+
kActualArgumentsCount, \
518+
##__VA_ARGS__, \
519+
\
520+
kParameterCount, \
521+
kContext = kParameterCount /* implicit parameter */ \
494522
};
495523

496524
#define DEFINE_JS_PARAMETER_TYPES(...) \
@@ -552,7 +580,8 @@ class TorqueInterfaceDescriptor : public CallInterfaceDescriptor {
552580
DCHECK_EQ(kReturnCount + kParameterCount, machine_types.size());
553581
data->InitializePlatformIndependent(Flags(kDescriptorFlags), kReturnCount,
554582
kParameterCount, machine_types.data(),
555-
static_cast<int>(machine_types.size()));
583+
static_cast<int>(machine_types.size()),
584+
StackArgumentOrder::kDefault);
556585
}
557586
};
558587

‎src/compiler/backend/instruction-selector.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,8 @@ void InstructionSelector::InitializeCallBuffer(Node* call, CallBuffer* buffer,
10431043
InstructionOperand op = g.UseLocation(*iter, location);
10441044
UnallocatedOperand unallocated = UnallocatedOperand::cast(op);
10451045
if (unallocated.HasFixedSlotPolicy() && !call_tail) {
1046-
int stack_index = -unallocated.fixed_slot_index() - 1;
1046+
int stack_index = buffer->descriptor->GetStackIndexFromSlot(
1047+
unallocated.fixed_slot_index());
10471048
// This can insert empty slots before stack_index and will insert enough
10481049
// slots after stack_index to store the parameter.
10491050
if (static_cast<size_t>(stack_index) >= buffer->pushed_nodes.size()) {

‎src/compiler/code-assembler.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,11 +1027,7 @@ Node* CodeAssembler::CallJSStubImpl(const CallInterfaceDescriptor& descriptor,
10271027
inputs.Add(new_target);
10281028
}
10291029
inputs.Add(arity);
1030-
#ifdef V8_REVERSE_JSARGS
1031-
for (auto arg : base::Reversed(args)) inputs.Add(arg);
1032-
#else
10331030
for (auto arg : args) inputs.Add(arg);
1034-
#endif
10351031
if (descriptor.HasContextParameter()) {
10361032
inputs.Add(context);
10371033
}

‎src/compiler/linkage.cc‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,11 @@ CallDescriptor* Linkage::GetJSCallDescriptor(Zone* zone, bool is_osr,
325325

326326
// All parameters to JS calls go on the stack.
327327
for (int i = 0; i < js_parameter_count; i++) {
328+
#ifdef V8_REVERSE_JSARGS
329+
int spill_slot_index = -i - 1;
330+
#else
328331
int spill_slot_index = i - js_parameter_count;
332+
#endif
329333
locations.AddParam(LinkageLocation::ForCallerFrameSlot(
330334
spill_slot_index, MachineType::AnyTagged()));
331335
}
@@ -358,7 +362,8 @@ CallDescriptor* Linkage::GetJSCallDescriptor(Zone* zone, bool is_osr,
358362
kNoCalleeSaved, // callee-saved
359363
kNoCalleeSaved, // callee-saved fp
360364
flags, // flags
361-
"js-call");
365+
"js-call", // debug name
366+
StackArgumentOrder::kJS); // stack order
362367
}
363368

364369
// TODO(turbofan): cache call descriptors for code stub calls.
@@ -458,6 +463,7 @@ CallDescriptor* Linkage::GetStubCallDescriptor(
458463
kNoCalleeSaved, // callee-saved fp
459464
CallDescriptor::kCanUseRoots | flags, // flags
460465
descriptor.DebugName(), // debug name
466+
descriptor.GetStackArgumentOrder(), // stack order
461467
descriptor.allocatable_registers());
462468
}
463469

‎src/compiler/linkage.h‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ class V8_EXPORT_PRIVATE CallDescriptor final
237237
RegList callee_saved_registers,
238238
RegList callee_saved_fp_registers, Flags flags,
239239
const char* debug_name = "",
240+
StackArgumentOrder stack_order = StackArgumentOrder::kDefault,
240241
const RegList allocatable_registers = 0,
241242
size_t stack_return_count = 0)
242243
: kind_(kind),
@@ -250,6 +251,7 @@ class V8_EXPORT_PRIVATE CallDescriptor final
250251
callee_saved_fp_registers_(callee_saved_fp_registers),
251252
allocatable_registers_(allocatable_registers),
252253
flags_(flags),
254+
stack_order_(stack_order),
253255
debug_name_(debug_name) {}
254256

255257
// Returns the kind of this call.
@@ -292,6 +294,19 @@ class V8_EXPORT_PRIVATE CallDescriptor final
292294
return stack_param_count_;
293295
}
294296

297+
int GetStackIndexFromSlot(int slot_index) const {
298+
#ifdef V8_REVERSE_JSARGS
299+
switch (GetStackArgumentOrder()) {
300+
case StackArgumentOrder::kDefault:
301+
return -slot_index - 1;
302+
case StackArgumentOrder::kJS:
303+
return slot_index + static_cast<int>(StackParameterCount());
304+
}
305+
#else
306+
return -slot_index - 1;
307+
#endif
308+
}
309+
295310
// The total number of inputs to this call, which includes the target,
296311
// receiver, context, etc.
297312
// TODO(titzer): this should input the framestate input too.
@@ -338,6 +353,8 @@ class V8_EXPORT_PRIVATE CallDescriptor final
338353
return location_sig_->GetParam(index).GetType();
339354
}
340355

356+
StackArgumentOrder GetStackArgumentOrder() const { return stack_order_; }
357+
341358
// Operator properties describe how this call can be optimized, if at all.
342359
Operator::Properties properties() const { return properties_; }
343360

@@ -391,6 +408,7 @@ class V8_EXPORT_PRIVATE CallDescriptor final
391408
// register allocator to use.
392409
const RegList allocatable_registers_;
393410
const Flags flags_;
411+
const StackArgumentOrder stack_order_;
394412
const char* const debug_name_;
395413
const CFunctionInfo* c_function_info_ = nullptr;
396414

0 commit comments

Comments
 (0)