From b4012176751767b1f2e58f9ac32520cb443ab2bd Mon Sep 17 00:00:00 2001 From: mythria Date: Tue, 19 Jul 2016 07:16:10 -0700 Subject: [PATCH] Revert of [Interpreter] Collect type feedback for 'new' in the bytecode handler (patchset #6 id:100001 of https://codereview.chromium.org/2153433002/ ) Reason for revert: This cl causes a large regression in octane (https://chromeperf.appspot.com/group_report?bug_id=629503). I have to investigate the reason before I can reland this. Original issue's description: > [Interpreter] Collect type feedback for 'new' in the bytecode handler > > Collect type feedback in the bytecode handler for 'new' bytecode. The > current implementation does not collect allocation site feedback. > > BUG=v8:4280, v8:4780 > LOG=N > > Committed: https://crrev.com/1eadc76419b323fb2e55ae9953142f801704aa59 > Cr-Commit-Position: refs/heads/master@{#37862} TBR=rmcilroy@chromium.org,bmeurer@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:4280, v8:4780 Review-Url: https://codereview.chromium.org/2165633003 Cr-Commit-Position: refs/heads/master@{#37872} --- src/builtins/arm/builtins-arm.cc | 15 +- src/builtins/arm64/builtins-arm64.cc | 15 +- src/builtins/builtins.cc | 22 --- src/builtins/builtins.h | 5 - src/builtins/ia32/builtins-ia32.cc | 16 +- src/builtins/mips/builtins-mips.cc | 15 +- src/builtins/mips64/builtins-mips64.cc | 15 +- src/builtins/x64/builtins-x64.cc | 15 +- src/code-factory.cc | 8 +- src/code-factory.h | 3 +- src/compiler/bytecode-graph-builder.cc | 11 +- src/interpreter/bytecode-array-builder.cc | 6 +- src/interpreter/bytecode-array-builder.h | 2 +- src/interpreter/bytecode-generator.cc | 13 +- src/interpreter/bytecodes.h | 2 +- src/interpreter/interpreter-assembler.cc | 152 +----------------- src/interpreter/interpreter-assembler.h | 4 +- src/interpreter/interpreter.cc | 6 +- test/cctest/cctest.status | 12 ++ .../bytecode_expectations/CallNew.golden | 12 +- .../ClassAndSuperClass.golden | 8 +- .../ClassDeclarations.golden | 4 +- .../bytecode-array-builder-unittest.cc | 6 +- 23 files changed, 68 insertions(+), 299 deletions(-) diff --git a/src/builtins/arm/builtins-arm.cc b/src/builtins/arm/builtins-arm.cc index 51b0f451127..2c9c9c0b0cd 100644 --- a/src/builtins/arm/builtins-arm.cc +++ b/src/builtins/arm/builtins-arm.cc @@ -1206,8 +1206,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType construct_type) { +void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- r0 : argument count (not including receiver) // -- r3 : new target @@ -1226,16 +1225,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl( // Push the arguments. Generate_InterpreterPushArgs(masm, r2, r4, r5); - if (construct_type == CallableType::kJSFunction) { - // TODO(mythria): Change this when allocation site feedback is available. - // ConstructFunction initializes allocation site to undefined. - __ Jump(masm->isolate()->builtins()->ConstructFunction(), - RelocInfo::CODE_TARGET); - } else { - DCHECK_EQ(construct_type, CallableType::kAny); - // Call the constructor with r0, r1, and r3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); - } + // Call the constructor with r0, r1, and r3 unmodified. + __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/arm64/builtins-arm64.cc b/src/builtins/arm64/builtins-arm64.cc index bd9706e6d8a..f4af8fdb9d1 100644 --- a/src/builtins/arm64/builtins-arm64.cc +++ b/src/builtins/arm64/builtins-arm64.cc @@ -1213,8 +1213,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType construct_type) { +void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- x0 : argument count (not including receiver) // -- x3 : new target @@ -1245,16 +1244,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl( __ Cmp(x6, x4); __ B(gt, &loop_header); - if (construct_type == CallableType::kJSFunction) { - // TODO(mythria): Change this when allocation site feedback is available. - // ConstructFunction initializes allocation site to undefined. - __ Jump(masm->isolate()->builtins()->ConstructFunction(), - RelocInfo::CODE_TARGET); - } else { - DCHECK_EQ(construct_type, CallableType::kAny); - // Call the constructor with x0, x1, and x3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); - } + // Call the constructor with x0, x1, and x3 unmodified. + __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/builtins.cc b/src/builtins/builtins.cc index 538c36d38c8..ac8419798ef 100644 --- a/src/builtins/builtins.cc +++ b/src/builtins/builtins.cc @@ -4733,18 +4733,6 @@ Handle Builtins::InterpreterPushArgsAndCall(TailCallMode tail_call_mode, return Handle::null(); } -Handle Builtins::InterpreterPushArgsAndConstruct( - CallableType function_type) { - switch (function_type) { - case CallableType::kJSFunction: - return InterpreterPushArgsAndConstructFunction(); - case CallableType::kAny: - return InterpreterPushArgsAndConstruct(); - } - UNREACHABLE(); - return Handle::null(); -} - namespace { class RelocatableArguments : public BuiltinArguments, public Relocatable { @@ -5850,16 +5838,6 @@ void Builtins::Generate_InterpreterPushArgsAndTailCallFunction( CallableType::kJSFunction); } -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { - return Generate_InterpreterPushArgsAndConstructImpl(masm, CallableType::kAny); -} - -void Builtins::Generate_InterpreterPushArgsAndConstructFunction( - MacroAssembler* masm) { - return Generate_InterpreterPushArgsAndConstructImpl( - masm, CallableType::kJSFunction); -} - void Builtins::Generate_MathMax(MacroAssembler* masm) { Generate_MathMaxMin(masm, MathMaxMinKind::kMax); } diff --git a/src/builtins/builtins.h b/src/builtins/builtins.h index eb7a46a8269..b6d40e9d6a5 100644 --- a/src/builtins/builtins.h +++ b/src/builtins/builtins.h @@ -129,7 +129,6 @@ namespace internal { ASM(InterpreterPushArgsAndCall) \ ASM(InterpreterPushArgsAndTailCall) \ ASM(InterpreterPushArgsAndConstruct) \ - ASM(InterpreterPushArgsAndConstructFunction) \ ASM(InterpreterEnterBytecodeDispatch) \ \ /* Code life-cycle */ \ @@ -558,7 +557,6 @@ class Builtins { Handle InterpreterPushArgsAndCall( TailCallMode tail_call_mode, CallableType function_type = CallableType::kAny); - Handle InterpreterPushArgsAndConstruct(CallableType function_type); Code* builtin(Name name) { // Code::cast cannot be used here since we access builtins @@ -600,9 +598,6 @@ class Builtins { MacroAssembler* masm, TailCallMode tail_call_mode, CallableType function_type); - static void Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType function_type); - static void Generate_DatePrototype_GetField(MacroAssembler* masm, int field_index); diff --git a/src/builtins/ia32/builtins-ia32.cc b/src/builtins/ia32/builtins-ia32.cc index d94700866bd..b549ab1336e 100644 --- a/src/builtins/ia32/builtins-ia32.cc +++ b/src/builtins/ia32/builtins-ia32.cc @@ -758,8 +758,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType construct_type) { +void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- eax : the number of arguments (not including the receiver) // -- edx : the new target @@ -791,17 +790,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl( // Re-push return address. __ Push(ecx); - if (construct_type == CallableType::kJSFunction) { - // TODO(mythria): Change this when allocation site feedback is available. - // ConstructFunction initializes allocation site to undefined. - __ Jump(masm->isolate()->builtins()->ConstructFunction(), - RelocInfo::CODE_TARGET); - } else { - DCHECK_EQ(construct_type, CallableType::kAny); - - // Call the constructor with unmodified eax, edi, ebi values. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); - } + // Call the constructor with unmodified eax, edi, ebi values. + __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/mips/builtins-mips.cc b/src/builtins/mips/builtins-mips.cc index e950112f9e5..63c3329e24d 100644 --- a/src/builtins/mips/builtins-mips.cc +++ b/src/builtins/mips/builtins-mips.cc @@ -1198,8 +1198,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType construct_type) { +void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- a0 : argument count (not including receiver) // -- a3 : new target @@ -1224,16 +1223,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl( __ bind(&loop_check); __ Branch(&loop_header, gt, a2, Operand(t0)); - if (construct_type == CallableType::kJSFunction) { - // TODO(mythria): Change this when allocation site feedback is available. - // ConstructFunction initializes allocation site to undefined. - __ Jump(masm->isolate()->builtins()->ConstructFunction(), - RelocInfo::CODE_TARGET); - } else { - DCHECK_EQ(construct_type, CallableType::kAny); - // Call the constructor with a0, a1, and a3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); - } + // Call the constructor with a0, a1, and a3 unmodified. + __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/mips64/builtins-mips64.cc b/src/builtins/mips64/builtins-mips64.cc index d9dd3992578..ffbc63eaa90 100644 --- a/src/builtins/mips64/builtins-mips64.cc +++ b/src/builtins/mips64/builtins-mips64.cc @@ -1190,8 +1190,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType construct_type) { +void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- a0 : argument count (not including receiver) // -- a3 : new target @@ -1216,16 +1215,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl( __ bind(&loop_check); __ Branch(&loop_header, gt, a2, Operand(t0)); - if (construct_type == CallableType::kJSFunction) { - // TODO(mythria): Change this when allocation site feedback is available. - // ConstructFunction initializes allocation site to undefined. - __ Jump(masm->isolate()->builtins()->ConstructFunction(), - RelocInfo::CODE_TARGET); - } else { - DCHECK_EQ(construct_type, CallableType::kAny); - // Call the constructor with a0, a1, and a3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); - } + // Call the constructor with a0, a1, and a3 unmodified. + __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/x64/builtins-x64.cc b/src/builtins/x64/builtins-x64.cc index 42a613ee32a..1fb26bf8dd4 100644 --- a/src/builtins/x64/builtins-x64.cc +++ b/src/builtins/x64/builtins-x64.cc @@ -840,8 +840,7 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstructImpl( - MacroAssembler* masm, CallableType construct_type) { +void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- rax : the number of arguments (not including the receiver) // -- rdx : the new target (either the same as the constructor or @@ -863,16 +862,8 @@ void Builtins::Generate_InterpreterPushArgsAndConstructImpl( // Push return address in preparation for the tail-call. __ PushReturnAddressFrom(kScratchRegister); - if (construct_type == CallableType::kJSFunction) { - // TODO(mythria): Change this when allocation site feedback is available. - // ConstructFunction initializes allocation site to undefined. - __ Jump(masm->isolate()->builtins()->ConstructFunction(), - RelocInfo::CODE_TARGET); - } else { - DCHECK_EQ(construct_type, CallableType::kAny); - // Call the constructor (rax, rdx, rdi passed on). - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); - } + // Call the constructor (rax, rdx, rdi passed on). + __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/code-factory.cc b/src/code-factory.cc index 62ed852f854..19bb44228c8 100644 --- a/src/code-factory.cc +++ b/src/code-factory.cc @@ -581,11 +581,9 @@ Callable CodeFactory::InterpreterPushArgsAndCall(Isolate* isolate, // static -Callable CodeFactory::InterpreterPushArgsAndConstruct( - Isolate* isolate, CallableType function_type) { - return Callable( - isolate->builtins()->InterpreterPushArgsAndConstruct(function_type), - InterpreterPushArgsAndConstructDescriptor(isolate)); +Callable CodeFactory::InterpreterPushArgsAndConstruct(Isolate* isolate) { + return Callable(isolate->builtins()->InterpreterPushArgsAndConstruct(), + InterpreterPushArgsAndConstructDescriptor(isolate)); } diff --git a/src/code-factory.h b/src/code-factory.h index 1913089fe72..b6bbe8c305e 100644 --- a/src/code-factory.h +++ b/src/code-factory.h @@ -154,8 +154,7 @@ class CodeFactory final { static Callable InterpreterPushArgsAndCall( Isolate* isolate, TailCallMode tail_call_mode, CallableType function_type = CallableType::kAny); - static Callable InterpreterPushArgsAndConstruct( - Isolate* isolate, CallableType function_type = CallableType::kAny); + static Callable InterpreterPushArgsAndConstruct(Isolate* isolate); static Callable InterpreterCEntry(Isolate* isolate, int result_size = 1); }; diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc index 2dae9a85394..8d58c288b20 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/src/compiler/bytecode-graph-builder.cc @@ -1060,17 +1060,12 @@ void BytecodeGraphBuilder::VisitNew() { interpreter::Register callee_reg = bytecode_iterator().GetRegisterOperand(0); interpreter::Register first_arg = bytecode_iterator().GetRegisterOperand(1); size_t arg_count = bytecode_iterator().GetRegisterCountOperand(2); - // Slot index of 0 is used indicate no feedback slot is available. Assert - // the assumption that slot index 0 is never a valid feedback slot. - STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0); - VectorSlotPair feedback = - CreateVectorSlotPair(bytecode_iterator().GetIndexOperand(3)); Node* new_target = environment()->LookupAccumulator(); Node* callee = environment()->LookupRegister(callee_reg); - - const Operator* call = - javascript()->CallConstruct(static_cast(arg_count) + 2, feedback); + // TODO(turbofan): Pass the feedback here. + const Operator* call = javascript()->CallConstruct( + static_cast(arg_count) + 2, VectorSlotPair()); Node* value = ProcessCallNewArguments(call, callee, new_target, first_arg, arg_count + 2); environment()->BindAccumulator(value, &states); diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc index 701149499a3..4194e85c259 100644 --- a/src/interpreter/bytecode-array-builder.cc +++ b/src/interpreter/bytecode-array-builder.cc @@ -569,15 +569,13 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::Call(Register callable, BytecodeArrayBuilder& BytecodeArrayBuilder::New(Register constructor, Register first_arg, - size_t arg_count, - int feedback_slot_id) { + size_t arg_count) { if (!first_arg.is_valid()) { DCHECK_EQ(0u, arg_count); first_arg = Register(0); } Output(Bytecode::kNew, RegisterOperand(constructor), - RegisterOperand(first_arg), UnsignedOperand(arg_count), - UnsignedOperand(feedback_slot_id)); + RegisterOperand(first_arg), UnsignedOperand(arg_count)); return *this; } diff --git a/src/interpreter/bytecode-array-builder.h b/src/interpreter/bytecode-array-builder.h index 991c9e6b958..ca62ba6b679 100644 --- a/src/interpreter/bytecode-array-builder.h +++ b/src/interpreter/bytecode-array-builder.h @@ -172,7 +172,7 @@ class BytecodeArrayBuilder final : public ZoneObject { // consecutive arguments starting at |first_arg| for the constuctor // invocation. BytecodeArrayBuilder& New(Register constructor, Register first_arg, - size_t arg_count, int feedback_slot); + size_t arg_count); // Call the runtime function with |function_id|. The first argument should be // in |first_arg| and all subsequent arguments should be in registers diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index d0b5183988a..ab8868977e5 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -2527,7 +2527,7 @@ void BytecodeGenerator::VisitCall(Call* expr) { if (expr->CallFeedbackICSlot().IsInvalid()) { DCHECK(call_type == Call::POSSIBLY_EVAL_CALL); // Valid type feedback slots can only be greater than kReservedIndexCount. - // We use 0 to indicate an invalid slot id. Statically assert that 0 cannot + // We use 0 to indicate an invalid slot it. Statically assert that 0 cannot // be a valid slot id. STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0); feedback_slot_index = 0; @@ -2562,13 +2562,7 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) { // Call construct. builder()->SetExpressionPosition(expr); - // Valid type feedback slots can only be greater than kReservedIndexCount. - // Assert that 0 cannot be valid a valid slot id. - STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0); - // Type feedback is not necessary for super constructor calls. The type - // information can be inferred in most cases. Slot id 0 indicates type - // feedback is not required. - builder()->New(constructor, first_arg, args->length(), 0); + builder()->New(constructor, first_arg, args->length()); execution_result()->SetResultInAccumulator(); } @@ -2585,8 +2579,7 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) { // constructor for CallNew. builder() ->LoadAccumulatorWithRegister(constructor) - .New(constructor, first_arg, args->length(), - feedback_index(expr->CallNewFeedbackSlot())); + .New(constructor, first_arg, args->length()); execution_result()->SetResultInAccumulator(); } diff --git a/src/interpreter/bytecodes.h b/src/interpreter/bytecodes.h index c63f1ac66bf..75c38af1660 100644 --- a/src/interpreter/bytecodes.h +++ b/src/interpreter/bytecodes.h @@ -197,7 +197,7 @@ namespace interpreter { \ /* New operator */ \ V(New, AccumulatorUse::kReadWrite, OperandType::kReg, \ - OperandType::kMaybeReg, OperandType::kRegCount, OperandType::kIdx) \ + OperandType::kMaybeReg, OperandType::kRegCount) \ \ /* Test Operators */ \ V(TestEqual, AccumulatorUse::kReadWrite, OperandType::kReg) \ diff --git a/src/interpreter/interpreter-assembler.cc b/src/interpreter/interpreter-assembler.cc index fe2cc01c82a..8f5d941e5a2 100644 --- a/src/interpreter/interpreter-assembler.cc +++ b/src/interpreter/interpreter-assembler.cc @@ -467,7 +467,7 @@ Node* InterpreterAssembler::CallJSWithFeedback(Node* function, Node* context, Node* is_feedback_unavailable = Word32Equal(slot_id, Int32Constant(0)); GotoIf(is_feedback_unavailable, &call); - // The checks. First, does function match the recorded monomorphic target? + // The checks. First, does rdi match the recorded monomorphic target? Node* feedback_element = LoadFixedArrayElement(type_feedback_vector, slot_id); Node* feedback_value = LoadWeakCellValue(feedback_element); Node* is_monomorphic = WordEqual(function, feedback_value); @@ -546,7 +546,6 @@ Node* InterpreterAssembler::CallJSWithFeedback(Node* function, Node* context, StoreFixedArrayElement(type_feedback_vector, call_count_slot, SmiTag(Int32Constant(1)), SKIP_WRITE_BARRIER); - // TODO(mythria): Inline the weak cell creation/registration. CreateWeakCellStub weak_cell_stub(isolate()); CallStub(weak_cell_stub.GetCallInterfaceDescriptor(), HeapConstant(weak_cell_stub.GetCode()), context, @@ -604,150 +603,11 @@ Node* InterpreterAssembler::CallJS(Node* function, Node* context, Node* InterpreterAssembler::CallConstruct(Node* constructor, Node* context, Node* new_target, Node* first_arg, - Node* arg_count, Node* slot_id, - Node* type_feedback_vector) { - Label call_construct(this), js_function(this), end(this); - Variable return_value(this, MachineRepresentation::kTagged); - - // Slot id of 0 is used to indicate no typefeedback is available. - STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0); - Node* is_feedback_unavailable = Word32Equal(slot_id, Int32Constant(0)); - GotoIf(is_feedback_unavailable, &call_construct); - - // Check that the constructor is not a smi. - Node* is_smi = WordIsSmi(constructor); - GotoIf(is_smi, &call_construct); - - // Check that constructor is a JSFunction. - Node* instance_type = LoadInstanceType(constructor); - Node* is_js_function = - WordEqual(instance_type, Int32Constant(JS_FUNCTION_TYPE)); - BranchIf(is_js_function, &js_function, &call_construct); - - Bind(&js_function); - // Cache the called function in a feedback vector slot. Cache states - // are uninitialized, monomorphic (indicated by a JSFunction), and - // megamorphic. - // TODO(mythria/v8:5210): Check if it is better to mark extra_checks as a - // deferred block so that call_construct_function will be scheduled just - // after increment_count and in the fast path we can reduce one branch in the - // fast path. - Label increment_count(this), extra_checks(this), - call_construct_function(this); - - Node* feedback_element = LoadFixedArrayElement(type_feedback_vector, slot_id); - Node* feedback_value = LoadWeakCellValue(feedback_element); - Node* is_monomorphic = WordEqual(constructor, feedback_value); - BranchIf(is_monomorphic, &increment_count, &extra_checks); - - Bind(&increment_count); - { - // Increment the call count. - Comment("increment call count"); - Node* call_count_slot = IntPtrAdd(slot_id, IntPtrConstant(1)); - Node* call_count = - LoadFixedArrayElement(type_feedback_vector, call_count_slot); - Node* new_count = SmiAdd(call_count, SmiTag(Int32Constant(1))); - // Count is Smi, so we don't need a write barrier. - StoreFixedArrayElement(type_feedback_vector, call_count_slot, new_count, - SKIP_WRITE_BARRIER); - Goto(&call_construct_function); - } - - Bind(&extra_checks); - { - Label mark_megamorphic(this), initialize(this), - check_weak_cell_cleared(this); - // Check if it is a megamorphic target - Comment("check if megamorphic"); - Node* is_megamorphic = WordEqual( - feedback_element, - HeapConstant(TypeFeedbackVector::MegamorphicSentinel(isolate()))); - GotoIf(is_megamorphic, &call_construct_function); - - // Check if it is uninitialized. - Comment("check if uninitialized"); - Node* is_uninitialized = WordEqual( - feedback_element, LoadRoot(Heap::kuninitialized_symbolRootIndex)); - BranchIf(is_uninitialized, &initialize, &check_weak_cell_cleared); - - Bind(&check_weak_cell_cleared); - { - Comment("check if weak cell"); - Node* is_weak_cell = WordEqual(LoadMap(feedback_element), - LoadRoot(Heap::kWeakCellMapRootIndex)); - GotoUnless(is_weak_cell, &mark_megamorphic); - - // If the weak cell is cleared, we have a new chance to become - // monomorphic. - Comment("check if weak cell is not cleared"); - Node* is_smi = WordIsSmi(feedback_value); - BranchIf(is_smi, &initialize, &mark_megamorphic); - } - - Bind(&initialize); - { - // Check that it is not the Array() function. - Comment("check it is not Array()"); - Node* context_slot = - LoadFixedArrayElement(LoadNativeContext(context), - Int32Constant(Context::ARRAY_FUNCTION_INDEX)); - Node* is_array_function = WordEqual(context_slot, constructor); - GotoIf(is_array_function, &mark_megamorphic); - - Node* call_count_slot = IntPtrAdd(slot_id, IntPtrConstant(1)); - // Count is Smi, so we don't need a write barrier. - StoreFixedArrayElement(type_feedback_vector, call_count_slot, - SmiTag(Int32Constant(1)), SKIP_WRITE_BARRIER); - - // TODO(mythria): Inline the weak cell creation/registration. - CreateWeakCellStub weak_cell_stub(isolate()); - CallStub(weak_cell_stub.GetCallInterfaceDescriptor(), - HeapConstant(weak_cell_stub.GetCode()), context, - type_feedback_vector, SmiTag(slot_id), constructor); - Goto(&call_construct_function); - } - - Bind(&mark_megamorphic); - { - // MegamorphicSentinel is an immortal immovable object so no write-barrier - // is needed. - Comment("transition to megamorphic"); - DCHECK(Heap::RootIsImmortalImmovable(Heap::kmegamorphic_symbolRootIndex)); - StoreFixedArrayElement( - type_feedback_vector, slot_id, - HeapConstant(TypeFeedbackVector::MegamorphicSentinel(isolate())), - SKIP_WRITE_BARRIER); - Goto(&call_construct_function); - } - } - - Bind(&call_construct_function); - { - // TODO(mythria): Get allocation site feedback if available. Currently - // we do not collect allocation site feedback. - Comment("call using callConstructFunction"); - Callable callable_function = CodeFactory::InterpreterPushArgsAndConstruct( - isolate(), CallableType::kJSFunction); - return_value.Bind(CallStub(callable_function.descriptor(), - HeapConstant(callable_function.code()), context, - arg_count, new_target, constructor, first_arg)); - Goto(&end); - } - - Bind(&call_construct); - { - Comment("call using callConstruct builtin"); - Callable callable = CodeFactory::InterpreterPushArgsAndConstruct( - isolate(), CallableType::kAny); - Node* code_target = HeapConstant(callable.code()); - return_value.Bind(CallStub(callable.descriptor(), code_target, context, - arg_count, new_target, constructor, first_arg)); - Goto(&end); - } - - Bind(&end); - return return_value.value(); + Node* arg_count) { + Callable callable = CodeFactory::InterpreterPushArgsAndConstruct(isolate()); + Node* code_target = HeapConstant(callable.code()); + return CallStub(callable.descriptor(), code_target, context, arg_count, + new_target, constructor, first_arg); } Node* InterpreterAssembler::CallRuntimeN(Node* function_id, Node* context, diff --git a/src/interpreter/interpreter-assembler.h b/src/interpreter/interpreter-assembler.h index b0d29eb48e9..1b77b52d885 100644 --- a/src/interpreter/interpreter-assembler.h +++ b/src/interpreter/interpreter-assembler.h @@ -118,9 +118,7 @@ class InterpreterAssembler : public CodeStubAssembler { compiler::Node* context, compiler::Node* new_target, compiler::Node* first_arg, - compiler::Node* arg_count, - compiler::Node* slot_id, - compiler::Node* type_feedback_vector); + compiler::Node* arg_count); // Call runtime function with |arg_count| arguments and the first argument // located at |first_arg|. diff --git a/src/interpreter/interpreter.cc b/src/interpreter/interpreter.cc index bc57e9c3a9a..4830f1dc72e 100644 --- a/src/interpreter/interpreter.cc +++ b/src/interpreter/interpreter.cc @@ -1285,11 +1285,9 @@ void Interpreter::DoCallConstruct(InterpreterAssembler* assembler) { Node* first_arg_reg = __ BytecodeOperandReg(1); Node* first_arg = __ RegisterLocation(first_arg_reg); Node* args_count = __ BytecodeOperandCount(2); - Node* slot_id = __ BytecodeOperandIdx(3); - Node* type_feedback_vector = __ LoadTypeFeedbackVector(); Node* context = __ GetContext(); - Node* result = __ CallConstruct(constructor, context, new_target, first_arg, - args_count, slot_id, type_feedback_vector); + Node* result = + __ CallConstruct(constructor, context, new_target, first_arg, args_count); __ SetAccumulator(result); __ Dispatch(); } diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 41d1f96ac20..547e5d50734 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -134,6 +134,12 @@ # TODO(mythria,4780): Related to type feedback support for Array function. 'test-feedback-vector/VectorCallFeedbackForArray': [PASS, NO_IGNITION], + # TODO(mythria,4780): Related to type feedback support for constructor. + 'test-feedback-vector/VectorConstructCounts': [PASS, NO_IGNITION], + 'test-heap/WeakFunctionInConstructor': [PASS, NO_IGNITION], + 'test-heap/IncrementalMarkingClearsMonomorphicConstructor': [PASS, NO_IGNITION], + 'test-heap/IncrementalMarkingPreservesMonomorphicConstructor': [PASS, NO_IGNITION], + # TODO(mythria,4680): Lack of code-ageing in interpreter. 'test-heap/Regress169209': [PASS, NO_IGNITION], @@ -410,6 +416,12 @@ # TODO(mythria,4780): Related to type feedback support for Array function. 'test-feedback-vector/VectorCallFeedbackForArray': [FAIL], + # TODO(mythria,4780): Related to type feedback support for constructor. + 'test-feedback-vector/VectorConstructCounts': [FAIL], + 'test-heap/WeakFunctionInConstructor': [FAIL], + 'test-heap/IncrementalMarkingClearsMonomorphicConstructor': [FAIL], + 'test-heap/IncrementalMarkingPreservesMonomorphicConstructor': [FAIL], + # TODO(mythria,4680): Lack of code-ageing in interpreter. 'test-heap/Regress169209': [FAIL], diff --git a/test/cctest/interpreter/bytecode_expectations/CallNew.golden b/test/cctest/interpreter/bytecode_expectations/CallNew.golden index 1253c9b3e66..2ee9613b59b 100644 --- a/test/cctest/interpreter/bytecode_expectations/CallNew.golden +++ b/test/cctest/interpreter/bytecode_expectations/CallNew.golden @@ -16,12 +16,12 @@ snippet: " " frame size: 1 parameter count: 1 -bytecode array length: 12 +bytecode array length: 11 bytecodes: [ /* 45 E> */ B(StackCheck), /* 50 S> */ B(LdrGlobal), U8(3), R(0), B(Ldar), R(0), - /* 57 E> */ B(New), R(0), R(0), U8(0), U8(1), + /* 57 E> */ B(New), R(0), R(0), U8(0), /* 68 S> */ B(Return), ] constant pool: [ @@ -37,14 +37,14 @@ snippet: " " frame size: 2 parameter count: 1 -bytecode array length: 16 +bytecode array length: 15 bytecodes: [ /* 58 E> */ B(StackCheck), /* 63 S> */ B(LdrGlobal), U8(3), R(0), B(LdaSmi), U8(3), B(Star), R(1), B(Ldar), R(0), - /* 70 E> */ B(New), R(0), R(1), U8(1), U8(1), + /* 70 E> */ B(New), R(0), R(1), U8(1), /* 82 S> */ B(Return), ] constant pool: [ @@ -65,7 +65,7 @@ snippet: " " frame size: 4 parameter count: 1 -bytecode array length: 24 +bytecode array length: 23 bytecodes: [ /* 100 E> */ B(StackCheck), /* 105 S> */ B(LdrGlobal), U8(3), R(0), @@ -76,7 +76,7 @@ bytecodes: [ B(LdaSmi), U8(5), B(Star), R(3), B(Ldar), R(0), - /* 112 E> */ B(New), R(0), R(1), U8(3), U8(1), + /* 112 E> */ B(New), R(0), R(1), U8(3), /* 130 S> */ B(Return), ] constant pool: [ diff --git a/test/cctest/interpreter/bytecode_expectations/ClassAndSuperClass.golden b/test/cctest/interpreter/bytecode_expectations/ClassAndSuperClass.golden index ef1470db46e..1092d91baf4 100644 --- a/test/cctest/interpreter/bytecode_expectations/ClassAndSuperClass.golden +++ b/test/cctest/interpreter/bytecode_expectations/ClassAndSuperClass.golden @@ -127,7 +127,7 @@ snippet: " " frame size: 5 parameter count: 1 -bytecode array length: 106 +bytecode array length: 105 bytecodes: [ B(Mov), R(closure), R(1), B(Mov), R(new_target), R(0), @@ -147,7 +147,7 @@ bytecodes: [ B(LdaConstant), U8(1), B(Star), R(4), /* 118 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(4), U8(1), - /* 118 E> */ B(New), R(2), R(3), U8(1), U8(0), + /* 118 E> */ B(New), R(2), R(3), U8(1), B(Star), R(2), B(Ldar), R(this), B(JumpIfNotHole), U8(4), @@ -195,7 +195,7 @@ snippet: " " frame size: 4 parameter count: 1 -bytecode array length: 102 +bytecode array length: 101 bytecodes: [ B(Mov), R(closure), R(1), B(Mov), R(new_target), R(0), @@ -213,7 +213,7 @@ bytecodes: [ B(LdaConstant), U8(1), B(Star), R(3), /* 117 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1), - /* 117 E> */ B(New), R(2), R(0), U8(0), U8(0), + /* 117 E> */ B(New), R(2), R(0), U8(0), B(Star), R(2), B(Ldar), R(this), B(JumpIfNotHole), U8(4), diff --git a/test/cctest/interpreter/bytecode_expectations/ClassDeclarations.golden b/test/cctest/interpreter/bytecode_expectations/ClassDeclarations.golden index 1f0ec5d5a3a..7195db3ef0a 100644 --- a/test/cctest/interpreter/bytecode_expectations/ClassDeclarations.golden +++ b/test/cctest/interpreter/bytecode_expectations/ClassDeclarations.golden @@ -195,7 +195,7 @@ snippet: " " frame size: 7 parameter count: 1 -bytecode array length: 74 +bytecode array length: 73 bytecodes: [ B(CallRuntime), U16(Runtime::kNewFunctionContext), R(closure), U8(1), B(PushContext), R(2), @@ -225,7 +225,7 @@ bytecodes: [ B(Star), R(4), B(CallRuntime), U16(Runtime::kThrowReferenceError), R(4), U8(1), B(Star), R(3), - /* 94 E> */ B(New), R(3), R(0), U8(0), U8(3), + /* 94 E> */ B(New), R(3), R(0), U8(0), /* 103 S> */ B(Return), ] constant pool: [ diff --git a/test/unittests/interpreter/bytecode-array-builder-unittest.cc b/test/unittests/interpreter/bytecode-array-builder-unittest.cc index ef4b1beb7b4..005dd935d90 100644 --- a/test/unittests/interpreter/bytecode-array-builder-unittest.cc +++ b/test/unittests/interpreter/bytecode-array-builder-unittest.cc @@ -165,8 +165,8 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) { builder.Delete(reg, LanguageMode::SLOPPY).Delete(reg, LanguageMode::STRICT); // Emit new. - builder.New(reg, reg, 0, 1); - builder.New(wide, wide, 0, 1); + builder.New(reg, reg, 0); + builder.New(wide, wide, 0); // Emit test operator invocations. builder.CompareOperation(Token::Value::EQ, reg) @@ -456,7 +456,7 @@ TEST_F(BytecodeArrayBuilderTest, FrameSizesLookGood) { // Ensure temporaries are used so not optimized away by the // register optimizer. builder.New(Register(locals + contexts), Register(locals + contexts), - static_cast(temps), 0); + static_cast(temps)); } builder.Return();