[turbofan] Allow registers/accum/params to share state value nodes

Previously, accumulators and registers each had a single element cache,
distinct from the local register cache. This meant that

 a) Dead accumulator state nodes were not re-used if the accumulator
    became live.
 b) Functions with only one parameter (the this object) or only one
    local register could not reuse the single-valued state value node
    of the accumulator.

This patch introduces heavier re-use of state-value nodes, decreasing
memory use when building the graph and decreasing the number of nodes
created overall.

Change-Id: Ie3cc6913483aab0819d99be382eb2cb42de8c3d2
Reviewed-on: https://chromium-review.googlesource.com/440926Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43148}

src/compiler/bytecode-graph-builder.cc

@@ -83,7 +83,8 @@ class BytecodeGraphBuilder::Environment : public ZoneObject {
   bool StateValuesRequireUpdate(Node** state_values, Node** values, int count);
   void UpdateStateValues(Node** state_values, Node** values, int count);
   void UpdateStateValuesWithCache(Node** state_values, Node** values, int count,
-                                  const BitVector* liveness);
+                                  const BitVector* liveness,
+                                  int liveness_offset);
 
   int RegisterToValuesIndex(interpreter::Register the_register) const;
 
@@ -350,7 +351,7 @@ bool BytecodeGraphBuilder::Environment::StateValuesRequireUpdate(
     return true;
   }
   Node::Inputs inputs = (*state_values)->inputs();
