From 1503d0e78c6b146c64f2bc028c534d5dd57d6944 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 14 Nov 2014 09:21:13 +0100 Subject: [PATCH] Move feedback slot allocation to post-pass R=mvstanton@chromium.org, svenpanne@chromium.org Review URL: https://codereview.chromium.org/670953003 Cr-Commit-Position: refs/heads/master@{#25348} --- src/ast-numbering.cc | 26 ++++++++++++++++++---- src/ast.cc | 5 ----- src/ast.h | 33 +++++++++++----------------- src/compiler.cc | 14 +++++++++++- src/compiler.h | 1 + src/full-codegen.cc | 3 +++ src/liveedit.cc | 42 ++++++++++++++++-------------------- src/liveedit.h | 13 ++--------- src/parser.cc | 10 --------- src/preparser.h | 13 ----------- test/cctest/test-compiler.cc | 19 ++++++++-------- 11 files changed, 81 insertions(+), 98 deletions(-) diff --git a/src/ast-numbering.cc b/src/ast-numbering.cc index 3894492a02..5ebbde8a5c 100644 --- a/src/ast-numbering.cc +++ b/src/ast-numbering.cc @@ -64,6 +64,21 @@ class AstNumberingVisitor FINAL : public AstVisitor { properties_.flags()->Add(kDontCache); } + template + void ReserveFeedbackSlots(Node* node) { + FeedbackVectorRequirements reqs = node->ComputeFeedbackRequirements(); + if (reqs.slots() > 0) { + node->SetFirstFeedbackSlot( + FeedbackVectorSlot(properties_.feedback_slots())); + properties_.increase_feedback_slots(reqs.slots()); + } + if (reqs.ic_slots() > 0) { + node->SetFirstFeedbackICSlot( + FeedbackVectorICSlot(properties_.ic_feedback_slots())); + properties_.increase_ic_feedback_slots(reqs.ic_slots()); + } + } + BailoutReason dont_optimize_reason() const { return (dont_turbofan_reason_ != kNoReason) ? dont_turbofan_reason_ : dont_crankshaft_reason_; @@ -145,6 +160,7 @@ void AstNumberingVisitor::VisitVariableProxy(VariableProxy* node) { if (node->var()->IsLookupSlot()) { DisableCrankshaft(kReferenceToAVariableWhichRequiresDynamicLookup); } + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(VariableProxy::num_ids())); } @@ -158,6 +174,7 @@ void AstNumberingVisitor::VisitThisFunction(ThisFunction* node) { void AstNumberingVisitor::VisitSuperReference(SuperReference* node) { IncrementNodeCount(); DisableTurbofan(kSuperReference); + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(SuperReference::num_ids())); Visit(node->this_var()); } @@ -215,6 +232,7 @@ void AstNumberingVisitor::VisitReturnStatement(ReturnStatement* node) { void AstNumberingVisitor::VisitYield(Yield* node) { IncrementNodeCount(); DisableCrankshaft(kYield); + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(Yield::num_ids())); Visit(node->generator_object()); Visit(node->expression()); @@ -319,6 +337,7 @@ void AstNumberingVisitor::VisitTryFinallyStatement(TryFinallyStatement* node) { void AstNumberingVisitor::VisitProperty(Property* node) { IncrementNodeCount(); + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(Property::num_ids())); Visit(node->key()); Visit(node->obj()); @@ -353,6 +372,7 @@ void AstNumberingVisitor::VisitCompareOperation(CompareOperation* node) { void AstNumberingVisitor::VisitForInStatement(ForInStatement* node) { IncrementNodeCount(); DisableSelfOptimization(); + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(ForInStatement::num_ids())); Visit(node->each()); Visit(node->enumerable()); @@ -461,6 +481,7 @@ void AstNumberingVisitor::VisitArrayLiteral(ArrayLiteral* node) { void AstNumberingVisitor::VisitCall(Call* node) { IncrementNodeCount(); + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(Call::num_ids())); Visit(node->expression()); VisitArguments(node->arguments()); @@ -469,6 +490,7 @@ void AstNumberingVisitor::VisitCall(Call* node) { void AstNumberingVisitor::VisitCallNew(CallNew* node) { IncrementNodeCount(); + ReserveFeedbackSlots(node); node->set_base_id(ReserveIdRange(CallNew::num_ids())); Visit(node->expression()); VisitArguments(node->arguments()); @@ -507,10 +529,6 @@ void AstNumberingVisitor::VisitFunctionLiteral(FunctionLiteral* node) { void AstNumberingVisitor::Renumber(FunctionLiteral* node) { - properties_.flags()->Add(*node->flags()); - properties_.increase_feedback_slots(node->slot_count()); - properties_.increase_ic_feedback_slots(node->ic_slot_count()); - if (node->scope()->HasIllegalRedeclaration()) { node->scope()->VisitIllegalRedeclaration(this); return; diff --git a/src/ast.cc b/src/ast.cc index 220dc04426..92a9ce6363 100644 --- a/src/ast.cc +++ b/src/ast.cc @@ -1006,28 +1006,24 @@ CaseClause::CaseClause(Zone* zone, Expression* label, } #define REGULAR_NODE_WITH_FEEDBACK_SLOTS(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ - add_slot_node(node); \ } #define DONT_OPTIMIZE_NODE(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ } #define DONT_OPTIMIZE_NODE_WITH_FEEDBACK_SLOTS(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ - add_slot_node(node); \ } #define DONT_TURBOFAN_NODE(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ } #define DONT_TURBOFAN_NODE_WITH_FEEDBACK_SLOTS(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ - add_slot_node(node); \ } #define DONT_SELFOPTIMIZE_NODE(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ } #define DONT_SELFOPTIMIZE_NODE_WITH_FEEDBACK_SLOTS(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ - add_slot_node(node); \ } #define DONT_CACHE_NODE(NodeType) \ void AstConstructionVisitor::Visit##NodeType(NodeType* node) { \ @@ -1101,7 +1097,6 @@ DONT_CACHE_NODE(ModuleLiteral) void AstConstructionVisitor::VisitCallRuntime(CallRuntime* node) { - add_slot_node(node); } #undef REGULAR_NODE diff --git a/src/ast.h b/src/ast.h index deead81af7..5e1eaa1123 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1705,6 +1705,7 @@ class VariableProxy FINAL : public Expression { } FeedbackVectorICSlot VariableFeedbackSlot() { + DCHECK(!FLAG_vector_ics || !variable_feedback_slot_.IsInvalid()); return variable_feedback_slot_; } @@ -1790,6 +1791,7 @@ class Property FINAL : public Expression { } FeedbackVectorICSlot PropertyFeedbackSlot() const { + DCHECK(!FLAG_vector_ics || !property_feedback_slot_.IsInvalid()); return property_feedback_slot_; } @@ -1834,7 +1836,10 @@ class Call FINAL : public Expression { } bool HasCallFeedbackSlot() const { return !call_feedback_slot_.IsInvalid(); } - FeedbackVectorICSlot CallFeedbackSlot() const { return call_feedback_slot_; } + FeedbackVectorICSlot CallFeedbackSlot() const { + DCHECK(!call_feedback_slot_.IsInvalid()); + return call_feedback_slot_; + } virtual SmallMapList* GetReceiverTypes() OVERRIDE { if (expression()->IsProperty()) { @@ -1933,7 +1938,10 @@ class CallNew FINAL : public Expression { callnew_feedback_slot_ = slot; } - FeedbackVectorSlot CallNewFeedbackSlot() { return callnew_feedback_slot_; } + FeedbackVectorSlot CallNewFeedbackSlot() { + DCHECK(!callnew_feedback_slot_.IsInvalid()); + return callnew_feedback_slot_; + } FeedbackVectorSlot AllocationSiteFeedbackSlot() { DCHECK(FLAG_pretenuring_call_new); return CallNewFeedbackSlot().next(); @@ -1997,6 +2005,8 @@ class CallRuntime FINAL : public Expression { } FeedbackVectorICSlot CallRuntimeFeedbackSlot() { + DCHECK(!(FLAG_vector_ics && is_jsruntime()) || + !callruntime_feedback_slot_.IsInvalid()); return callruntime_feedback_slot_; } @@ -2383,6 +2393,7 @@ class Yield FINAL : public Expression { } FeedbackVectorICSlot KeyedLoadFeedbackSlot() { + DCHECK(!FLAG_vector_ics || !yield_first_feedback_slot_.IsInvalid()); return yield_first_feedback_slot_; } @@ -3159,8 +3170,6 @@ class AstConstructionVisitor BASE_EMBEDDED { public: AstConstructionVisitor() {} - AstProperties* ast_properties() { return &properties_; } - private: template friend class AstNodeFactory; @@ -3169,22 +3178,6 @@ class AstConstructionVisitor BASE_EMBEDDED { void Visit##type(type* node); AST_NODE_LIST(DEF_VISIT) #undef DEF_VISIT - - void add_slot_node(AstNode* slot_node) { - FeedbackVectorRequirements reqs = slot_node->ComputeFeedbackRequirements(); - if (reqs.slots() > 0) { - slot_node->SetFirstFeedbackSlot( - FeedbackVectorSlot(properties_.feedback_slots())); - properties_.increase_feedback_slots(reqs.slots()); - } - if (reqs.ic_slots() > 0) { - slot_node->SetFirstFeedbackICSlot( - FeedbackVectorICSlot(properties_.ic_feedback_slots())); - properties_.increase_ic_feedback_slots(reqs.ic_slots()); - } - } - - AstProperties properties_; }; diff --git a/src/compiler.cc b/src/compiler.cc index 4472afa347..c4eed84a4d 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -291,9 +291,11 @@ bool CompilationInfo::ShouldSelfOptimize() { void CompilationInfo::PrepareForCompilation(Scope* scope) { DCHECK(scope_ == NULL); scope_ = scope; +} + +void CompilationInfo::EnsureFeedbackVector() { if (feedback_vector_.is_null()) { - // Allocate the feedback vector too. feedback_vector_ = isolate()->factory()->NewTypeFeedbackVector( function()->slot_count(), function()->ic_slot_count()); } @@ -1326,8 +1328,18 @@ Handle Compiler::BuildFunctionInfo( if (FLAG_lazy && allow_lazy && !literal->is_parenthesized()) { Handle code = isolate->builtins()->CompileLazy(); info.SetCode(code); + // There's no need in theory for a lazy-compiled function to have a type + // feedback vector, but some parts of the system expect all + // SharedFunctionInfo instances to have one. The size of the vector depends + // on how many feedback-needing nodes are in the tree, and when lazily + // parsing we might not know that, if this function was never parsed before. + // In that case the vector will be replaced the next time MakeCode is + // called. + info.EnsureFeedbackVector(); scope_info = Handle(ScopeInfo::Empty(isolate)); } else if (Renumber(&info) && FullCodeGenerator::MakeCode(&info)) { + // MakeCode will ensure that the feedback vector is present and + // appropriately sized. DCHECK(!info.code().is_null()); scope_info = ScopeInfo::Create(info.scope(), info.zone()); } else { diff --git a/src/compiler.h b/src/compiler.h index 822151d64a..07bc92a8e9 100644 --- a/src/compiler.h +++ b/src/compiler.h @@ -235,6 +235,7 @@ class CompilationInfo { DCHECK(script_scope_ == NULL); script_scope_ = script_scope; } + void EnsureFeedbackVector(); Handle feedback_vector() const { return feedback_vector_; } diff --git a/src/full-codegen.cc b/src/full-codegen.cc index e399a3496c..a993d4856b 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -306,6 +306,9 @@ bool FullCodeGenerator::MakeCode(CompilationInfo* info) { TimerEventScope timer(info->isolate()); + // Ensure that the feedback vector is large enough. + info->EnsureFeedbackVector(); + Handle