Skip to content

Commit

Permalink
[maglev][turbofan] Use the fast construct builtin when deopting
Browse files Browse the repository at this point in the history
... from constructor calls.

The fast construct builtin has a simplified frame, so we do not
need to maintain the closure, nor the arguments in the translated
frame states.

Bug: v8:14192
Change-Id: Ice30a1d990bab807fa19a7c2d74078c6401191e5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4717489
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#89205}
  • Loading branch information
victorgomes authored and V8 LUCI CQ committed Jul 26, 2023
1 parent e6ff37d commit f991de3
Show file tree
Hide file tree
Showing 32 changed files with 366 additions and 253 deletions.
27 changes: 14 additions & 13 deletions src/builtins/arm/builtins-arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -241,19 +241,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
// Call the function.
__ InvokeFunctionWithNewTarget(r1, r3, r0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- r0: constructor result
// -- sp[0*kPointerSize]: implicit receiver
// -- sp[1*kPointerSize]: padding
// -- sp[2*kPointerSize]: constructor function
// -- sp[3*kPointerSize]: number of arguments
// -- sp[4*kPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1595,6 +1582,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunctionWithNewTarget(r1, r3, r0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- r0 constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
26 changes: 14 additions & 12 deletions src/builtins/arm64/builtins-arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -319,18 +319,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
__ Mov(x0, x12);
__ InvokeFunctionWithNewTarget(x1, x3, x0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- sp[0*kSystemPointerSize]: implicit receiver
// -- sp[1*kSystemPointerSize]: padding
// -- sp[2*kSystemPointerSize]: constructor function
// -- sp[3*kSystemPointerSize]: number of arguments
// -- sp[4*kSystemPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1796,6 +1784,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunctionWithNewTarget(x1, x3, x0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- x0 constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
27 changes: 14 additions & 13 deletions src/builtins/ia32/builtins-ia32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,19 +248,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
__ mov(edi, Operand(ebp, ConstructFrameConstants::kConstructorOffset));
__ InvokeFunction(edi, edx, eax, InvokeType::kCall);

// ----------- S t a t e -------------
// -- eax: constructor result
// -- sp[0*kSystemPointerSize]: implicit receiver
// -- sp[1*kSystemPointerSize]: padding
// -- sp[2*kSystemPointerSize]: constructor function
// -- sp[3*kSystemPointerSize]: number of arguments
// -- sp[4*kSystemPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1539,6 +1526,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the constructor.
__ InvokeFunction(edi, edx, eax, InvokeType::kCall);

// ----------- S t a t e -------------
// -- eax constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
27 changes: 14 additions & 13 deletions src/builtins/loong64/builtins-loong64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,19 +231,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
// Call the function.
__ InvokeFunctionWithNewTarget(a1, a3, a0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- s0: constructor result
// -- sp[0*kSystemPointerSize]: implicit receiver
// -- sp[1*kSystemPointerSize]: padding
// -- sp[2*kSystemPointerSize]: constructor function
// -- sp[3*kSystemPointerSize]: number of arguments
// -- sp[4*kSystemPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1597,6 +1584,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunctionWithNewTarget(a1, a3, a0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- a0 constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
27 changes: 14 additions & 13 deletions src/builtins/mips64/builtins-mips64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,19 +230,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
// Call the function.
__ InvokeFunctionWithNewTarget(a1, a3, a0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- v0: constructor result
// -- sp[0*kSystemPointerSize]: implicit receiver
// -- sp[1*kSystemPointerSize]: padding
// -- sp[2*kSystemPointerSize]: constructor function
// -- sp[3*kSystemPointerSize]: number of arguments
// -- sp[4*kSystemPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1562,6 +1549,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunctionWithNewTarget(a1, a3, a0, InvokeType::kCall);

// ----------- S t a t e -------------
// -- v0 constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
27 changes: 14 additions & 13 deletions src/builtins/ppc/builtins-ppc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -581,19 +581,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
__ InvokeFunctionWithNewTarget(r4, r6, r3, InvokeType::kCall);
}

// ----------- S t a t e -------------
// -- r0: constructor result
// -- sp[0*kSystemPointerSize]: implicit receiver
// -- sp[1*kSystemPointerSize]: padding
// -- sp[2*kSystemPointerSize]: constructor function
// -- sp[3*kSystemPointerSize]: number of arguments
// -- sp[4*kSystemPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1873,6 +1860,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunctionWithNewTarget(r4, r6, r3, InvokeType::kCall);

// ----------- S t a t e -------------
// -- r0 constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
27 changes: 14 additions & 13 deletions src/builtins/s390/builtins-s390.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,19 +556,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
// Call the function.
__ InvokeFunctionWithNewTarget(r3, r5, r2, InvokeType::kCall);

// ----------- S t a t e -------------
// -- r0: constructor result
// -- sp[0*kSystemPointerSize]: implicit receiver
// -- sp[1*kSystemPointerSize]: padding
// -- sp[2*kSystemPointerSize]: constructor function
// -- sp[3*kSystemPointerSize]: number of arguments
// -- sp[4*kSystemPointerSize]: context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1896,6 +1883,20 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunctionWithNewTarget(r3, r5, r2, InvokeType::kCall);

// ----------- S t a t e -------------
// -- r0 constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down
27 changes: 13 additions & 14 deletions src/builtins/x64/builtins-x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,19 +248,6 @@ void Builtins::Generate_JSConstructStubGeneric(MacroAssembler* masm) {
// Call the function.
__ InvokeFunction(rdi, rdx, rax, InvokeType::kCall);

// ----------- S t a t e -------------
// -- rax constructor result
// -- sp[0*kSystemPointerSize] implicit receiver
// -- sp[1*kSystemPointerSize] padding
// -- sp[2*kSystemPointerSize] constructor function
// -- sp[3*kSystemPointerSize] number of arguments
// -- sp[4*kSystemPointerSize] context
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
// on page 74.
Expand Down Expand Up @@ -1520,7 +1507,19 @@ void Builtins::Generate_InterpreterPushArgsThenFastConstructFunction(
// Call the function.
__ InvokeFunction(rdi, rdx, rax, InvokeType::kCall);

// TODO(victorgomes): Change deopt frame in Maglev/TF and point to here!
// ----------- S t a t e -------------
// -- rax constructor result
//
// Stack:
// -- Implicit Receiver
// -- Context
// -- FastConstructMarker
// -- FramePointer
// -----------------------------------

// Store offset of return address for deoptimizer.
masm->isolate()->heap()->SetConstructStubInvokeDeoptPCOffset(
masm->pc_offset());

// If the result is an object (in the ECMA sense), we should get rid
// of the receiver and use the result; see ECMA-262 section 13.2.2-7
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/backend/code-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor(
translations_.BeginConstructCreateStubFrame(shared_info_id, height);
break;
case FrameStateType::kConstructInvokeStub:
translations_.BeginConstructInvokeStubFrame(shared_info_id, height);
translations_.BeginConstructInvokeStubFrame(shared_info_id);
break;
case FrameStateType::kBuiltinContinuation: {
translations_.BeginBuiltinContinuationFrame(bailout_id, shared_info_id,
Expand Down
18 changes: 11 additions & 7 deletions src/compiler/backend/instruction-selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1017,9 +1017,11 @@ size_t InstructionSelectorT<TurboshaftAdapter>::AddInputsToFrameStateDescriptor(
values_descriptor->ReserveSize(descriptor->GetSize());

// Function
entries += v8::internal::compiler::AddOperandToStateValueDescriptor(
this, values_descriptor, inputs, g, deduplicator, &it,
FrameStateInputKind::kStackSlot, zone);
if (descriptor->HasClosure()) {
entries += v8::internal::compiler::AddOperandToStateValueDescriptor(
this, values_descriptor, inputs, g, deduplicator, &it,
FrameStateInputKind::kStackSlot, zone);
}

// Parameters
for (size_t i = 0; i < descriptor->parameters_count(); ++i) {
Expand Down Expand Up @@ -1082,10 +1084,12 @@ size_t InstructionSelectorT<TurbofanAdapter>::AddInputsToFrameStateDescriptor(
DCHECK_EQ(values_descriptor->size(), 0u);
values_descriptor->ReserveSize(descriptor->GetSize());

DCHECK_NOT_NULL(function);
entries += AddOperandToStateValueDescriptor(
values_descriptor, inputs, g, deduplicator, function,
MachineType::AnyTagged(), FrameStateInputKind::kStackSlot, zone);
if (descriptor->HasClosure()) {
DCHECK_NOT_NULL(function);
entries += AddOperandToStateValueDescriptor(
values_descriptor, inputs, g, deduplicator, function,
MachineType::AnyTagged(), FrameStateInputKind::kStackSlot, zone);
}

entries += AddInputsToFrameStateDescriptor(
values_descriptor, inputs, g, deduplicator, parameters, kind, zone);
Expand Down
9 changes: 5 additions & 4 deletions src/compiler/backend/instruction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1147,12 +1147,13 @@ size_t GetConservativeFrameSizeInBytes(FrameStateType type,
#if V8_ENABLE_WEBASSEMBLY
case FrameStateType::kWasmInlinedIntoJS:
#endif
case FrameStateType::kConstructCreateStub:
case FrameStateType::kConstructInvokeStub: {
case FrameStateType::kConstructCreateStub: {
auto info = ConstructStubFrameInfo::Conservative(
static_cast<int>(parameters_count));
return info.frame_size_in_bytes();
}
case FrameStateType::kConstructInvokeStub:
return FastConstructStubFrameInfo::Conservative().frame_size_in_bytes();
case FrameStateType::kBuiltinContinuation:
#if V8_ENABLE_WEBASSEMBLY
case FrameStateType::kJSToWasmBuiltinContinuation:
Expand Down Expand Up @@ -1234,8 +1235,8 @@ size_t FrameStateDescriptor::GetHeight() const {
}

size_t FrameStateDescriptor::GetSize() const {
return 1 + parameters_count() + locals_count() + stack_count() +
(HasContext() ? 1 : 0);
return (HasClosure() ? 1 : 0) + parameters_count() + locals_count() +
stack_count() + (HasContext() ? 1 : 0);
}

size_t FrameStateDescriptor::GetTotalSize() const {
Expand Down
Loading

0 comments on commit f991de3

Please sign in to comment.