-  DCHECK_EQ(inputs.count(), count);
+  if (inputs.count() != count) return true;
   for (int i = 0; i < count; i++) {
     if (inputs[i] != values[i]) {
       return true;
@@ -411,28 +412,43 @@ void BytecodeGraphBuilder::Environment::UpdateStateValues(Node** state_values,
 }
 
 void BytecodeGraphBuilder::Environment::UpdateStateValuesWithCache(
-    Node** state_values, Node** values, int count, const BitVector* liveness) {
+    Node** state_values, Node** values, int count, const BitVector* liveness,
+    int liveness_offset) {
   *state_values = builder_->state_values_cache_.GetNodeForValues(
-      values, static_cast<size_t>(count), liveness);
+      values, static_cast<size_t>(count), liveness, liveness_offset);
 }
 
 Node* BytecodeGraphBuilder::Environment::Checkpoint(
     BailoutId bailout_id, OutputFrameStateCombine combine,
     bool owner_has_exception, const BytecodeLivenessState* liveness) {
-  UpdateStateValues(&parameters_state_values_, &values()->at(0),
-                    parameter_count());
+  if (parameter_count() == register_count()) {
+    // Re-use the state-value cache if the number of local registers happens
+    // to match the parameter count.
+    UpdateStateValuesWithCache(&parameters_state_values_, &values()->at(0),
+                               parameter_count(), nullptr, 0);
+  } else {
+    UpdateStateValues(&parameters_state_values_, &values()->at(0),
+                      parameter_count());
+  }
 
-  // TODO(leszeks): We should pass a view of the liveness bitvector here, with
-  // offset and count, rather than passing the entire bitvector and assuming
-  // that register liveness starts at offset 0.
   UpdateStateValuesWithCache(&registers_state_values_,
                              &values()->at(register_base()), register_count(),
-                             liveness ? &liveness->bit_vector() : nullptr);
-
-  Node* accumulator_value = liveness == nullptr || liveness->AccumulatorIsLive()
-                                ? values()->at(accumulator_base())
-                                : builder()->jsgraph()->OptimizedOutConstant();
-  UpdateStateValues(&accumulator_state_values_, &accumulator_value, 1);
+                             liveness ? &liveness->bit_vector() : nullptr, 0);
+
+  bool accumulator_is_live = !liveness || liveness->AccumulatorIsLive();
+  if (parameter_count() == 1 && accumulator_is_live &&
+      values()->at(accumulator_base()) == values()->at(0)) {
+    // Re-use the parameter state values if there happens to only be one
+    // parameter and the accumulator is live and holds that parameter's value.
+    accumulator_state_values_ = parameters_state_values_;
+  } else {
+    // Otherwise, use the state values cache to hopefully re-use local register
+    // state values (if there is only one local register), or at the very least
+    // re-use previous accumulator state values.
+    UpdateStateValuesWithCache(
+        &accumulator_state_values_, &values()->at(accumulator_base()), 1,
+        liveness ? &liveness->bit_vector() : nullptr, register_count());
+  }
 
   const Operator* op = common()->FrameState(
       bailout_id, combine, builder()->frame_state_function_info());

src/compiler/state-values-utils.cc

@@ -137,7 +137,8 @@ Node* StateValuesCache::GetValuesNodeFromCache(Node** nodes, size_t count,
 
 SparseInputMask::BitMaskType StateValuesCache::FillBufferWithValues(
     WorkingBuffer* node_buffer, size_t* node_count, size_t* values_idx,
-    Node** values, size_t count, const BitVector* liveness) {
+    Node** values, size_t count, const BitVector* liveness,
+    int liveness_offset) {
   SparseInputMask::BitMaskType input_mask = 0;
 
   // Virtual nodes are the live nodes plus the implicit optimized out nodes,
@@ -149,7 +150,7 @@ SparseInputMask::BitMaskType StateValuesCache::FillBufferWithValues(
     DCHECK_LE(*values_idx, static_cast<size_t>(INT_MAX));
 
     if (liveness == nullptr ||
-        liveness->Contains(static_cast<int>(*values_idx))) {
+        liveness->Contains(liveness_offset + static_cast<int>(*values_idx))) {
       input_mask |= 1 << (virtual_node_count);
       (*node_buffer)[(*node_count)++] = values[*values_idx];
     }
@@ -169,14 +170,14 @@ SparseInputMask::BitMaskType StateValuesCache::FillBufferWithValues(
 
 Node* StateValuesCache::BuildTree(size_t* values_idx, Node** values,
                                   size_t count, const BitVector* liveness,
-                                  size_t level) {
+                                  int liveness_offset, size_t level) {
   WorkingBuffer* node_buffer = GetWorkingSpace(level);
   size_t node_count = 0;
   SparseInputMask::BitMaskType input_mask = SparseInputMask::kDenseBitMask;
 
   if (level == 0) {
     input_mask = FillBufferWithValues(node_buffer, &node_count, values_idx,
-                                      values, count, liveness);
+                                      values, count, liveness, liveness_offset);
     // Make sure we returned a sparse input mask.
     DCHECK_NE(input_mask, SparseInputMask::kDenseBitMask);
   } else {
@@ -188,8 +189,9 @@ Node* StateValuesCache::BuildTree(size_t* values_idx, Node** values,
         // remaining live nodes.
 
         size_t previous_input_count = node_count;
-        input_mask = FillBufferWithValues(node_buffer, &node_count, values_idx,
-                                          values, count, liveness);
+        input_mask =
+            FillBufferWithValues(node_buffer, &node_count, values_idx, values,
+                                 count, liveness, liveness_offset);
         // Make sure we have exhausted our values.
         DCHECK_EQ(*values_idx, count);
         // Make sure we returned a sparse input mask.
@@ -205,8 +207,8 @@ Node* StateValuesCache::BuildTree(size_t* values_idx, Node** values,
 
       } else {
         // Otherwise, add the values to a subtree and add that as an input.
-        Node* subtree =
-            BuildTree(values_idx, values, count, liveness, level - 1);
+        Node* subtree = BuildTree(values_idx, values, count, liveness,
+                                  liveness_offset, level - 1);
         (*node_buffer)[node_count++] = subtree;
         // Don't touch the bitmask, so that it stays dense.
       }
@@ -229,7 +231,7 @@ Node* StateValuesCache::BuildTree(size_t* values_idx, Node** values,
 namespace {
 
 void CheckTreeContainsValues(Node* tree, Node** values, size_t count,
-                             const BitVector* liveness) {
+                             const BitVector* liveness, int liveness_offset) {
   CHECK_EQ(count, StateValuesAccess(tree).size());
 
   int i;
@@ -237,7 +239,7 @@ void CheckTreeContainsValues(Node* tree, Node** values, size_t count,
   auto it = access.begin();
   auto itend = access.end();
   for (i = 0; it != itend; ++it, ++i) {
-    if (liveness == nullptr || liveness->Contains(i)) {
+    if (liveness == nullptr || liveness->Contains(liveness_offset + i)) {
       CHECK((*it).node == values[i]);
     } else {
       CHECK((*it).node == nullptr);
@@ -250,7 +252,8 @@ void CheckTreeContainsValues(Node* tree, Node** values, size_t count,
 #endif
 
 Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
-                                         const BitVector* liveness) {
+                                         const BitVector* liveness,
+                                         int liveness_offset) {
 #if DEBUG
   // Check that the values represent actual values, and not a tree of values.
   for (size_t i = 0; i < count; i++) {
@@ -260,12 +263,10 @@ Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
     }
   }
   if (liveness != nullptr) {
-    // Liveness can have extra bits for the stack or accumulator, which we
-    // ignore here.
-    DCHECK_LE(count, static_cast<size_t>(liveness->length()));
+    DCHECK_LE(liveness_offset + count, static_cast<size_t>(liveness->length()));
 
     for (size_t i = 0; i < count; i++) {
-      if (liveness->Contains(static_cast<int>(i))) {
+      if (liveness->Contains(liveness_offset + static_cast<int>(i))) {
         DCHECK_NOT_NULL(values[i]);
       }
     }
@@ -288,7 +289,8 @@ Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
   }
 
   size_t values_idx = 0;
-  Node* tree = BuildTree(&values_idx, values, count, liveness, height);
+  Node* tree =
+      BuildTree(&values_idx, values, count, liveness, liveness_offset, height);
   // The values should be exhausted by the end of BuildTree.
   DCHECK_EQ(values_idx, count);
 
@@ -296,7 +298,7 @@ Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
   DCHECK_EQ(tree->opcode(), IrOpcode::kStateValues);
 
 #if DEBUG
-  CheckTreeContainsValues(tree, values, count, liveness);
+  CheckTreeContainsValues(tree, values, count, liveness, liveness_offset);
 #endif
 
   return tree;

src/compiler/state-values-utils.h

@@ -24,7 +24,8 @@ class V8_EXPORT_PRIVATE StateValuesCache {
   explicit StateValuesCache(JSGraph* js_graph);
 
   Node* GetNodeForValues(Node** values, size_t count,
-                         const BitVector* liveness = nullptr);
+                         const BitVector* liveness = nullptr,
+                         int liveness_offset = 0);
 
  private:
   static const size_t kMaxInputCount = 8;
@@ -58,10 +59,11 @@ class V8_EXPORT_PRIVATE StateValuesCache {
                                                     size_t* node_count,
                                                     size_t* values_idx,
                                                     Node** values, size_t count,
-                                                    const BitVector* liveness);
+                                                    const BitVector* liveness,
+                                                    int liveness_offset);
 
   Node* BuildTree(size_t* values_idx, Node** values, size_t count,
-                  const BitVector* liveness, size_t level);
+                  const BitVector* liveness, int liveness_offset, size_t level);
 
   WorkingBuffer* GetWorkingSpace(size_t level);
   Node* GetEmptyStateValues();