Stop overallocating feedback vector slots.

When a Property or a VariableProxy is used as the left hand side of an
assignment statement, there is no need to allocate a LOAD_IC feedback
vector slot for it. Alter the numbering phase to support this.

BUG=

Review URL: https://codereview.chromium.org/1262803002

Cr-Commit-Position: refs/heads/master@{#29924}

diff --git a/src/ast-numbering.cc b/src/ast-numbering.cc
index e14b84dfe8fc26dbc27ca768e59099a7f9f432a8..8676eeb995e064ffd5bb77c7097903459286de83 100644 (file)
--- a/src/ast-numbering.cc
+++ b/src/ast-numbering.cc
@@ -11,7 +11,7 @@ namespace internal {
 
 class AstNumberingVisitor final : public AstVisitor {
  public:
-  explicit AstNumberingVisitor(Isolate* isolate, Zone* zone)
+  AstNumberingVisitor(Isolate* isolate, Zone* zone)
       : AstVisitor(),
         next_id_(BailoutId::FirstUsable().ToInt()),
         properties_(zone),
@@ -30,6 +30,10 @@ class AstNumberingVisitor final : public AstVisitor {
 
   bool Finish(FunctionLiteral* node);
 
+  void VisitVariableProxyReference(VariableProxy* node);
+  void VisitPropertyReference(Property* node);
+  void VisitReference(Expression* expr);
+
   void VisitStatements(ZoneList<Statement*>* statements) override;
   void VisitDeclarations(ZoneList<Declaration*>* declarations) override;
   void VisitArguments(ZoneList<Expression*>* arguments);
@@ -145,16 +149,21 @@ void AstNumberingVisitor::VisitRegExpLiteral(RegExpLiteral* node) {
 }
 
 
-void AstNumberingVisitor::VisitVariableProxy(VariableProxy* node) {
+void AstNumberingVisitor::VisitVariableProxyReference(VariableProxy* node) {
   IncrementNodeCount();
   if (node->var()->IsLookupSlot()) {
     DisableCrankshaft(kReferenceToAVariableWhichRequiresDynamicLookup);
   }
-  ReserveFeedbackSlots(node);
   node->set_base_id(ReserveIdRange(VariableProxy::num_ids()));
 }
 
 
+void AstNumberingVisitor::VisitVariableProxy(VariableProxy* node) {
+  VisitVariableProxyReference(node);
+  ReserveFeedbackSlots(node);
+}
+
+
 void AstNumberingVisitor::VisitThisFunction(ThisFunction* node) {
   IncrementNodeCount();
   node->set_base_id(ReserveIdRange(ThisFunction::num_ids()));
@@ -304,20 +313,35 @@ void AstNumberingVisitor::VisitTryFinallyStatement(TryFinallyStatement* node) {
 }
 
 
-void AstNumberingVisitor::VisitProperty(Property* node) {
+void AstNumberingVisitor::VisitPropertyReference(Property* node) {
   IncrementNodeCount();
-  ReserveFeedbackSlots(node);
   node->set_base_id(ReserveIdRange(Property::num_ids()));
   Visit(node->key());
   Visit(node->obj());
 }
 
 
+void AstNumberingVisitor::VisitReference(Expression* expr) {
+  DCHECK(expr->IsProperty() || expr->IsVariableProxy());
+  if (expr->IsProperty()) {
+    VisitPropertyReference(expr->AsProperty());
+  } else {
+    VisitVariableProxyReference(expr->AsVariableProxy());
+  }
+}
+
+
+void AstNumberingVisitor::VisitProperty(Property* node) {
+  VisitPropertyReference(node);
+  ReserveFeedbackSlots(node);
+}
+
+
 void AstNumberingVisitor::VisitAssignment(Assignment* node) {
   IncrementNodeCount();
   node->set_base_id(ReserveIdRange(Assignment::num_ids()));
   if (node->is_compound()) VisitBinaryOperation(node->binary_operation());
-  Visit(node->target());
+  VisitReference(node->target());
   Visit(node->value());
   ReserveFeedbackSlots(node);
 }

diff --git a/src/ast.h b/src/ast.h
index ee019e8f0fa60ee540fd9361029208bea07c27a8..0e419c564dd4a2047925c49774b9b100e1901a2a 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -1684,7 +1684,6 @@ class VariableProxy final : public Expression {
                               ICSlotCache* cache) override;
   Code::Kind FeedbackICSlotKind(int index) override { return Code::LOAD_IC; }
   FeedbackVectorICSlot VariableFeedbackSlot() {
-    DCHECK(!UsesVariableFeedbackSlot() || !variable_feedback_slot_.IsInvalid());
     return variable_feedback_slot_;
   }
 
@@ -1792,7 +1791,6 @@ class Property final : public Expression {
   }
 
   FeedbackVectorICSlot PropertyFeedbackSlot() const {
-    DCHECK(!property_feedback_slot_.IsInvalid());
     return property_feedback_slot_;
   }
 

diff --git a/src/type-info.cc b/src/type-info.cc
index a813311bf149c2888d0533c7a70d1851d63c0726..1b7bc8e8b614407d9acf0bd426f3ef830e5eccec 100644 (file)
--- a/src/type-info.cc
+++ b/src/type-info.cc
@@ -106,13 +106,15 @@ InlineCacheState TypeFeedbackOracle::LoadInlineCacheState(TypeFeedbackId id) {
 
 InlineCacheState TypeFeedbackOracle::LoadInlineCacheState(
     FeedbackVectorICSlot slot) {
-  Code::Kind kind = feedback_vector_->GetKind(slot);
-  if (kind == Code::LOAD_IC) {
-    LoadICNexus nexus(feedback_vector_, slot);
-    return nexus.StateFromFeedback();
-  } else if (kind == Code::KEYED_LOAD_IC) {
-    KeyedLoadICNexus nexus(feedback_vector_, slot);
-    return nexus.StateFromFeedback();
+  if (!slot.IsInvalid()) {
+    Code::Kind kind = feedback_vector_->GetKind(slot);
+    if (kind == Code::LOAD_IC) {
+      LoadICNexus nexus(feedback_vector_, slot);
+      return nexus.StateFromFeedback();
+    } else if (kind == Code::KEYED_LOAD_IC) {
+      KeyedLoadICNexus nexus(feedback_vector_, slot);
+      return nexus.StateFromFeedback();
+    }
   }
 
   // If we can't find an IC, assume we've seen *something*, but we don't know
@@ -331,9 +333,11 @@ void TypeFeedbackOracle::PropertyReceiverTypes(FeedbackVectorICSlot slot,
                                                Handle<Name> name,
                                                SmallMapList* receiver_types) {
   receiver_types->Clear();
-  LoadICNexus nexus(feedback_vector_, slot);
-  Code::Flags flags = Code::ComputeHandlerFlags(Code::LOAD_IC);
-  CollectReceiverTypes(&nexus, name, flags, receiver_types);
+  if (!slot.IsInvalid()) {
+    LoadICNexus nexus(feedback_vector_, slot);
+    Code::Flags flags = Code::ComputeHandlerFlags(Code::LOAD_IC);
+    CollectReceiverTypes(&nexus, name, flags, receiver_types);
+  }
 }
 
 
@@ -341,10 +345,15 @@ void TypeFeedbackOracle::KeyedPropertyReceiverTypes(
     FeedbackVectorICSlot slot, SmallMapList* receiver_types, bool* is_string,
     IcCheckType* key_type) {
   receiver_types->Clear();
-  KeyedLoadICNexus nexus(feedback_vector_, slot);
-  CollectReceiverTypes<FeedbackNexus>(&nexus, receiver_types);
-  *is_string = HasOnlyStringMaps(receiver_types);
-  *key_type = nexus.FindFirstName() != NULL ? PROPERTY : ELEMENT;
+  if (slot.IsInvalid()) {
+    *is_string = false;
+    *key_type = ELEMENT;
+  } else {
+    KeyedLoadICNexus nexus(feedback_vector_, slot);
+    CollectReceiverTypes<FeedbackNexus>(&nexus, receiver_types);
+    *is_string = HasOnlyStringMaps(receiver_types);
+    *key_type = nexus.FindFirstName() != NULL ? PROPERTY : ELEMENT;
+  }
 }
 
 

diff --git a/test/cctest/test-feedback-vector.cc b/test/cctest/test-feedback-vector.cc
index cf8a730fb71c8c5fcc4d2e8763a737098e6b29e6..2995fb9730eeec0172f2772344dc688abc66852e 100644 (file)
--- a/test/cctest/test-feedback-vector.cc
+++ b/test/cctest/test-feedback-vector.cc
@@ -385,4 +385,101 @@ TEST(VectorLoadICOnSmi) {
   nexus.FindAllMaps(&maps2);
   CHECK_EQ(2, maps2.length());
 }
+
+
+static Handle<JSFunction> GetFunction(const char* name) {
+  Handle<JSFunction> f = v8::Utils::OpenHandle(
+      *v8::Handle<v8::Function>::Cast(CcTest::global()->Get(v8_str(name))));
+  return f;
+}
+
+
+TEST(ReferenceContextAllocatesNoSlots) {
+  if (i::FLAG_always_opt) return;
+  CcTest::InitializeVM();
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+  Isolate* isolate = CcTest::i_isolate();
+
+  CompileRun(
+      "function testvar(x) {"
+      "  y = x;"
+      "  y = a;"
+      "  return y;"
+      "}"
+      "a = 3;"
+      "testvar({});");
+
+  Handle<JSFunction> f = GetFunction("testvar");
+
+  // There should be two LOAD_ICs, one for a and one for y at the end.
+  Handle<TypeFeedbackVector> feedback_vector =
+      handle(f->shared()->feedback_vector(), isolate);
+  CHECK_EQ(2, feedback_vector->ICSlots());
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(0)) == Code::LOAD_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(1)) == Code::LOAD_IC);
+
+  CompileRun(
+      "function testprop(x) {"
+      "  x.blue = a;"
+      "}"
+      "testprop({ blue: 3 });");
+
+  f = GetFunction("testprop");
+
+  // There should be one LOAD_IC, for the load of a.
+  feedback_vector = handle(f->shared()->feedback_vector(), isolate);
+  CHECK_EQ(1, feedback_vector->ICSlots());
+
+  CompileRun(
+      "function testpropfunc(x) {"
+      "  x().blue = a;"
+      "  return x().blue;"
+      "}"
+      "function makeresult() { return { blue: 3 }; }"
+      "testpropfunc(makeresult);");
+
+  f = GetFunction("testpropfunc");
+
+  // There should be 2 LOAD_ICs and 2 CALL_ICs.
+  feedback_vector = handle(f->shared()->feedback_vector(), isolate);
+  CHECK_EQ(4, feedback_vector->ICSlots());
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(0)) == Code::CALL_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(1)) == Code::LOAD_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(2)) == Code::CALL_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(3)) == Code::LOAD_IC);
+
+  CompileRun(
+      "function testkeyedprop(x) {"
+      "  x[0] = a;"
+      "  return x[0];"
+      "}"
+      "testkeyedprop([0, 1, 2]);");
+
+  f = GetFunction("testkeyedprop");
+
+  // There should be 1 LOAD_ICs for the load of a, and one KEYED_LOAD_IC for the
+  // load of x[0] in the return statement.
+  feedback_vector = handle(f->shared()->feedback_vector(), isolate);
+  CHECK_EQ(2, feedback_vector->ICSlots());
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(0)) == Code::LOAD_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(1)) ==
+        Code::KEYED_LOAD_IC);
+
+  CompileRun(
+      "function testcompound(x) {"
+      "  x.old = x.young = x.in_between = a;"
+      "  return x.old + x.young;"
+      "}"
+      "testcompound({ old: 3, young: 3, in_between: 3 });");
+
+  f = GetFunction("testcompound");
+
+  // There should be 3 LOAD_ICs, for load of a and load of x.old and x.young.
+  feedback_vector = handle(f->shared()->feedback_vector(), isolate);
+  CHECK_EQ(3, feedback_vector->ICSlots());
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(0)) == Code::LOAD_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(1)) == Code::LOAD_IC);
+  CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(2)) == Code::LOAD_IC);
+}
 }