From 5d8afae6d409cb59df959e2138180ed7be545ee5 Mon Sep 17 00:00:00 2001 From: Victor Gomes Date: Tue, 31 Jan 2023 17:01:23 +0100 Subject: [PATCH] [maglev] Remove MaglevOutOfLinePrologue The flag is currently poorly tested and we are unlikely to use the OutOfLinePrologue in its current form. Bug: v8:7700, chromium:1411153 Change-Id: Ifd5867910d79fbdeaebb4c21f7070f806d78052c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4208932 Auto-Submit: Victor Gomes Commit-Queue: Leszek Swirski Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#85568} --- src/builtins/builtins-definitions.h | 1 - src/builtins/builtins-internal-gen.cc | 6 - src/builtins/x64/builtins-x64.cc | 204 --------------------- src/execution/frames.cc | 15 +- src/flags/flag-definitions.h | 1 - src/maglev/arm64/maglev-assembler-arm64.cc | 5 - src/maglev/x64/maglev-assembler-x64.cc | 8 - 7 files changed, 3 insertions(+), 237 deletions(-) diff --git a/src/builtins/builtins-definitions.h b/src/builtins/builtins-definitions.h index 424580e38a..56b5846cd0 100644 --- a/src/builtins/builtins-definitions.h +++ b/src/builtins/builtins-definitions.h @@ -198,7 +198,6 @@ namespace internal { \ /* Maglev Compiler */ \ ASM(MaglevOnStackReplacement, OnStackReplacement) \ - ASM(MaglevOutOfLinePrologue, NoContext) \ \ /* Code life-cycle */ \ TFC(CompileLazy, JSTrampoline) \ diff --git a/src/builtins/builtins-internal-gen.cc b/src/builtins/builtins-internal-gen.cc index 19e3a57729..62dcd88bb4 100644 --- a/src/builtins/builtins-internal-gen.cc +++ b/src/builtins/builtins-internal-gen.cc @@ -1373,12 +1373,6 @@ void Builtins::Generate_MaglevOnStackReplacement(MacroAssembler* masm) { static_assert(D::kParameterCount == 1); masm->Trap(); } -void Builtins::Generate_MaglevOutOfLinePrologue(MacroAssembler* masm) { - using D = - i::CallInterfaceDescriptorFor::type; - static_assert(D::kParameterCount == 0); - masm->Trap(); -} #endif // V8_TARGET_ARCH_X64 // ES6 [[Get]] operation. diff --git a/src/builtins/x64/builtins-x64.cc b/src/builtins/x64/builtins-x64.cc index 41309578b0..c93632d92d 100644 --- a/src/builtins/x64/builtins-x64.cc +++ b/src/builtins/x64/builtins-x64.cc @@ -2720,210 +2720,6 @@ void Builtins::Generate_MaglevOnStackReplacement(MacroAssembler* masm) { D::MaybeTargetCodeRegister()); } -// Called immediately at the start of Maglev-generated functions, with all -// state (register and stack) unchanged, except: -// -// - the stack slot byte size and -// - the tagged stack slot byte size -// -// are pushed as untagged arguments to the stack. This prologue builtin takes -// care of a few things that each Maglev function needs on entry: -// -// - the deoptimization check -// - tiering support (checking FeedbackVector flags) -// - the stack overflow / interrupt check -// - and finally, setting up the Maglev frame. -// -// If this builtin returns, the Maglev frame is fully set up and we are -// prepared for continued execution. Otherwise, we take one of multiple -// possible non-standard exit paths (deoptimization, tailcalling other code, or -// throwing a stack overflow exception). -void Builtins::Generate_MaglevOutOfLinePrologue(MacroAssembler* masm) { - using D = - i::CallInterfaceDescriptorFor::type; - static_assert(D::kParameterCount == 0); - - // This builtin is called by Maglev code prior to any register mutations, and - // the only stack mutation is pushing the arguments for this builtin. In - // other words: - // - // - The register state is the same as when we entered the Maglev code object, - // i.e. set up for a standard JS call. - // - The caller has not yet set up a stack frame. - // - The caller has pushed the (untagged) stack parameters for this builtin. - - static constexpr int kStackParameterCount = 2; - static constexpr int kReturnAddressCount = 1; - static constexpr int kReturnAddressOffset = 0 * kSystemPointerSize; - static constexpr int kTaggedStackSlotBytesOffset = 1 * kSystemPointerSize; - static constexpr int kTotalStackSlotBytesOffset = 2 * kSystemPointerSize; - USE(kReturnAddressOffset); - USE(kTaggedStackSlotBytesOffset); - USE(kTotalStackSlotBytesOffset); - - // Scratch registers. Don't clobber regs related to the calling - // convention (e.g. kJavaScriptCallArgCountRegister). - const Register scratch0 = rcx; - const Register scratch1 = r9; - const Register scratch2 = rbx; - - Label deoptimize, optimize, call_stack_guard, call_stack_guard_return; - - // A modified version of BailoutIfDeoptimized that drops the builtin frame - // before deoptimizing. - { - static constexpr int kCodeStartToCodeOffset = - InstructionStream::kCodeOffset - InstructionStream::kHeaderSize; - __ LoadTaggedPointerField( - scratch0, - Operand(kJavaScriptCallCodeStartRegister, kCodeStartToCodeOffset)); - __ testl(FieldOperand(scratch0, Code::kKindSpecificFlagsOffset), - Immediate(1 << InstructionStream::kMarkedForDeoptimizationBit)); - __ j(not_zero, &deoptimize); - } - - // Tiering support. - const Register flags = scratch0; - const Register feedback_vector = scratch1; - { - __ LoadTaggedPointerField( - feedback_vector, - FieldOperand(kJSFunctionRegister, JSFunction::kFeedbackCellOffset)); - __ LoadTaggedPointerField( - feedback_vector, FieldOperand(feedback_vector, Cell::kValueOffset)); - __ AssertFeedbackVector(feedback_vector); - - __ LoadFeedbackVectorFlagsAndJumpIfNeedsProcessing( - flags, feedback_vector, CodeKind::MAGLEV, &optimize); - } - - // Good to go - set up the MAGLEV stack frame and return. - - // First, tear down to the caller frame. - const Register tagged_stack_slot_bytes = scratch1; - const Register total_stack_slot_bytes = scratch0; - const Register return_address = scratch2; - __ PopReturnAddressTo(return_address); - __ Pop(tagged_stack_slot_bytes); - __ Pop(total_stack_slot_bytes); - - __ EnterFrame(StackFrame::MAGLEV); - - // Save arguments in frame. - // TODO(leszeks): Consider eliding this frame if we don't make any calls - // that could clobber these registers. - __ Push(kContextRegister); - __ Push(kJSFunctionRegister); // Callee's JS function. - __ Push(kJavaScriptCallArgCountRegister); // Actual argument count. - - { - ASM_CODE_COMMENT_STRING(masm, " Stack/interrupt check"); - // Stack check. This folds the checks for both the interrupt stack limit - // check and the real stack limit into one by just checking for the - // interrupt limit. The interrupt limit is either equal to the real stack - // limit or tighter. By ensuring we have space until that limit after - // building the frame we can quickly precheck both at once. - // TODO(leszeks): Include a max call argument size here. - __ Move(kScratchRegister, rsp); - __ subq(kScratchRegister, total_stack_slot_bytes); - __ cmpq(kScratchRegister, - __ StackLimitAsOperand(StackLimitKind::kInterruptStackLimit)); - __ j(below, &call_stack_guard); - __ bind(&call_stack_guard_return); - } - - // Initialize stack slots: - // - // - tagged slots are initialized with smi zero. - // - untagged slots are simply reserved without initialization. - // - // Tagged slots first. - const Register untagged_stack_slot_bytes = total_stack_slot_bytes; - { - Label next, loop_condition, loop_header; - - DCHECK_EQ(total_stack_slot_bytes, untagged_stack_slot_bytes); - __ subq(total_stack_slot_bytes, tagged_stack_slot_bytes); - - const Register smi_zero = rax; - DCHECK(!AreAliased(smi_zero, scratch0, scratch1, scratch2)); - __ Move(smi_zero, Smi::zero()); - - __ jmp(&loop_condition, Label::kNear); - - // TODO(leszeks): Consider filling with xmm + movdqa instead. - // TODO(v8:7700): Consider doing more than one push per loop iteration. - __ bind(&loop_header); - __ pushq(rax); - __ bind(&loop_condition); - __ subq(tagged_stack_slot_bytes, Immediate(kSystemPointerSize)); - __ j(greater_equal, &loop_header, Label::kNear); - - __ bind(&next); - } - - // Untagged slots second. - __ subq(rsp, untagged_stack_slot_bytes); - - // The "all-good" return location. This is the only spot where we actually - // return to the caller. - __ PushReturnAddressFrom(return_address); - __ ret(0); - - __ bind(&deoptimize); - { - // Drop the frame and jump to CompileLazyDeoptimizedCode. This is slightly - // fiddly due to the CET shadow stack (otherwise we could do a conditional - // Jump to the builtin). - __ Drop(kStackParameterCount + kReturnAddressCount); - __ Move(scratch0, - BUILTIN_CODE(masm->isolate(), CompileLazyDeoptimizedCode)); - __ LoadCodeEntry(scratch0, scratch0); - __ PushReturnAddressFrom(scratch0); - __ ret(0); - } - - __ bind(&optimize); - { - __ Drop(kStackParameterCount + kReturnAddressCount); - __ AssertFunction(kJSFunctionRegister); - __ OptimizeCodeOrTailCallOptimizedCodeSlot( - flags, feedback_vector, kJSFunctionRegister, JumpMode::kPushAndReturn); - __ Trap(); - } - - __ bind(&call_stack_guard); - { - ASM_CODE_COMMENT_STRING(masm, "Stack/interrupt call"); - - // Push the MAGLEV code return address now, as if it had been pushed by the - // call to this builtin. - __ PushReturnAddressFrom(return_address); - - { - FrameScope inner_frame_scope(masm, StackFrame::INTERNAL); - __ SmiTag(total_stack_slot_bytes); - __ Push(total_stack_slot_bytes); - __ SmiTag(tagged_stack_slot_bytes); - __ Push(tagged_stack_slot_bytes); - // Save any registers that can be referenced by maglev::RegisterInput. - // TODO(leszeks): Only push those that are used by the graph. - __ Push(kJavaScriptCallNewTargetRegister); - // Push the frame size. - __ Push(total_stack_slot_bytes); - __ CallRuntime(Runtime::kStackGuardWithGap, 1); - __ Pop(kJavaScriptCallNewTargetRegister); - __ Pop(tagged_stack_slot_bytes); - __ SmiUntag(tagged_stack_slot_bytes); - __ Pop(total_stack_slot_bytes); - __ SmiUntag(total_stack_slot_bytes); - } - - __ PopReturnAddressTo(return_address); - __ jmp(&call_stack_guard_return); - } -} - #if V8_ENABLE_WEBASSEMBLY // Returns the offset beyond the last saved FP register. diff --git a/src/execution/frames.cc b/src/execution/frames.cc index ec4b8cd2b1..1b6c4e857b 100644 --- a/src/execution/frames.cc +++ b/src/execution/frames.cc @@ -1468,18 +1468,9 @@ void MaglevFrame::Iterate(RootVisitor* v) const { // We don't have a complete frame in this case. uint32_t register_input_count = maglev_safepoint_entry.register_input_count(); - if (v8_flags.maglev_ool_prologue) { - // DCHECK the frame setup under the above assumption. The - // MaglevOutOfLinePrologue builtin creates an INTERNAL frame for the - // StackGuardWithGap call (where extra slots and args are), so the MAGLEV - // frame itself is exactly kFixedFrameSizeFromFp. - DCHECK_EQ(static_cast(fp() - sp()), - StandardFrameConstants::kFixedFrameSizeFromFp); - } else { - // DCHECK the frame setup under the above assumption. - DCHECK_EQ(static_cast(fp() - sp()), - MaglevFrame::StackGuardFrameSize(register_input_count)); - } + // DCHECK the frame setup under the above assumption. + DCHECK_EQ(static_cast(fp() - sp()), + MaglevFrame::StackGuardFrameSize(register_input_count)); spill_slot_count = 0; // We visit the saved register inputs as if they were tagged slots. tagged_slot_count = register_input_count; diff --git a/src/flags/flag-definitions.h b/src/flags/flag-definitions.h index 497e5a890c..af293f0533 100644 --- a/src/flags/flag-definitions.h +++ b/src/flags/flag-definitions.h @@ -490,7 +490,6 @@ DEFINE_BOOL(trace_maglev_inlining, false, "trace maglev inlining") // TODO(v8:7700): Remove once stable. DEFINE_BOOL(maglev_function_context_specialization, true, "enable function context specialization in maglev") -DEFINE_BOOL(maglev_ool_prologue, false, "use the Maglev out of line prologue") #if ENABLE_SPARKPLUG DEFINE_WEAK_IMPLICATION(future, flush_baseline_code) diff --git a/src/maglev/arm64/maglev-assembler-arm64.cc b/src/maglev/arm64/maglev-assembler-arm64.cc index 48e40fab58..85b109df4b 100644 --- a/src/maglev/arm64/maglev-assembler-arm64.cc +++ b/src/maglev/arm64/maglev-assembler-arm64.cc @@ -293,11 +293,6 @@ void MaglevAssembler::TestTypeOf( } void MaglevAssembler::Prologue(Graph* graph) { - if (v8_flags.maglev_ool_prologue) { - // TODO(v8:7700): Implement! - UNREACHABLE(); - } - ScratchRegisterScope temps(this); // We add two extra registers to the scope. Ideally we could add all the // allocatable general registers, except Context, JSFunction, NewTarget and diff --git a/src/maglev/x64/maglev-assembler-x64.cc b/src/maglev/x64/maglev-assembler-x64.cc index 59d6613f37..4f4edb932e 100644 --- a/src/maglev/x64/maglev-assembler-x64.cc +++ b/src/maglev/x64/maglev-assembler-x64.cc @@ -487,14 +487,6 @@ void MaglevAssembler::TruncateDoubleToInt32(Register dst, DoubleRegister src) { } void MaglevAssembler::Prologue(Graph* graph) { - if (v8_flags.maglev_ool_prologue) { - // Call the out-of-line prologue (with parameters passed on the stack). - Push(Immediate(code_gen_state()->stack_slots() * kSystemPointerSize)); - Push(Immediate(code_gen_state()->tagged_slots() * kSystemPointerSize)); - CallBuiltin(Builtin::kMaglevOutOfLinePrologue); - return; - } - BailoutIfDeoptimized(rbx); // Tiering support.