From 1eadc76419b323fb2e55ae9953142f801704aa59 Mon Sep 17 00:00:00 2001 From: mythria Date: Tue, 19 Jul 2016 04:09:55 -0700 Subject: [PATCH] [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 Review-Url: https://codereview.chromium.org/2153433002 Cr-Commit-Position: refs/heads/master@{#37862} --- 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, 299 insertions(+), 68 deletions(-) diff --git a/src/builtins/arm/builtins-arm.cc b/src/builtins/arm/builtins-arm.cc index 2c9c9c0b0c..51b0f45112 100644 --- a/src/builtins/arm/builtins-arm.cc +++ b/src/builtins/arm/builtins-arm.cc @@ -1206,7 +1206,8 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { +void Builtins::Generate_InterpreterPushArgsAndConstructImpl( + MacroAssembler* masm, CallableType construct_type) { // ----------- S t a t e ------------- // -- r0 : argument count (not including receiver) // -- r3 : new target @@ -1225,8 +1226,16 @@ void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // Push the arguments. Generate_InterpreterPushArgs(masm, r2, r4, r5); - // Call the constructor with r0, r1, and r3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); + 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); + } } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/arm64/builtins-arm64.cc b/src/builtins/arm64/builtins-arm64.cc index f4af8fdb9d..bd9706e6d8 100644 --- a/src/builtins/arm64/builtins-arm64.cc +++ b/src/builtins/arm64/builtins-arm64.cc @@ -1213,7 +1213,8 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { +void Builtins::Generate_InterpreterPushArgsAndConstructImpl( + MacroAssembler* masm, CallableType construct_type) { // ----------- S t a t e ------------- // -- x0 : argument count (not including receiver) // -- x3 : new target @@ -1244,8 +1245,16 @@ void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { __ Cmp(x6, x4); __ B(gt, &loop_header); - // Call the constructor with x0, x1, and x3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); + 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); + } } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/builtins.cc b/src/builtins/builtins.cc index ac8419798e..538c36d38c 100644 --- a/src/builtins/builtins.cc +++ b/src/builtins/builtins.cc @@ -4733,6 +4733,18 @@ 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 { @@ -5838,6 +5850,16 @@ 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 929446de1b..97e55fed37 100644 --- a/src/builtins/builtins.h +++ b/src/builtins/builtins.h @@ -129,6 +129,7 @@ namespace internal { ASM(InterpreterPushArgsAndCall) \ ASM(InterpreterPushArgsAndTailCall) \ ASM(InterpreterPushArgsAndConstruct) \ + ASM(InterpreterPushArgsAndConstructFunction) \ ASM(InterpreterEnterBytecodeDispatch) \ \ /* Code life-cycle */ \ @@ -552,6 +553,7 @@ 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 @@ -593,6 +595,9 @@ 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 b549ab1336..d94700866b 100644 --- a/src/builtins/ia32/builtins-ia32.cc +++ b/src/builtins/ia32/builtins-ia32.cc @@ -758,7 +758,8 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { +void Builtins::Generate_InterpreterPushArgsAndConstructImpl( + MacroAssembler* masm, CallableType construct_type) { // ----------- S t a t e ------------- // -- eax : the number of arguments (not including the receiver) // -- edx : the new target @@ -790,8 +791,17 @@ void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // Re-push return address. __ Push(ecx); - // Call the constructor with unmodified eax, edi, ebi values. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); + 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); + } } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/mips/builtins-mips.cc b/src/builtins/mips/builtins-mips.cc index 63c3329e24..e950112f9e 100644 --- a/src/builtins/mips/builtins-mips.cc +++ b/src/builtins/mips/builtins-mips.cc @@ -1198,7 +1198,8 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { +void Builtins::Generate_InterpreterPushArgsAndConstructImpl( + MacroAssembler* masm, CallableType construct_type) { // ----------- S t a t e ------------- // -- a0 : argument count (not including receiver) // -- a3 : new target @@ -1223,8 +1224,16 @@ void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { __ bind(&loop_check); __ Branch(&loop_header, gt, a2, Operand(t0)); - // Call the constructor with a0, a1, and a3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); + 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); + } } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/mips64/builtins-mips64.cc b/src/builtins/mips64/builtins-mips64.cc index ffbc63eaa9..d9dd399257 100644 --- a/src/builtins/mips64/builtins-mips64.cc +++ b/src/builtins/mips64/builtins-mips64.cc @@ -1190,7 +1190,8 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { +void Builtins::Generate_InterpreterPushArgsAndConstructImpl( + MacroAssembler* masm, CallableType construct_type) { // ----------- S t a t e ------------- // -- a0 : argument count (not including receiver) // -- a3 : new target @@ -1215,8 +1216,16 @@ void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { __ bind(&loop_check); __ Branch(&loop_header, gt, a2, Operand(t0)); - // Call the constructor with a0, a1, and a3 unmodified. - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); + 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); + } } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/builtins/x64/builtins-x64.cc b/src/builtins/x64/builtins-x64.cc index 1fb26bf8dd..42a613ee32 100644 --- a/src/builtins/x64/builtins-x64.cc +++ b/src/builtins/x64/builtins-x64.cc @@ -840,7 +840,8 @@ void Builtins::Generate_InterpreterPushArgsAndCallImpl( } // static -void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { +void Builtins::Generate_InterpreterPushArgsAndConstructImpl( + MacroAssembler* masm, CallableType construct_type) { // ----------- 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 @@ -862,8 +863,16 @@ void Builtins::Generate_InterpreterPushArgsAndConstruct(MacroAssembler* masm) { // Push return address in preparation for the tail-call. __ PushReturnAddressFrom(kScratchRegister); - // Call the constructor (rax, rdx, rdi passed on). - __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET); + 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); + } } void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) { diff --git a/src/code-factory.cc b/src/code-factory.cc index 19bb44228c..62ed852f85 100644 --- a/src/code-factory.cc +++ b/src/code-factory.cc @@ -581,9 +581,11 @@ Callable CodeFactory::InterpreterPushArgsAndCall(Isolate* isolate, // static -Callable CodeFactory::InterpreterPushArgsAndConstruct(Isolate* isolate) { - return Callable(isolate->builtins()->InterpreterPushArgsAndConstruct(), - InterpreterPushArgsAndConstructDescriptor(isolate)); +Callable CodeFactory::InterpreterPushArgsAndConstruct( + Isolate* isolate, CallableType function_type) { + return Callable( + isolate->builtins()->InterpreterPushArgsAndConstruct(function_type), + InterpreterPushArgsAndConstructDescriptor(isolate)); } diff --git a/src/code-factory.h b/src/code-factory.h index b6bbe8c305..1913089fe7 100644 --- a/src/code-factory.h +++ b/src/code-factory.h @@ -154,7 +154,8 @@ class CodeFactory final { static Callable InterpreterPushArgsAndCall( Isolate* isolate, TailCallMode tail_call_mode, CallableType function_type = CallableType::kAny); - static Callable InterpreterPushArgsAndConstruct(Isolate* isolate); + static Callable InterpreterPushArgsAndConstruct( + Isolate* isolate, CallableType function_type = CallableType::kAny); 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 8d58c288b2..2dae9a8539 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/src/compiler/bytecode-graph-builder.cc @@ -1060,12 +1060,17 @@ 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); - // TODO(turbofan): Pass the feedback here. - const Operator* call = javascript()->CallConstruct( - static_cast(arg_count) + 2, VectorSlotPair()); + + const Operator* call = + javascript()->CallConstruct(static_cast(arg_count) + 2, feedback); 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 4194e85c25..701149499a 100644 --- a/src/interpreter/bytecode-array-builder.cc +++ b/src/interpreter/bytecode-array-builder.cc @@ -569,13 +569,15 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::Call(Register callable, BytecodeArrayBuilder& BytecodeArrayBuilder::New(Register constructor, Register first_arg, - size_t arg_count) { + size_t arg_count, + int feedback_slot_id) { 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)); + RegisterOperand(first_arg), UnsignedOperand(arg_count), + UnsignedOperand(feedback_slot_id)); return *this; } diff --git a/src/interpreter/bytecode-array-builder.h b/src/interpreter/bytecode-array-builder.h index ca62ba6b67..991c9e6b95 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); + size_t arg_count, int feedback_slot); // 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 ab8868977e..d0b5183988 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 it. Statically assert that 0 cannot + // We use 0 to indicate an invalid slot id. Statically assert that 0 cannot // be a valid slot id. STATIC_ASSERT(TypeFeedbackVector::kReservedIndexCount > 0); feedback_slot_index = 0; @@ -2562,7 +2562,13 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) { // Call construct. builder()->SetExpressionPosition(expr); - builder()->New(constructor, first_arg, args->length()); + // 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); execution_result()->SetResultInAccumulator(); } @@ -2579,7 +2585,8 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) { // constructor for CallNew. builder() ->LoadAccumulatorWithRegister(constructor) - .New(constructor, first_arg, args->length()); + .New(constructor, first_arg, args->length(), + feedback_index(expr->CallNewFeedbackSlot())); execution_result()->SetResultInAccumulator(); } diff --git a/src/interpreter/bytecodes.h b/src/interpreter/bytecodes.h index 43aeb30d3b..f8d0e17204 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::kMaybeReg, OperandType::kRegCount, OperandType::kIdx) \ \ /* Test Operators */ \ V(TestEqual, AccumulatorUse::kReadWrite, OperandType::kReg) \ diff --git a/src/interpreter/interpreter-assembler.cc b/src/interpreter/interpreter-assembler.cc index 8f5d941e5a..fe2cc01c82 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 rdi match the recorded monomorphic target? + // The checks. First, does function 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,6 +546,7 @@ 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, @@ -603,11 +604,150 @@ Node* InterpreterAssembler::CallJS(Node* function, Node* context, Node* InterpreterAssembler::CallConstruct(Node* constructor, Node* context, Node* new_target, Node* first_arg, - 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* 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* InterpreterAssembler::CallRuntimeN(Node* function_id, Node* context, diff --git a/src/interpreter/interpreter-assembler.h b/src/interpreter/interpreter-assembler.h index 1b77b52d88..b0d29eb48e 100644 --- a/src/interpreter/interpreter-assembler.h +++ b/src/interpreter/interpreter-assembler.h @@ -118,7 +118,9 @@ class InterpreterAssembler : public CodeStubAssembler { compiler::Node* context, compiler::Node* new_target, compiler::Node* first_arg, - compiler::Node* arg_count); + compiler::Node* arg_count, + compiler::Node* slot_id, + compiler::Node* type_feedback_vector); // 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 4830f1dc72..bc57e9c3a9 100644 --- a/src/interpreter/interpreter.cc +++ b/src/interpreter/interpreter.cc @@ -1285,9 +1285,11 @@ 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); + Node* result = __ CallConstruct(constructor, context, new_target, first_arg, + args_count, slot_id, type_feedback_vector); __ SetAccumulator(result); __ Dispatch(); } diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 547e5d5073..41d1f96ac2 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -134,12 +134,6 @@ # 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], @@ -416,12 +410,6 @@ # 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 2ee9613b59..1253c9b3e6 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: 11 +bytecode array length: 12 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), + /* 57 E> */ B(New), R(0), R(0), U8(0), U8(1), /* 68 S> */ B(Return), ] constant pool: [ @@ -37,14 +37,14 @@ snippet: " " frame size: 2 parameter count: 1 -bytecode array length: 15 +bytecode array length: 16 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), + /* 70 E> */ B(New), R(0), R(1), U8(1), U8(1), /* 82 S> */ B(Return), ] constant pool: [ @@ -65,7 +65,7 @@ snippet: " " frame size: 4 parameter count: 1 -bytecode array length: 23 +bytecode array length: 24 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), + /* 112 E> */ B(New), R(0), R(1), U8(3), U8(1), /* 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 1092d91baf..ef1470db46 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: 105 +bytecode array length: 106 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), + /* 118 E> */ B(New), R(2), R(3), U8(1), U8(0), 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: 101 +bytecode array length: 102 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), + /* 117 E> */ B(New), R(2), R(0), U8(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 7195db3ef0..1f0ec5d5a3 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: 73 +bytecode array length: 74 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), + /* 94 E> */ B(New), R(3), R(0), U8(0), U8(3), /* 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 005dd935d9..ef4b1beb7b 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); - builder.New(wide, wide, 0); + builder.New(reg, reg, 0, 1); + builder.New(wide, wide, 0, 1); // 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)); + static_cast(temps), 0); } builder.Return();