[Interpreter] Share feedback slots for load / store named properties

Shares the feedback slot when loading / storing named properties
when the name of the property and the variable corresponding
to the object are the same. This reduces the memory usage on most
real world benchmarks. There is a slight (~1%) increase in the overall
time spent in V8 on a couple of these pages.

There is also no overall performance regression on peak-performance
benchmarks like Octane, ARES. More detailed results are in this doc[1]

[1]: https://docs.google.com/document/d/1rPNjXU-WOlyNQovuQS28Zf2PHCENR97Bi76gV9mHHOc/edit?usp=sharing

BUG: v8:7530
Change-Id: I7dd98c2d26f4e6c94690ca7d9a8a4a8281b3142d
Reviewed-on: https://chromium-review.googlesource.com/966302
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53145}

src/flag-definitions.h

@@ -334,6 +334,9 @@ DEFINE_BOOL(ignition_elide_noneffectful_bytecodes, true,
 DEFINE_BOOL(ignition_reo, true, "use ignition register equivalence optimizer")
 DEFINE_BOOL(ignition_filter_expression_positions, true,
             "filter expression positions before the bytecode pipeline")
+DEFINE_BOOL(ignition_share_named_property_feedback, false,
+            "share feedback slots when loading the same named property from "
+            "the same object")
 DEFINE_BOOL(print_bytecode, false,
             "print bytecode generated by ignition interpreter")
 DEFINE_STRING(print_bytecode_filter, "*",

src/interpreter/bytecode-generator.cc

@@ -807,30 +807,40 @@ class BytecodeGenerator::FeedbackSlotCache : public ZoneObject {
   explicit FeedbackSlotCache(Zone* zone) : map_(zone) {}
 
   void Put(FeedbackSlotKind slot_kind, Variable* variable, FeedbackSlot slot) {
-    PutImpl(slot_kind, variable, slot);
+    PutImpl(slot_kind, 0, variable, slot);
   }
   void Put(FeedbackSlotKind slot_kind, AstNode* node, FeedbackSlot slot) {
-    PutImpl(slot_kind, node, slot);
+    PutImpl(slot_kind, 0, node, slot);
+  }
+  void Put(FeedbackSlotKind slot_kind, int variable_index,
+           const AstRawString* name, FeedbackSlot slot) {
+    PutImpl(slot_kind, variable_index, name, slot);
   }
 
   FeedbackSlot Get(FeedbackSlotKind slot_kind, Variable* variable) const {
-    return GetImpl(slot_kind, variable);
+    return GetImpl(slot_kind, 0, variable);
   }
   FeedbackSlot Get(FeedbackSlotKind slot_kind, AstNode* node) const {
-    return GetImpl(slot_kind, node);
+    return GetImpl(slot_kind, 0, node);
+  }
+  FeedbackSlot Get(FeedbackSlotKind slot_kind, int variable_index,
+                   const AstRawString* name) const {
+    return GetImpl(slot_kind, variable_index, name);
   }
 
  private:
-  typedef std::pair<FeedbackSlotKind, void*> Key;
+  typedef std::tuple<FeedbackSlotKind, int, const void*> Key;
 
-  void PutImpl(FeedbackSlotKind slot_kind, void* node, FeedbackSlot slot) {
-    Key key = std::make_pair(slot_kind, node);
+  void PutImpl(FeedbackSlotKind slot_kind, int index, const void* node,
+               FeedbackSlot slot) {
+    Key key = std::make_tuple(slot_kind, index, node);
     auto entry = std::make_pair(key, slot);
     map_.insert(entry);
   }
 
-  FeedbackSlot GetImpl(FeedbackSlotKind slot_kind, void* node) const {
-    Key key = std::make_pair(slot_kind, node);
+  FeedbackSlot GetImpl(FeedbackSlotKind slot_kind, int index,
+                       const void* node) const {
+    Key key = std::make_tuple(slot_kind, index, node);
     auto iter = map_.find(key);
     if (iter != map_.end()) {
       return iter->second;
@@ -1553,7 +1563,7 @@ void BytecodeGenerator::VisitForInAssignment(Expression* expr) {
       const AstRawString* name =
           property->key()->AsLiteral()->AsRawPropertyName();
       builder()->LoadAccumulatorWithRegister(value);
-      FeedbackSlot slot = feedback_spec()->AddStoreICSlot(language_mode());
+      FeedbackSlot slot = GetCachedStoreICSlot(property->obj(), name);
       builder()->StoreNamedProperty(object, name, feedback_index(slot),
                                     language_mode());
       builder()->LoadAccumulatorWithRegister(value);
@@ -2802,7 +2812,7 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
         break;
       }
       case NAMED_PROPERTY: {
-        FeedbackSlot slot = feedback_spec()->AddLoadICSlot();
+        FeedbackSlot slot = GetCachedLoadICSlot(property->obj(), name);
         builder()->LoadNamedProperty(object, name, feedback_index(slot));
         break;
       }
@@ -2853,7 +2863,7 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
       break;
     }
     case NAMED_PROPERTY: {
-      FeedbackSlot slot = feedback_spec()->AddStoreICSlot(language_mode());
+      FeedbackSlot slot = GetCachedStoreICSlot(property->obj(), name);
       Register value;
       if (!execution_result()->IsEffect()) {
         value = register_allocator()->NewRegister();
@@ -3331,7 +3341,9 @@ void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* property) {
       builder()->SetExpressionPosition(property);
       builder()->LoadNamedProperty(
           obj, property->key()->AsLiteral()->AsRawPropertyName(),
-          feedback_index(feedback_spec()->AddLoadICSlot()));
+          feedback_index(GetCachedLoadICSlot(
+              property->obj(),
+              property->key()->AsLiteral()->AsRawPropertyName())));
       break;
     }
     case KEYED_PROPERTY: {
@@ -3851,7 +3863,8 @@ void BytecodeGenerator::VisitCountOperation(CountOperation* expr) {
       object = VisitForRegisterValue(property->obj());
       name = property->key()->AsLiteral()->AsRawPropertyName();
       builder()->LoadNamedProperty(
-          object, name, feedback_index(feedback_spec()->AddLoadICSlot()));
+          object, name,
+          feedback_index(GetCachedLoadICSlot(property->obj(), name)));
       break;
     }
     case KEYED_PROPERTY: {
@@ -3915,7 +3928,7 @@ void BytecodeGenerator::VisitCountOperation(CountOperation* expr) {
       break;
     }
     case NAMED_PROPERTY: {
-      FeedbackSlot slot = feedback_spec()->AddStoreICSlot(language_mode());
+      FeedbackSlot slot = GetCachedStoreICSlot(property->obj(), name);
       Register value;
       if (!execution_result()->IsEffect()) {
         value = register_allocator()->NewRegister();
@@ -5007,6 +5020,48 @@ FeedbackSlot BytecodeGenerator::GetCachedStoreGlobalICSlot(
   return slot;
 }
 
+FeedbackSlot BytecodeGenerator::GetCachedLoadICSlot(const Expression* expr,
+                                                    const AstRawString* name) {
+  if (!FLAG_ignition_share_named_property_feedback) {
+    return feedback_spec()->AddLoadICSlot();
+  }
+  FeedbackSlotKind slot_kind = FeedbackSlotKind::kLoadProperty;
+  if (!expr->IsVariableProxy()) {
+    return feedback_spec()->AddLoadICSlot();
+  }
+  const VariableProxy* proxy = expr->AsVariableProxy();
+  FeedbackSlot slot =
+      feedback_slot_cache()->Get(slot_kind, proxy->var()->index(), name);
+  if (!slot.IsInvalid()) {
+    return slot;
+  }
+  slot = feedback_spec()->AddLoadICSlot();
+  feedback_slot_cache()->Put(slot_kind, proxy->var()->index(), name, slot);
+  return slot;
+}
+
+FeedbackSlot BytecodeGenerator::GetCachedStoreICSlot(const Expression* expr,
+                                                     const AstRawString* name) {
+  if (!FLAG_ignition_share_named_property_feedback) {
+    return feedback_spec()->AddStoreICSlot(language_mode());
+  }
+  FeedbackSlotKind slot_kind = is_strict(language_mode())
+                                   ? FeedbackSlotKind::kStoreNamedStrict
+                                   : FeedbackSlotKind::kStoreNamedSloppy;
+  if (!expr->IsVariableProxy()) {
+    return feedback_spec()->AddStoreICSlot(language_mode());
+  }
+  const VariableProxy* proxy = expr->AsVariableProxy();
+  FeedbackSlot slot =
+      feedback_slot_cache()->Get(slot_kind, proxy->var()->index(), name);
+  if (!slot.IsInvalid()) {
+    return slot;
+  }
+  slot = feedback_spec()->AddStoreICSlot(language_mode());
+  feedback_slot_cache()->Put(slot_kind, proxy->var()->index(), name, slot);
+  return slot;
+}
+
 FeedbackSlot BytecodeGenerator::GetCachedCreateClosureSlot(
     FunctionLiteral* literal) {
   FeedbackSlotKind slot_kind = FeedbackSlotKind::kCreateClosure;

src/interpreter/bytecode-generator.h

@@ -269,6 +269,10 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   FeedbackSlot GetCachedStoreGlobalICSlot(LanguageMode language_mode,
                                           Variable* variable);
   FeedbackSlot GetCachedCreateClosureSlot(FunctionLiteral* literal);
+  FeedbackSlot GetCachedLoadICSlot(const Expression* expr,
+                                   const AstRawString* name);
+  FeedbackSlot GetCachedStoreICSlot(const Expression* expr,
+                                    const AstRawString* name);
   FeedbackSlot GetDummyCompareICSlot();
 
   void AddToEagerLiteralsIfEager(FunctionLiteral* literal);

test/cctest/interpreter/test-bytecode-generator.cc

@@ -23,6 +23,8 @@ namespace interpreter {
 
 #define UNIQUE_VAR() "var a" STR(__COUNTER__) " = 0;\n"
 
+#define LOAD_UNIQUE_PROPERTY() "  b.name" STR(__COUNTER__) ";\n"
+
 #define REPEAT_2(...) __VA_ARGS__ __VA_ARGS__
 #define REPEAT_4(...) REPEAT_2(__VA_ARGS__) REPEAT_2(__VA_ARGS__)
 #define REPEAT_8(...) REPEAT_4(__VA_ARGS__) REPEAT_4(__VA_ARGS__)
@@ -66,6 +68,21 @@ namespace interpreter {
   UNIQUE_VAR()                   \
   UNIQUE_VAR()
 
+#define REPEAT_2_LOAD_UNIQUE_PROPERTY() \
+  LOAD_UNIQUE_PROPERTY() LOAD_UNIQUE_PROPERTY()
+#define REPEAT_4_LOAD_UNIQUE_PROPERTY() \
+  REPEAT_2_LOAD_UNIQUE_PROPERTY() REPEAT_2_LOAD_UNIQUE_PROPERTY()
+#define REPEAT_8_LOAD_UNIQUE_PROPERTY() \
+  REPEAT_4_LOAD_UNIQUE_PROPERTY() REPEAT_4_LOAD_UNIQUE_PROPERTY()
+#define REPEAT_16_LOAD_UNIQUE_PROPERTY() \
+  REPEAT_8_LOAD_UNIQUE_PROPERTY() REPEAT_8_LOAD_UNIQUE_PROPERTY()
+#define REPEAT_32_LOAD_UNIQUE_PROPERTY() \
+  REPEAT_16_LOAD_UNIQUE_PROPERTY() REPEAT_16_LOAD_UNIQUE_PROPERTY()
+#define REPEAT_64_LOAD_UNIQUE_PROPERTY() \
+  REPEAT_32_LOAD_UNIQUE_PROPERTY() REPEAT_32_LOAD_UNIQUE_PROPERTY()
+#define REPEAT_128_LOAD_UNIQUE_PROPERTY() \
+  REPEAT_64_LOAD_UNIQUE_PROPERTY() REPEAT_64_LOAD_UNIQUE_PROPERTY()
+
 static const char* kGoldenFileDirectory =
     "test/cctest/interpreter/bytecode_expectations/";
 
@@ -375,9 +392,8 @@ TEST(PropertyLoads) {
     "f({\"-124\" : \"test\", name : 123 })",
 
     "function f(a) {\n"
-    "  var b;\n"
-    "  b = a.name;\n"
-    REPEAT_127("  b = a.name;\n")
+    "  var b = {};\n"
+    REPEAT_128_LOAD_UNIQUE_PROPERTY()
     "  return a.name;\n"
     "}\n"
     "f({name : \"test\"})\n",
@@ -425,7 +441,8 @@ TEST(PropertyStores) {
 
     "function f(a) {\n"
     "  a.name = 1;\n"
-    REPEAT_127("  a.name = 1;\n")
+    "  var b = {};\n"
+    REPEAT_128_LOAD_UNIQUE_PROPERTY()
     "  a.name = 2;\n"
     "}\n"
     "f({name : \"test\"})\n",
@@ -433,7 +450,8 @@ TEST(PropertyStores) {
     "function f(a) {\n"
     " 'use strict';\n"
     "  a.name = 1;\n"
-    REPEAT_127("  a.name = 1;\n")
+    "  var b = {};\n"
+    REPEAT_128_LOAD_UNIQUE_PROPERTY()
     "  a.name = 2;\n"
     "}\n"
     "f({name : \"test\"})\n",
@@ -477,9 +495,10 @@ TEST(PropertyCall) {
       "f(" FUNC_ARG ", 1)",
 
       "function f(a) {\n"
-      " a.func;\n"              //
-      REPEAT_127(" a.func;\n")  //
-      " return a.func(); }\n"
+      "  var b = {};\n"
+      REPEAT_128_LOAD_UNIQUE_PROPERTY()
+      "  a.func;\n"              //
+      "  return a.func(); }\n"
       "f(" FUNC_ARG ")",
 
       "function f(a) { return a.func(1).func(2).func(3); }\n"
@@ -510,9 +529,9 @@ TEST(LoadGlobal) {
     "f()",
 
     "a = 1;\n"
-    "function f(b) {\n"
-    "  b.name;\n"
-    REPEAT_127("  b.name;\n")
+    "function f(c) {\n"
+    "  var b = {};\n"
+    REPEAT_128_LOAD_UNIQUE_PROPERTY()
     "  return a;\n"
     "}\n"
     "f({name: 1});\n",
@@ -545,18 +564,18 @@ TEST(StoreGlobal) {
     "f();\n",
 
     "a = 1;\n"
-    "function f(b) {\n"
-    "  b.name;\n"
-    REPEAT_127("  b.name;\n")
+    "function f(c) {\n"
+    "  var b = {};\n"
+    REPEAT_128_LOAD_UNIQUE_PROPERTY()
     "  a = 2;\n"
     "}\n"
     "f({name: 1});\n",
 
     "a = 1;\n"
-    "function f(b) {\n"
+    "function f(c) {\n"
     "  'use strict';\n"
-    "  b.name;\n"
-    REPEAT_127("  b.name;\n")
+    "  var b = {};\n"
+    REPEAT_128_LOAD_UNIQUE_PROPERTY()
     "  a = 2;\n"
     "}\n"
     "f({name: 1});\n",
@@ -2803,6 +2822,14 @@ TEST(TemplateLiterals) {
 #undef REPEAT_64_UNIQUE_VARS
 #undef REPEAT_128_UNIQUE_VARS
 #undef REPEAT_250_UNIQUE_VARS
+#undef LOAD_UNIQUE_PROPERTY
+#undef REPEAT_2_LOAD_UNIQUE_PROPERTY
+#undef REPEAT_4_LOAD_UNIQUE_PROPERTY
+#undef REPEAT_8_LOAD_UNIQUE_PROPERTY
+#undef REPEAT_16_LOAD_UNIQUE_PROPERTY
+#undef REPEAT_32_LOAD_UNIQUE_PROPERTY
+#undef REPEAT_64_LOAD_UNIQUE_PROPERTY
+#undef REPEAT_128_LOAD_UNIQUE_PROPERTY
 #undef FUNC_ARG
 
 }  // namespace interpreter