Commit 4ae5a813 authored by Jakob Gruber's avatar Jakob Gruber Committed by V8 LUCI CQ

Remove the Dummy interface descriptor

Having it around is an invitation to use it in new places. This CL
removes the generic Dummy descriptor and replaces it by other existing
descriptors if possible, and defines specialized dummies otherwise.

In the future, every builtin should have a real descriptor. Especially
new ASM builtins should define descriptors and use them in their
implementation (use Descriptor::FooRegister() instead of documenting
the calling convention as comments).

Change-Id: Ib577aa03b5e5a522460d1084cc9605c55cd29d6c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3585945
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80030}
parent 6bf7e04e
......@@ -28,9 +28,6 @@ namespace internal {
// ASM: Builtin in platform-dependent assembly.
// Args: name, interface descriptor
// TODO(jgruber): Remove DummyDescriptor once all ASM builtins have been
// properly associated with their descriptor.
// Builtins are additionally split into tiers, where the tier determines the
// distance of the builtins table from the root register within IsolateData.
//
......@@ -146,16 +143,16 @@ namespace internal {
ASM(ConstructFunctionForwardVarargs, ConstructForwardVarargs) \
TFC(Construct_Baseline, Construct_Baseline) \
TFC(Construct_WithFeedback, Construct_WithFeedback) \
ASM(JSConstructStubGeneric, Dummy) \
ASM(JSBuiltinsConstructStub, Dummy) \
ASM(JSConstructStubGeneric, ConstructStub) \
ASM(JSBuiltinsConstructStub, ConstructStub) \
TFC(FastNewObject, FastNewObject) \
TFS(FastNewClosure, kSharedFunctionInfo, kFeedbackCell) \
/* ES6 section 9.5.14 [[Construct]] ( argumentsList, newTarget) */ \
TFC(ConstructProxy, JSTrampoline) \
\
/* Apply and entries */ \
ASM(JSEntry, Dummy) \
ASM(JSConstructEntry, Dummy) \
ASM(JSEntry, JSEntry) \
ASM(JSConstructEntry, JSEntry) \
ASM(JSRunMicrotasksEntry, RunMicrotasksEntry) \
/* Call a JSValue. */ \
ASM(JSEntryTrampoline, JSTrampoline) \
......@@ -187,8 +184,8 @@ namespace internal {
InterpreterPushArgsThenConstruct) \
ASM(InterpreterPushArgsThenConstructWithFinalSpread, \
InterpreterPushArgsThenConstruct) \
ASM(InterpreterEnterAtBytecode, Dummy) \
ASM(InterpreterEnterAtNextBytecode, Dummy) \
ASM(InterpreterEnterAtBytecode, Void) \
ASM(InterpreterEnterAtNextBytecode, Void) \
ASM(InterpreterOnStackReplacement, InterpreterOnStackReplacement) \
\
/* Baseline Compiler */ \
......@@ -203,7 +200,7 @@ namespace internal {
TFC(CompileLazy, JSTrampoline) \
TFC(CompileLazyDeoptimizedCode, JSTrampoline) \
TFC(InstantiateAsmJs, JSTrampoline) \
ASM(NotifyDeoptimized, Dummy) \
ASM(NotifyDeoptimized, Void) \
\
/* Trampolines called when returning from a deoptimization that expects */ \
/* to continue in a JavaScript builtin to finish the functionality of a */ \
......@@ -225,10 +222,10 @@ namespace internal {
/* stack parameter to the JavaScript builtin by the "WithResult" */ \
/* trampoline variant. The plain variant is used in EAGER deopt contexts */ \
/* and has no such special handling. */ \
ASM(ContinueToCodeStubBuiltin, Dummy) \
ASM(ContinueToCodeStubBuiltinWithResult, Dummy) \
ASM(ContinueToJavaScriptBuiltin, Dummy) \
ASM(ContinueToJavaScriptBuiltinWithResult, Dummy) \
ASM(ContinueToCodeStubBuiltin, ContinueToBuiltin) \
ASM(ContinueToCodeStubBuiltinWithResult, ContinueToBuiltin) \
ASM(ContinueToJavaScriptBuiltin, ContinueToBuiltin) \
ASM(ContinueToJavaScriptBuiltinWithResult, ContinueToBuiltin) \
\
/* API callback handling */ \
ASM(CallApiCallback, ApiCallback) \
......@@ -952,13 +949,13 @@ namespace internal {
TFJ(TypedArrayPrototypeMap, kDontAdaptArgumentsSentinel) \
\
/* Wasm */ \
IF_WASM(ASM, GenericJSToWasmWrapper, Dummy) \
IF_WASM(ASM, WasmReturnPromiseOnSuspend, Dummy) \
IF_WASM(ASM, GenericJSToWasmWrapper, WasmDummy) \
IF_WASM(ASM, WasmReturnPromiseOnSuspend, WasmDummy) \
IF_WASM(ASM, WasmSuspend, WasmSuspend) \
IF_WASM(ASM, WasmResume, Dummy) \
IF_WASM(ASM, WasmCompileLazy, Dummy) \
IF_WASM(ASM, WasmDebugBreak, Dummy) \
IF_WASM(ASM, WasmOnStackReplace, Dummy) \
IF_WASM(ASM, WasmResume, WasmDummy) \
IF_WASM(ASM, WasmCompileLazy, WasmDummy) \
IF_WASM(ASM, WasmDebugBreak, WasmDummy) \
IF_WASM(ASM, WasmOnStackReplace, WasmDummy) \
IF_WASM(TFC, WasmFloat32ToNumber, WasmFloat32ToNumber) \
IF_WASM(TFC, WasmFloat64ToNumber, WasmFloat64ToNumber) \
IF_WASM(TFC, WasmI32AtomicWait32, WasmI32AtomicWait32) \
......@@ -1039,25 +1036,25 @@ namespace internal {
TFJ(AsyncIteratorValueUnwrap, kJSArgcReceiverSlots + 1, kReceiver, kValue) \
\
/* CEntry */ \
ASM(CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit, Dummy) \
ASM(CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit, Dummy) \
ASM(CEntry_Return1_DontSaveFPRegs_ArgvInRegister_NoBuiltinExit, Dummy) \
ASM(CEntry_Return1_SaveFPRegs_ArgvOnStack_NoBuiltinExit, Dummy) \
ASM(CEntry_Return1_SaveFPRegs_ArgvOnStack_BuiltinExit, Dummy) \
ASM(CEntry_Return2_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit, Dummy) \
ASM(CEntry_Return2_DontSaveFPRegs_ArgvOnStack_BuiltinExit, Dummy) \
ASM(CEntry_Return2_DontSaveFPRegs_ArgvInRegister_NoBuiltinExit, Dummy) \
ASM(CEntry_Return2_SaveFPRegs_ArgvOnStack_NoBuiltinExit, Dummy) \
ASM(CEntry_Return2_SaveFPRegs_ArgvOnStack_BuiltinExit, Dummy) \
ASM(DirectCEntry, Dummy) \
ASM(CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit, CEntryDummy) \
ASM(CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit, \
CEntry1ArgvOnStack) \
ASM(CEntry_Return1_DontSaveFPRegs_ArgvInRegister_NoBuiltinExit, CEntryDummy) \
ASM(CEntry_Return1_SaveFPRegs_ArgvOnStack_NoBuiltinExit, CEntryDummy) \
ASM(CEntry_Return1_SaveFPRegs_ArgvOnStack_BuiltinExit, CEntryDummy) \
ASM(CEntry_Return2_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit, CEntryDummy) \
ASM(CEntry_Return2_DontSaveFPRegs_ArgvOnStack_BuiltinExit, CEntryDummy) \
ASM(CEntry_Return2_DontSaveFPRegs_ArgvInRegister_NoBuiltinExit, CEntryDummy) \
ASM(CEntry_Return2_SaveFPRegs_ArgvOnStack_NoBuiltinExit, CEntryDummy) \
ASM(CEntry_Return2_SaveFPRegs_ArgvOnStack_BuiltinExit, CEntryDummy) \
ASM(DirectCEntry, CEntryDummy) \
\
/* String helpers */ \
TFS(StringAdd_CheckNone, kLeft, kRight) \
TFS(SubString, kString, kFrom, kTo) \
\
/* Miscellaneous */ \
ASM(StackCheck, Dummy) \
ASM(DoubleToI, Dummy) \
ASM(DoubleToI, Void) \
TFC(GetProperty, GetProperty) \
TFS(GetPropertyWithReceiver, kObject, kKey, kReceiver, kOnNonExistent) \
TFS(SetProperty, kReceiver, kKey, kValue) \
......
......@@ -22,13 +22,6 @@
namespace v8 {
namespace internal {
// -----------------------------------------------------------------------------
// Stack checks.
void Builtins::Generate_StackCheck(MacroAssembler* masm) {
masm->TailCallRuntime(Runtime::kStackGuard);
}
// -----------------------------------------------------------------------------
// TurboFan support builtins.
......
......@@ -154,7 +154,6 @@ class Builtins {
Handle<CodeT> NonPrimitiveToPrimitive(
ToPrimitiveHint hint = ToPrimitiveHint::kDefault);
Handle<CodeT> OrdinaryToPrimitive(OrdinaryToPrimitiveHint hint);
Handle<CodeT> JSConstructStubGeneric();
// Used by CreateOffHeapTrampolines in isolate.cc.
void set_code(Builtin builtin, CodeT code);
......
......@@ -229,8 +229,7 @@ constexpr auto ConstructStubDescriptor::registers() {
// r0 : number of arguments
// r1 : the target to call
// r3 : the new target
// r2 : allocation site or undefined
return RegisterArray(r1, r3, r0, r2);
return RegisterArray(r1, r3, r0);
}
// static
......
......@@ -228,8 +228,7 @@ constexpr auto ConstructStubDescriptor::registers() {
// x3: new target
// x1: target
// x0: number of arguments
// x2: allocation site or undefined
return RegisterArray(x1, x3, x0, x2);
return RegisterArray(x1, x3, x0);
}
// static
......
......@@ -226,9 +226,7 @@ constexpr auto ConstructStubDescriptor::registers() {
// eax : number of arguments
// edx : the new target
// edi : the target to call
// ecx : allocation site or undefined
// TODO(jgruber): Remove the unused allocation site parameter.
return RegisterArray(edi, edx, eax, ecx);
return RegisterArray(edi, edx, eax);
}
// static
......
......@@ -677,11 +677,8 @@ class V8_EXPORT_PRIVATE VoidDescriptor
static constexpr auto registers();
};
// Dummy descriptor used to mark builtins that don't yet have their proper
// descriptor associated.
using DummyDescriptor = VoidDescriptor;
// Dummy descriptor that marks builtins with C calling convention.
// TODO(jgruber): Define real descriptors for C calling conventions.
using CCallDescriptor = VoidDescriptor;
// Marks deoptimization entry builtins. Precise calling conventions currently
......@@ -690,6 +687,22 @@ using CCallDescriptor = VoidDescriptor;
// here.
using DeoptimizationEntryDescriptor = VoidDescriptor;
// TODO(jgruber): Consider filling in the details here; however, this doesn't
// make too much sense as long as the descriptor isn't used or verified.
using JSEntryDescriptor = VoidDescriptor;
// TODO(jgruber): Consider filling in the details here; however, this doesn't
// make too much sense as long as the descriptor isn't used or verified.
using CEntryDummyDescriptor = VoidDescriptor;
// TODO(jgruber): Consider filling in the details here; however, this doesn't
// make too much sense as long as the descriptor isn't used or verified.
using ContinueToBuiltinDescriptor = VoidDescriptor;
// TODO(wasm): Consider filling in details / defining real descriptors for all
// builtins still using this placeholder descriptor.
using WasmDummyDescriptor = VoidDescriptor;
class AllocateDescriptor
: public StaticCallInterfaceDescriptor<AllocateDescriptor> {
public:
......@@ -1429,9 +1442,8 @@ class ConstructWithArrayLike_WithFeedbackDescriptor
class ConstructStubDescriptor
: public StaticCallInterfaceDescriptor<ConstructStubDescriptor> {
public:
// TODO(jgruber): Remove the unused allocation site parameter.
DEFINE_JS_PARAMETERS(kAllocationSite)
DEFINE_JS_PARAMETER_TYPES(MachineType::AnyTagged())
DEFINE_JS_PARAMETERS()
DEFINE_JS_PARAMETER_TYPES()
// TODO(ishell): Use DECLARE_JS_COMPATIBLE_DESCRIPTOR if registers match
DECLARE_DESCRIPTOR(ConstructStubDescriptor)
......
......@@ -241,8 +241,7 @@ constexpr auto ConstructStubDescriptor::registers() {
// rax : number of arguments
// rdx : the new target
// rdi : the target to call
// rbx : allocation site or undefined
return RegisterArray(rdi, rdx, rax, rbx);
return RegisterArray(rdi, rdx, rax);
}
// static
......
......@@ -1625,22 +1625,21 @@ Reduction JSTypedLowering::ReduceJSConstruct(Node* node) {
if (!function.map().is_constructor()) return NoChange();
// Patch {node} to an indirect call via the {function}s construct stub.
bool use_builtin_construct_stub = function.shared().construct_as_builtin();
CodeTRef code = MakeRef(
broker(), use_builtin_construct_stub
? BUILTIN_CODE(isolate(), JSBuiltinsConstructStub)
: BUILTIN_CODE(isolate(), JSConstructStubGeneric));
Callable callable = Builtins::CallableFor(
isolate(), function.shared().construct_as_builtin()
? Builtin::kJSBuiltinsConstructStub
: Builtin::kJSConstructStubGeneric);
STATIC_ASSERT(JSConstructNode::TargetIndex() == 0);
STATIC_ASSERT(JSConstructNode::NewTargetIndex() == 1);
node->RemoveInput(n.FeedbackVectorIndex());
node->InsertInput(graph()->zone(), 0, jsgraph()->Constant(code));
node->InsertInput(graph()->zone(), 0,
jsgraph()->HeapConstant(callable.code()));
node->InsertInput(graph()->zone(), 3,
jsgraph()->Constant(JSParameterCount(arity)));
node->InsertInput(graph()->zone(), 4, jsgraph()->UndefinedConstant());
node->InsertInput(graph()->zone(), 5, jsgraph()->UndefinedConstant());
NodeProperties::ChangeOp(
node, common()->Call(Linkage::GetStubCallDescriptor(
graph()->zone(), ConstructStubDescriptor{}, 1 + arity,
graph()->zone(), callable.descriptor(), 1 + arity,
CallDescriptor::kNeedsFrameState)));
return Changed(node);
}
......
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