[compiler] Make accumulator index 0 in liveness bitvectors

Previously, the accumulator was at the end of liveness bitvectors, which
meant that checking for accumulator liveness required a length lookup.
This CL moves it to the start of the bitvector, with registers starting
at index 1 -- the assumption is that the addition of 1 to the index on
register liveness access can be constant folded away.

As a cleanup, replace all the custom liveness printing code with a
single unified ToString. This places the accumulator at the end of the
printed liveness, to avoid having to change test expectations (also, the
position of the accumulator is now an implementation detail). As a
similar cleanup, change StateValue node building to use the
BytecodeLivenessState interface rather than the underlying bitvector.
These two cleanups allow us to remove the raw bitvector accessor from
liveness entirely.

Change-Id: Ic2744b5e8e16b8527e6a4e8d3b4ddad7096289d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3455144
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79066}

src/compiler/bytecode-analysis.cc

@@ -717,21 +717,12 @@ std::ostream& BytecodeAnalysis::PrintLivenessTo(std::ostream& os) const {
   for (; !iterator.done(); iterator.Advance()) {
     int current_offset = iterator.current_offset();
 
-    const BitVector& in_liveness =
-        GetInLivenessFor(current_offset)->bit_vector();
-    const BitVector& out_liveness =
-        GetOutLivenessFor(current_offset)->bit_vector();
+    const BytecodeLivenessState* in_liveness = GetInLivenessFor(current_offset);
+    const BytecodeLivenessState* out_liveness =
+        GetOutLivenessFor(current_offset);
 
-    for (int i = 0; i < in_liveness.length(); ++i) {
-      os << (in_liveness.Contains(i) ? "L" : ".");
-    }
-    os << " -> ";
-
-    for (int i = 0; i < out_liveness.length(); ++i) {
-      os << (out_liveness.Contains(i) ? "L" : ".");
-    }
-
-    os << " | " << current_offset << ": ";
+    os << ToString(*in_liveness) << " -> " << ToString(*out_liveness) << " | "
+       << current_offset << ": ";
     iterator.PrintTo(os) << std::endl;
   }
 
@@ -984,22 +975,16 @@ bool BytecodeAnalysis::LivenessIsValid() {
     interpreter::BytecodeArrayIterator forward_iterator(bytecode_array());
     for (; !forward_iterator.done(); forward_iterator.Advance()) {
       int current_offset = forward_iterator.current_offset();
-      const BitVector& in_liveness =
-          GetInLivenessFor(current_offset)->bit_vector();
-      const BitVector& out_liveness =
-          GetOutLivenessFor(current_offset)->bit_vector();
-
-      for (int i = 0; i < in_liveness.length(); ++i) {
-        of << (in_liveness.Contains(i) ? 'L' : '.');
-      }
-
-      of << " | ";
+      const BytecodeLivenessState* in_liveness =
+          GetInLivenessFor(current_offset);
+      const BytecodeLivenessState* out_liveness =
+          GetOutLivenessFor(current_offset);
 
-      for (int i = 0; i < out_liveness.length(); ++i) {
-        of << (out_liveness.Contains(i) ? 'L' : '.');
-      }
+      std::string in_liveness_str = ToString(*in_liveness);
+      std::string out_liveness_str = ToString(*out_liveness);
 
-      of << " : " << current_offset << " : ";
+      of << in_liveness_str << " | " << out_liveness_str << " : "
+         << current_offset << " : ";
 
       // Draw loop back edges by indentin everything between loop headers and
       // jump loop instructions.
@@ -1023,21 +1008,10 @@ bool BytecodeAnalysis::LivenessIsValid() {
 
       if (current_offset == invalid_offset) {
         // Underline the invalid liveness.
-        if (which_invalid == 0) {
-          for (int i = 0; i < in_liveness.length(); ++i) {
-            of << '^';
-          }
-          for (int i = 0; i < out_liveness.length() + 3; ++i) {
-            of << ' ';
-          }
-        } else {
-          for (int i = 0; i < in_liveness.length() + 3; ++i) {
-            of << ' ';
-          }
-          for (int i = 0; i < out_liveness.length(); ++i) {
-            of << '^';
-          }
-        }
+        char in_underline = which_invalid == 0 ? '^' : ' ';
+        char out_underline = which_invalid == 0 ? ' ' : '^';
+        of << std::string(in_liveness_str.size(), in_underline) << "   "
+           << std::string(out_liveness_str.size(), out_underline);
 
         // Make sure to draw the loop indentation marks on this additional line.
         of << " : " << current_offset << " : ";
@@ -1049,19 +1023,11 @@ bool BytecodeAnalysis::LivenessIsValid() {
 
         // Print the invalid liveness.
         if (which_invalid == 0) {
-          for (int i = 0; i < in_liveness.length(); ++i) {
-            of << (invalid_liveness.bit_vector().Contains(i) ? 'L' : '.');
-          }
-          for (int i = 0; i < out_liveness.length() + 3; ++i) {
-            of << ' ';
-          }
+          of << ToString(invalid_liveness) << "   "
+             << std::string(out_liveness_str.size(), ' ');
         } else {
-          for (int i = 0; i < in_liveness.length() + 3; ++i) {
-            of << ' ';
-          }
-          for (int i = 0; i < out_liveness.length(); ++i) {
-            of << (invalid_liveness.bit_vector().Contains(i) ? 'L' : '.');
-          }
+          of << std::string(in_liveness_str.size(), ' ') << "   "
+             << ToString(invalid_liveness);
         }
 
         // Make sure to draw the loop indentation marks on this additional line.

src/compiler/bytecode-graph-builder.cc

@@ -610,7 +610,7 @@ class BytecodeGraphBuilder::Environment : public ZoneObject {
   bool StateValuesRequireUpdate(Node** state_values, Node** values, int count);
   void UpdateStateValues(Node** state_values, Node** values, int count);
   Node* GetStateValuesFromCache(Node** values, int count,
-                                const BitVector* liveness, int liveness_offset);
+                                const BytecodeLivenessState* liveness);
 
   int RegisterToValuesIndex(interpreter::Register the_register) const;
 
@@ -995,9 +995,9 @@ void BytecodeGraphBuilder::Environment::UpdateStateValues(Node** state_values,
 }
 
 Node* BytecodeGraphBuilder::Environment::GetStateValuesFromCache(
-    Node** values, int count, const BitVector* liveness, int liveness_offset) {
+    Node** values, int count, const BytecodeLivenessState* liveness) {
   return builder_->state_values_cache_.GetNodeForValues(
-      values, static_cast<size_t>(count), liveness, liveness_offset);
+      values, static_cast<size_t>(count), liveness);
 }
 
 Node* BytecodeGraphBuilder::Environment::Checkpoint(
@@ -1006,16 +1006,15 @@ Node* BytecodeGraphBuilder::Environment::Checkpoint(
   if (parameter_count() == register_count()) {
     // Re-use the state-value cache if the number of local registers happens
     // to match the parameter count.
-    parameters_state_values_ = GetStateValuesFromCache(
-        &values()->at(0), parameter_count(), nullptr, 0);
+    parameters_state_values_ =
+        GetStateValuesFromCache(&values()->at(0), parameter_count(), nullptr);
   } else {
     UpdateStateValues(&parameters_state_values_, &values()->at(0),
                       parameter_count());
   }
 
-  Node* registers_state_values =
-      GetStateValuesFromCache(&values()->at(register_base()), register_count(),
-                              liveness ? &liveness->bit_vector() : nullptr, 0);
+  Node* registers_state_values = GetStateValuesFromCache(
+      &values()->at(register_base()), register_count(), liveness);
 
   bool accumulator_is_live = !liveness || liveness->AccumulatorIsLive();
   Node* accumulator_state_value =

src/compiler/bytecode-liveness-map.cc

@@ -8,6 +8,24 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+std::string ToString(const BytecodeLivenessState& liveness) {
+  std::string out;
+  out.resize(liveness.register_count() + 1);
+  for (int i = 0; i < liveness.register_count(); ++i) {
+    if (liveness.RegisterIsLive(i)) {
+      out[i] = 'L';
+    } else {
+      out[i] = '.';
+    }
+  }
+  if (liveness.AccumulatorIsLive()) {
+    out[liveness.register_count()] = 'L';
+  } else {
+    out[liveness.register_count()] = '.';
+  }
+  return out;
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/bytecode-liveness-map.h

@@ -5,6 +5,8 @@
 #ifndef V8_COMPILER_BYTECODE_LIVENESS_MAP_H_
 #define V8_COMPILER_BYTECODE_LIVENESS_MAP_H_
 
+#include <string>
+
 #include "src/utils/bit-vector.h"
 #include "src/zone/zone.h"
 
@@ -25,19 +27,13 @@ class BytecodeLivenessState : public ZoneObject {
   BytecodeLivenessState(const BytecodeLivenessState& other, Zone* zone)
       : bit_vector_(other.bit_vector_, zone) {}
 
-  const BitVector& bit_vector() const { return bit_vector_; }
-
-  BitVector& bit_vector() { return bit_vector_; }
-
   bool RegisterIsLive(int index) const {
     DCHECK_GE(index, 0);
     DCHECK_LT(index, bit_vector_.length() - 1);
-    return bit_vector_.Contains(index);
+    return bit_vector_.Contains(index + 1);
   }
 
-  bool AccumulatorIsLive() const {
-    return bit_vector_.Contains(bit_vector_.length() - 1);
-  }
+  bool AccumulatorIsLive() const { return bit_vector_.Contains(0); }
 
   bool Equals(const BytecodeLivenessState& other) const {
     return bit_vector_.Equals(other.bit_vector_);
@@ -46,18 +42,18 @@ class BytecodeLivenessState : public ZoneObject {
   void MarkRegisterLive(int index) {
     DCHECK_GE(index, 0);
     DCHECK_LT(index, bit_vector_.length() - 1);
-    bit_vector_.Add(index);
+    bit_vector_.Add(index + 1);
   }
 
   void MarkRegisterDead(int index) {
     DCHECK_GE(index, 0);
     DCHECK_LT(index, bit_vector_.length() - 1);
-    bit_vector_.Remove(index);
+    bit_vector_.Remove(index + 1);
   }
 
-  void MarkAccumulatorLive() { bit_vector_.Add(bit_vector_.length() - 1); }
+  void MarkAccumulatorLive() { bit_vector_.Add(0); }
 
-  void MarkAccumulatorDead() { bit_vector_.Remove(bit_vector_.length() - 1); }
+  void MarkAccumulatorDead() { bit_vector_.Remove(0); }
 
   void MarkAllLive() { bit_vector_.AddAll(); }
 
@@ -73,6 +69,8 @@ class BytecodeLivenessState : public ZoneObject {
     bit_vector_.CopyFrom(other.bit_vector_);
   }
 
+  int register_count() const { return bit_vector_.length() - 1; }
+
  private:
   BitVector bit_vector_;
 };
@@ -138,6 +136,8 @@ class V8_EXPORT_PRIVATE BytecodeLivenessMap {
 #endif
 };
 
+V8_EXPORT_PRIVATE std::string ToString(const BytecodeLivenessState& liveness);
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/state-values-utils.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/state-values-utils.h"
 
+#include "src/compiler/bytecode-liveness-map.h"
 #include "src/compiler/common-operator.h"
 #include "src/utils/bit-vector.h"
 
@@ -137,8 +138,7 @@ 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,
-    int liveness_offset) {
+    Node** values, size_t count, const BytecodeLivenessState* liveness) {
   SparseInputMask::BitMaskType input_mask = 0;
 
   // Virtual nodes are the live nodes plus the implicit optimized out nodes,
@@ -150,7 +150,7 @@ SparseInputMask::BitMaskType StateValuesCache::FillBufferWithValues(
     DCHECK_LE(*values_idx, static_cast<size_t>(INT_MAX));
 
     if (liveness == nullptr ||
-        liveness->Contains(liveness_offset + static_cast<int>(*values_idx))) {
+        liveness->RegisterIsLive(static_cast<int>(*values_idx))) {
       input_mask |= 1 << (virtual_node_count);
       (*node_buffer)[(*node_count)++] = values[*values_idx];
     }
@@ -169,15 +169,16 @@ SparseInputMask::BitMaskType StateValuesCache::FillBufferWithValues(
 }
 
 Node* StateValuesCache::BuildTree(size_t* values_idx, Node** values,
-                                  size_t count, const BitVector* liveness,
-                                  int liveness_offset, size_t level) {
+                                  size_t count,
+                                  const BytecodeLivenessState* liveness,
+                                  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, liveness_offset);
+                                      values, count, liveness);
     // Make sure we returned a sparse input mask.
     DCHECK_NE(input_mask, SparseInputMask::kDenseBitMask);
   } else {
@@ -189,9 +190,8 @@ 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, liveness_offset);
+        input_mask = FillBufferWithValues(node_buffer, &node_count, values_idx,
+                                          values, count, liveness);
         // Make sure we have exhausted our values.
         DCHECK_EQ(*values_idx, count);
         // Make sure we returned a sparse input mask.
@@ -207,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,
-                                  liveness_offset, level - 1);
+        Node* subtree =
+            BuildTree(values_idx, values, count, liveness, level - 1);
         (*node_buffer)[node_count++] = subtree;
         // Don't touch the bitmask, so that it stays dense.
       }
@@ -231,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, int liveness_offset) {
+                             const BytecodeLivenessState* liveness) {
   DCHECK_EQ(count, StateValuesAccess(tree).size());
 
   int i;
@@ -239,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(liveness_offset + i)) {
+    if (liveness == nullptr || liveness->RegisterIsLive(i)) {
       DCHECK_EQ(it.node(), values[i]);
     } else {
       DCHECK_NULL(it.node());
@@ -251,9 +251,8 @@ void CheckTreeContainsValues(Node* tree, Node** values, size_t count,
 }  // namespace
 #endif
 
-Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
-                                         const BitVector* liveness,
-                                         int liveness_offset) {
+Node* StateValuesCache::GetNodeForValues(
+    Node** values, size_t count, const BytecodeLivenessState* liveness) {
 #if DEBUG
   // Check that the values represent actual values, and not a tree of values.
   for (size_t i = 0; i < count; i++) {
@@ -263,10 +262,10 @@ Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
     }
   }
   if (liveness != nullptr) {
-    DCHECK_LE(liveness_offset + count, static_cast<size_t>(liveness->length()));
+    DCHECK_LE(count, static_cast<size_t>(liveness->register_count()));
 
     for (size_t i = 0; i < count; i++) {
-      if (liveness->Contains(liveness_offset + static_cast<int>(i))) {
+      if (liveness->RegisterIsLive(static_cast<int>(i))) {
         DCHECK_NOT_NULL(values[i]);
       }
     }
@@ -289,8 +288,7 @@ Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
   }
 
   size_t values_idx = 0;
-  Node* tree =
-      BuildTree(&values_idx, values, count, liveness, liveness_offset, height);
+  Node* tree = BuildTree(&values_idx, values, count, liveness, height);
   // The values should be exhausted by the end of BuildTree.
   DCHECK_EQ(values_idx, count);
 
@@ -298,7 +296,7 @@ Node* StateValuesCache::GetNodeForValues(Node** values, size_t count,
   DCHECK_EQ(tree->opcode(), IrOpcode::kStateValues);
 
 #if DEBUG
-  CheckTreeContainsValues(tree, values, count, liveness, liveness_offset);
+  CheckTreeContainsValues(tree, values, count, liveness);
 #endif
 
   return tree;

src/compiler/state-values-utils.h

@@ -20,14 +20,14 @@ class BitVector;
 namespace compiler {
 
 class Graph;
+class BytecodeLivenessState;
 
 class V8_EXPORT_PRIVATE StateValuesCache {
  public:
   explicit StateValuesCache(JSGraph* js_graph);
 
   Node* GetNodeForValues(Node** values, size_t count,
-                         const BitVector* liveness = nullptr,
-                         int liveness_offset = 0);
+                         const BytecodeLivenessState* liveness = nullptr);
 
  private:
   static const size_t kMaxInputCount = 8;
@@ -57,15 +57,12 @@ class V8_EXPORT_PRIVATE StateValuesCache {
   // at {values_idx}, sparsely encoding according to {liveness}. {node_count} is
   // updated with the new number of inputs in {node_buffer}, and a bitmask of
   // the sparse encoding is returned.
-  SparseInputMask::BitMaskType FillBufferWithValues(WorkingBuffer* node_buffer,
-                                                    size_t* node_count,
-                                                    size_t* values_idx,
-                                                    Node** values, size_t count,
-                                                    const BitVector* liveness,
-                                                    int liveness_offset);
+  SparseInputMask::BitMaskType FillBufferWithValues(
+      WorkingBuffer* node_buffer, size_t* node_count, size_t* values_idx,
+      Node** values, size_t count, const BytecodeLivenessState* liveness);
 
   Node* BuildTree(size_t* values_idx, Node** values, size_t count,
-                  const BitVector* liveness, int liveness_offset, size_t level);
+                  const BytecodeLivenessState* liveness, size_t level);
 
   WorkingBuffer* GetWorkingSpace(size_t level);
   Node* GetEmptyStateValues();

test/unittests/compiler/bytecode-analysis-unittest.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/bytecode-analysis.h"
 
+#include "src/compiler/bytecode-liveness-map.h"
 #include "src/init/v8.h"
 #include "src/interpreter/bytecode-array-builder.h"
 #include "src/interpreter/bytecode-array-iterator.h"
@@ -42,21 +43,6 @@ class BytecodeAnalysisTest : public TestWithIsolateAndZone {
     save_flags_ = nullptr;
   }
 
-  std::string ToLivenessString(const BytecodeLivenessState* liveness) const {
-    const BitVector& bit_vector = liveness->bit_vector();
-
-    std::string out;
-    out.resize(bit_vector.length());
-    for (int i = 0; i < bit_vector.length(); ++i) {
-      if (bit_vector.Contains(i)) {
-        out[i] = 'L';
-      } else {
-        out[i] = '.';
-      }
-    }
-    return out;
-  }
-
   void EnsureLivenessMatches(
       Handle<BytecodeArray> bytecode,
       const std::vector<std::pair<std::string, std::string>>&
@@ -69,12 +55,13 @@ class BytecodeAnalysisTest : public TestWithIsolateAndZone {
       ss << std::setw(4) << iterator.current_offset() << " : ";
       iterator.PrintTo(ss);
 
-      EXPECT_EQ(liveness.first, ToLivenessString(analysis.GetInLivenessFor(
-                                    iterator.current_offset())))
+      EXPECT_EQ(liveness.first,
+                ToString(*analysis.GetInLivenessFor(iterator.current_offset())))
           << " at bytecode " << ss.str();
 
-      EXPECT_EQ(liveness.second, ToLivenessString(analysis.GetOutLivenessFor(
-                                     iterator.current_offset())))
+      EXPECT_EQ(
+          liveness.second,
+          ToString(*analysis.GetOutLivenessFor(iterator.current_offset())))
           << " at bytecode " << ss.str();
 
       iterator.Advance();

test/unittests/compiler/state-values-utils-unittest.cc

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "src/compiler/state-values-utils.h"
+
+#include "src/compiler/bytecode-liveness-map.h"
 #include "src/utils/bit-vector.h"
 #include "test/unittests/compiler/graph-unittest.h"
 #include "test/unittests/compiler/node-test-utils.h"
@@ -138,10 +140,10 @@ TEST_F(StateValuesIteratorTest, TreeFromVectorWithLiveness) {
       inputs.push_back(Int32Constant(i));
     }
     // Generate the input liveness.
-    BitVector liveness(count, zone());
+    BytecodeLivenessState liveness(count, zone());
     for (int i = 0; i < count; i++) {
       if (i % 3 == 0) {
-        liveness.Add(i);
+        liveness.MarkRegisterLive(i);
       }
     }
 
@@ -156,7 +158,7 @@ TEST_F(StateValuesIteratorTest, TreeFromVectorWithLiveness) {
     for (StateValuesAccess::iterator it =
              StateValuesAccess(values_node).begin();
          !it.done(); ++it) {
-      if (liveness.Contains(i)) {
+      if (liveness.RegisterIsLive(i)) {
         EXPECT_THAT(it.node(), IsInt32Constant(i));
       } else {
         EXPECT_EQ(it.node(), nullptr);
@@ -209,10 +211,10 @@ TEST_F(StateValuesIteratorTest, BuildTreeWithLivenessIdentical) {
       inputs.push_back(Int32Constant(i));
     }
     // Generate the input liveness.
-    BitVector liveness(count, zone());
+    BytecodeLivenessState liveness(count, zone());
     for (int i = 0; i < count; i++) {
       if (i % 3 == 0) {
-        liveness.Add(i);
+        liveness.MarkRegisterLive(i);
       }
     }