From f2f46aff8b776365e0bafd7826f23c8b9c16d928 Mon Sep 17 00:00:00 2001 From: mvstanton Date: Tue, 13 Oct 2015 07:48:33 -0700 Subject: [PATCH] VectorICs: use a vector slot to aid in array literal processing. The lack of a vector slot for the keyed store operation in filling in non-constant array literal properties led to undesirable contortions in compilers downwind of full-codegen. The use of a single slot to initialize all the array elements is sufficient. BUG= Review URL: https://codereview.chromium.org/1405503002 Cr-Commit-Position: refs/heads/master@{#31242} --- src/ast-numbering.cc | 2 ++ src/ast.cc | 21 +++++++++++++++++++ src/ast.h | 5 +++++ src/compiler/ast-graph-builder.cc | 9 +++----- src/compiler/js-generic-lowering.cc | 10 +++------ src/full-codegen/arm/full-codegen-arm.cc | 12 ++++++++--- src/full-codegen/arm64/full-codegen-arm64.cc | 10 +++++++-- src/full-codegen/ia32/full-codegen-ia32.cc | 18 +++++++++------- src/full-codegen/mips/full-codegen-mips.cc | 13 +++++++++--- .../mips64/full-codegen-mips64.cc | 13 +++++++++--- src/full-codegen/x64/full-codegen-x64.cc | 10 +++++++-- src/hydrogen.cc | 1 - 12 files changed, 90 insertions(+), 34 deletions(-) diff --git a/src/ast-numbering.cc b/src/ast-numbering.cc index c328ef1efd..95059aa317 100644 --- a/src/ast-numbering.cc +++ b/src/ast-numbering.cc @@ -489,6 +489,8 @@ void AstNumberingVisitor::VisitArrayLiteral(ArrayLiteral* node) { for (int i = 0; i < node->values()->length(); i++) { Visit(node->values()->at(i)); } + node->BuildConstantElements(isolate()); + ReserveFeedbackSlots(node); } diff --git a/src/ast.cc b/src/ast.cc index 91b5c30dff..f94e370f3c 100644 --- a/src/ast.cc +++ b/src/ast.cc @@ -547,6 +547,27 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate) { } +void ArrayLiteral::AssignFeedbackVectorSlots(Isolate* isolate, + FeedbackVectorSpec* spec, + FeedbackVectorSlotCache* cache) { + if (!FLAG_vector_stores) return; + + // This logic that computes the number of slots needed for vector store + // ics must mirror FullCodeGenerator::VisitArrayLiteral. + int array_index = 0; + for (; array_index < values()->length(); array_index++) { + Expression* subexpr = values()->at(array_index); + if (subexpr->IsSpread()) break; + if (CompileTimeValue::IsCompileTimeValue(subexpr)) continue; + + // We'll reuse the same literal slot for all of the non-constant + // subexpressions that use a keyed store IC. + literal_slot_ = spec->AddKeyedStoreICSlot(); + return; + } +} + + Handle MaterializedLiteral::GetBoilerplateValue(Expression* expression, Isolate* isolate) { if (expression->IsLiteral()) { diff --git a/src/ast.h b/src/ast.h index af6ecb8d10..ffae46225b 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1584,6 +1584,10 @@ class ArrayLiteral final : public MaterializedLiteral { kIsStrong = 1 << 2 }; + void AssignFeedbackVectorSlots(Isolate* isolate, FeedbackVectorSpec* spec, + FeedbackVectorSlotCache* cache) override; + FeedbackVectorSlot LiteralFeedbackSlot() const { return literal_slot_; } + protected: ArrayLiteral(Zone* zone, ZoneList* values, int first_spread_index, int literal_index, bool is_strong, @@ -1599,6 +1603,7 @@ class ArrayLiteral final : public MaterializedLiteral { Handle constant_elements_; ZoneList* values_; int first_spread_index_; + FeedbackVectorSlot literal_slot_; }; diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index f2e10ecfe5..d0ebd6bf6f 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -1911,7 +1911,6 @@ void AstGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) { Node* closure = GetFunctionClosure(); // Create node to deep-copy the literal boilerplate. - expr->BuildConstantElements(isolate()); Node* literals_array = BuildLoadObjectField(closure, JSFunction::kLiteralsOffset); Node* literal_index = jsgraph()->Constant(expr->literal_index()); @@ -1938,13 +1937,11 @@ void AstGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) { VisitForValue(subexpr); { FrameStateBeforeAndAfter states(this, subexpr->id()); + VectorSlotPair pair = CreateVectorSlotPair(expr->LiteralFeedbackSlot()); Node* value = environment()->Pop(); Node* index = jsgraph()->Constant(array_index); - // TODO(turbofan): More efficient code could be generated here. Consider - // that the store will be generic because we don't have a feedback vector - // slot. - Node* store = BuildKeyedStore(literal, index, value, VectorSlotPair(), - TypeFeedbackId::None()); + Node* store = + BuildKeyedStore(literal, index, value, pair, TypeFeedbackId::None()); states.AddToNode(store, expr->GetIdForElement(array_index), OutputFrameStateCombine::Ignore()); } diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index eac0565786..94b1c4e9da 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/src/compiler/js-generic-lowering.cc @@ -332,14 +332,10 @@ void JSGenericLowering::LowerJSStoreProperty(Node* node) { CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); const StorePropertyParameters& p = StorePropertyParametersOf(node->op()); LanguageMode language_mode = p.language_mode(); - // We have a special case where we do keyed stores but don't have a type - // feedback vector slot allocated to support it. In this case, install - // the megamorphic keyed store stub which needs neither vector nor slot. - bool use_vector_slot = FLAG_vector_stores && p.feedback().index() != -1; Callable callable = CodeFactory::KeyedStoreICInOptimizedCode( - isolate(), language_mode, - (use_vector_slot || !FLAG_vector_stores) ? UNINITIALIZED : MEGAMORPHIC); - if (use_vector_slot) { + isolate(), language_mode, UNINITIALIZED); + if (FLAG_vector_stores) { + DCHECK(p.feedback().index() != -1); node->InsertInput(zone(), 3, jsgraph()->SmiConstant(p.feedback().index())); } else { node->RemoveInput(3); diff --git a/src/full-codegen/arm/full-codegen-arm.cc b/src/full-codegen/arm/full-codegen-arm.cc index 26e929a057..f6c38cf7bf 100644 --- a/src/full-codegen/arm/full-codegen-arm.cc +++ b/src/full-codegen/arm/full-codegen-arm.cc @@ -1752,8 +1752,6 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - expr->BuildConstantElements(isolate()); - Handle constant_elements = expr->constant_elements(); bool has_fast_elements = IsFastObjectElementsKind(expr->constant_elements_kind()); @@ -1803,7 +1801,15 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } VisitForAccumulatorValue(subexpr); - if (has_fast_elements) { + if (FLAG_vector_stores) { + __ mov(StoreDescriptor::NameRegister(), + Operand(Smi::FromInt(array_index))); + __ ldr(StoreDescriptor::ReceiverRegister(), MemOperand(sp, kPointerSize)); + EmitLoadStoreICSlot(expr->LiteralFeedbackSlot()); + Handle ic = + CodeFactory::KeyedStoreIC(isolate(), language_mode()).code(); + CallIC(ic); + } else if (has_fast_elements) { int offset = FixedArray::kHeaderSize + (array_index * kPointerSize); __ ldr(r6, MemOperand(sp, kPointerSize)); // Copy of array literal. __ ldr(r1, FieldMemOperand(r6, JSObject::kElementsOffset)); diff --git a/src/full-codegen/arm64/full-codegen-arm64.cc b/src/full-codegen/arm64/full-codegen-arm64.cc index fa5ead9531..c8023dea97 100644 --- a/src/full-codegen/arm64/full-codegen-arm64.cc +++ b/src/full-codegen/arm64/full-codegen-arm64.cc @@ -1736,7 +1736,6 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - expr->BuildConstantElements(isolate()); Handle constant_elements = expr->constant_elements(); bool has_fast_elements = IsFastObjectElementsKind(expr->constant_elements_kind()); @@ -1784,7 +1783,14 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } VisitForAccumulatorValue(subexpr); - if (has_fast_elements) { + if (FLAG_vector_stores) { + __ Mov(StoreDescriptor::NameRegister(), Smi::FromInt(array_index)); + __ Peek(StoreDescriptor::ReceiverRegister(), kPointerSize); + EmitLoadStoreICSlot(expr->LiteralFeedbackSlot()); + Handle ic = + CodeFactory::KeyedStoreIC(isolate(), language_mode()).code(); + CallIC(ic); + } else if (has_fast_elements) { int offset = FixedArray::kHeaderSize + (array_index * kPointerSize); __ Peek(x6, kPointerSize); // Copy of array literal. __ Ldr(x1, FieldMemOperand(x6, JSObject::kElementsOffset)); diff --git a/src/full-codegen/ia32/full-codegen-ia32.cc b/src/full-codegen/ia32/full-codegen-ia32.cc index 194267de35..953ba260be 100644 --- a/src/full-codegen/ia32/full-codegen-ia32.cc +++ b/src/full-codegen/ia32/full-codegen-ia32.cc @@ -1676,7 +1676,6 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - expr->BuildConstantElements(isolate()); Handle constant_elements = expr->constant_elements(); bool has_constant_fast_elements = IsFastObjectElementsKind(expr->constant_elements_kind()); @@ -1727,7 +1726,15 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } VisitForAccumulatorValue(subexpr); - if (has_constant_fast_elements) { + if (FLAG_vector_stores) { + __ mov(StoreDescriptor::NameRegister(), + Immediate(Smi::FromInt(array_index))); + __ mov(StoreDescriptor::ReceiverRegister(), Operand(esp, kPointerSize)); + EmitLoadStoreICSlot(expr->LiteralFeedbackSlot()); + Handle ic = + CodeFactory::KeyedStoreIC(isolate(), language_mode()).code(); + CallIC(ic); + } else if (has_constant_fast_elements) { // Fast-case array literal with ElementsKind of FAST_*_ELEMENTS, they // cannot transition and don't need to call the runtime stub. int offset = FixedArray::kHeaderSize + (array_index * kPointerSize); @@ -1736,17 +1743,14 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { // Store the subexpression value in the array's elements. __ mov(FieldOperand(ebx, offset), result_register()); // Update the write barrier for the array store. - __ RecordWriteField(ebx, offset, result_register(), ecx, - kDontSaveFPRegs, - EMIT_REMEMBERED_SET, - INLINE_SMI_CHECK); + __ RecordWriteField(ebx, offset, result_register(), ecx, kDontSaveFPRegs, + EMIT_REMEMBERED_SET, INLINE_SMI_CHECK); } else { // Store the subexpression value in the array's elements. __ mov(ecx, Immediate(Smi::FromInt(array_index))); StoreArrayLiteralElementStub stub(isolate()); __ CallStub(&stub); } - PrepareForBailoutForId(expr->GetIdForElement(array_index), NO_REGISTERS); } diff --git a/src/full-codegen/mips/full-codegen-mips.cc b/src/full-codegen/mips/full-codegen-mips.cc index fdee8cdb16..e8092a87a8 100644 --- a/src/full-codegen/mips/full-codegen-mips.cc +++ b/src/full-codegen/mips/full-codegen-mips.cc @@ -1749,8 +1749,6 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - expr->BuildConstantElements(isolate()); - Handle constant_elements = expr->constant_elements(); bool has_fast_elements = IsFastObjectElementsKind(expr->constant_elements_kind()); @@ -1800,7 +1798,16 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { VisitForAccumulatorValue(subexpr); - if (has_fast_elements) { + if (FLAG_vector_stores) { + __ li(StoreDescriptor::NameRegister(), + Operand(Smi::FromInt(array_index))); + __ lw(StoreDescriptor::ReceiverRegister(), MemOperand(sp, kPointerSize)); + __ mov(StoreDescriptor::ValueRegister(), result_register()); + EmitLoadStoreICSlot(expr->LiteralFeedbackSlot()); + Handle ic = + CodeFactory::KeyedStoreIC(isolate(), language_mode()).code(); + CallIC(ic); + } else if (has_fast_elements) { int offset = FixedArray::kHeaderSize + (array_index * kPointerSize); __ lw(t2, MemOperand(sp, kPointerSize)); // Copy of array literal. __ lw(a1, FieldMemOperand(t2, JSObject::kElementsOffset)); diff --git a/src/full-codegen/mips64/full-codegen-mips64.cc b/src/full-codegen/mips64/full-codegen-mips64.cc index e5d8beb3e7..a3b8321d47 100644 --- a/src/full-codegen/mips64/full-codegen-mips64.cc +++ b/src/full-codegen/mips64/full-codegen-mips64.cc @@ -1747,8 +1747,6 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - expr->BuildConstantElements(isolate()); - Handle constant_elements = expr->constant_elements(); bool has_fast_elements = IsFastObjectElementsKind(expr->constant_elements_kind()); @@ -1798,7 +1796,16 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { VisitForAccumulatorValue(subexpr); - if (has_fast_elements) { + if (FLAG_vector_stores) { + __ li(StoreDescriptor::NameRegister(), + Operand(Smi::FromInt(array_index))); + __ ld(StoreDescriptor::ReceiverRegister(), MemOperand(sp, kPointerSize)); + __ mov(StoreDescriptor::ValueRegister(), result_register()); + EmitLoadStoreICSlot(expr->LiteralFeedbackSlot()); + Handle ic = + CodeFactory::KeyedStoreIC(isolate(), language_mode()).code(); + CallIC(ic); + } else if (has_fast_elements) { int offset = FixedArray::kHeaderSize + (array_index * kPointerSize); __ ld(a6, MemOperand(sp, kPointerSize)); // Copy of array literal. __ ld(a1, FieldMemOperand(a6, JSObject::kElementsOffset)); diff --git a/src/full-codegen/x64/full-codegen-x64.cc b/src/full-codegen/x64/full-codegen-x64.cc index c522d2cf76..ac0d879911 100644 --- a/src/full-codegen/x64/full-codegen-x64.cc +++ b/src/full-codegen/x64/full-codegen-x64.cc @@ -1701,7 +1701,6 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - expr->BuildConstantElements(isolate()); Handle constant_elements = expr->constant_elements(); bool has_constant_fast_elements = IsFastObjectElementsKind(expr->constant_elements_kind()); @@ -1752,7 +1751,14 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } VisitForAccumulatorValue(subexpr); - if (has_constant_fast_elements) { + if (FLAG_vector_stores) { + __ Move(StoreDescriptor::NameRegister(), Smi::FromInt(array_index)); + __ movp(StoreDescriptor::ReceiverRegister(), Operand(rsp, kPointerSize)); + EmitLoadStoreICSlot(expr->LiteralFeedbackSlot()); + Handle ic = + CodeFactory::KeyedStoreIC(isolate(), language_mode()).code(); + CallIC(ic); + } else if (has_constant_fast_elements) { // Fast-case array literal with ElementsKind of FAST_*_ELEMENTS, they // cannot transition and don't need to call the runtime stub. int offset = FixedArray::kHeaderSize + (array_index * kPointerSize); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index fc3aa4cabd..939cd92605 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5989,7 +5989,6 @@ void HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) { DCHECK(!HasStackOverflow()); DCHECK(current_block() != NULL); DCHECK(current_block()->HasPredecessor()); - expr->BuildConstantElements(isolate()); ZoneList* subexprs = expr->values(); int length = subexprs->length(); HInstruction* literal;