[ignition] Rewrite reverse iterator as random iterator

This allows us to optimise the bytecode liveness analysis to jump
directly to previously seen indices. The analysis is optimised to store
a stack of loop ends (JumpLoop bytecode indices), and iterate through
these indices directly rather than looping through the bytecode array to
find them.

Review-Url: https://codereview.chromium.org/2536653003
Cr-Commit-Position: refs/heads/master@{#41485}

BUILD.gn

@@ -1444,8 +1444,8 @@ v8_source_set("v8_base") {
     "src/interpreter/bytecode-array-builder.h",
     "src/interpreter/bytecode-array-iterator.cc",
     "src/interpreter/bytecode-array-iterator.h",
-    "src/interpreter/bytecode-array-reverse-iterator.cc",
-    "src/interpreter/bytecode-array-reverse-iterator.h",
+    "src/interpreter/bytecode-array-random-iterator.cc",
+    "src/interpreter/bytecode-array-random-iterator.h",
     "src/interpreter/bytecode-array-writer.cc",
     "src/interpreter/bytecode-array-writer.h",
     "src/interpreter/bytecode-dead-code-optimizer.cc",

src/DEPS

@@ -12,7 +12,7 @@ include_rules = [
   "-src/interpreter",
   "+src/interpreter/bytecode-array-accessor.h",
   "+src/interpreter/bytecode-array-iterator.h",
-  "+src/interpreter/bytecode-array-reverse-iterator.h",
+  "+src/interpreter/bytecode-array-random-iterator.h",
   "+src/interpreter/bytecode-decoder.h",
   "+src/interpreter/bytecode-flags.h",
   "+src/interpreter/bytecode-register.h",

src/compiler/bytecode-analysis.cc

@@ -5,7 +5,7 @@
 #include "src/compiler/bytecode-analysis.h"
 
 #include "src/interpreter/bytecode-array-iterator.h"
-#include "src/interpreter/bytecode-array-reverse-iterator.h"
+#include "src/interpreter/bytecode-array-random-iterator.h"
 #include "src/objects-inl.h"
 
 namespace v8 {
@@ -20,6 +20,7 @@ BytecodeAnalysis::BytecodeAnalysis(Handle<BytecodeArray> bytecode_array,
       do_liveness_analysis_(do_liveness_analysis),
       zone_(zone),
       loop_stack_(zone),
+      loop_end_index_queue_(zone),
       end_to_header_(zone),
       header_to_parent_(zone),
       liveness_map_(bytecode_array->length(), zone) {}
@@ -151,12 +152,8 @@ void BytecodeAnalysis::Analyze() {
 
   BitVector* next_bytecode_in_liveness = nullptr;
 
-  // The last JumpLoop that we haven't done a guaranteed valid liveness pass
-  // over. See the below wall of text for a more thorough explanation.
-  int last_invalid_jumploop_offset = -1;
-
-  BytecodeArrayReverseIterator iterator(bytecode_array(), zone());
-  for (; !iterator.done(); iterator.Advance()) {
+  BytecodeArrayRandomIterator iterator(bytecode_array(), zone());
+  for (iterator.GoToEnd(); iterator.IsValid(); --iterator) {
     Bytecode bytecode = iterator.current_bytecode();
     int current_offset = iterator.current_offset();
 
@@ -166,9 +163,9 @@ void BytecodeAnalysis::Analyze() {
       int loop_end = current_offset + iterator.current_bytecode_size();
       PushLoop(iterator.GetJumpTargetOffset(), loop_end);
 
-      // Save the last offset so that we can do another pass later.
-      if (last_invalid_jumploop_offset == -1) {
-        last_invalid_jumploop_offset = current_offset;
+      // Save the index so that we can do another pass later.
+      if (do_liveness_analysis_) {
+        loop_end_index_queue_.push_back(iterator.current_index());
       }
     } else if (current_offset == loop_stack_.top()) {
       loop_stack_.pop();
@@ -214,69 +211,48 @@ void BytecodeAnalysis::Analyze() {
   // This means that in a pass, we can iterate backwards over the bytecode
   // array, process any loops that we encounter, and on subsequent passes we can
   // skip processing those loops (though we still have to process inner loops).
+  //
+  // Equivalently, we can queue up loop ends from back to front, and pass over
+  // the loops in that order, as this preserves both the bottom-to-top and
+  // outer-to-inner requirements.
 
-  while (last_invalid_jumploop_offset != -1) {
-    // TODO(leszeks): We shouldn't need to iterate here, we should just have a
-    // random access iterator.
-    iterator.Reset();
-    while (last_invalid_jumploop_offset < iterator.current_offset()) {
-      iterator.Advance();
-    }
-    last_invalid_jumploop_offset = -1;
+  for (int loop_end_index : loop_end_index_queue_) {
+    iterator.GoToIndex(loop_end_index);
 
     DCHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpLoop);
 
-    for (; !iterator.done(); iterator.Advance()) {
-      Bytecode bytecode = iterator.current_bytecode();
-      if (bytecode != Bytecode::kJumpLoop) {
-        // Skip bytecodes until we hit a JumpLoop. This check isn't needed for
-        // the first loop we see (thanks to saving its offset), but it is for
-        // subsequent ones we want to process on this pass.
-        continue;
-      }
+    int header_offset = iterator.GetJumpTargetOffset();
+    int end_offset = iterator.current_offset();
 
-      int header_offset = iterator.GetJumpTargetOffset();
-      int end_offset = iterator.current_offset();
-
-      Liveness& header_liveness = liveness_map_.GetLiveness(header_offset);
-      Liveness& end_liveness = liveness_map_.GetLiveness(end_offset);
-
-      if (end_liveness.out->UnionIsChanged(*header_liveness.in)) {
-        // Only update the loop body if the loop end liveness changed.
-        end_liveness.in->CopyFrom(*end_liveness.out);
-        next_bytecode_in_liveness = end_liveness.in;
-
-        // Advance into the loop body.
-        iterator.Advance();
-        for (; iterator.current_offset() > header_offset; iterator.Advance()) {
-          bytecode = iterator.current_bytecode();
-          if (bytecode == Bytecode::kJumpLoop) {
-            // We can't validate this loop at the moment because we can't
-            // guarantee that its header is valid yet. Save it for later.
-            if (last_invalid_jumploop_offset == -1) {
-              last_invalid_jumploop_offset = iterator.current_offset();
-            }
-          }
+    Liveness& header_liveness = liveness_map_.GetLiveness(header_offset);
+    Liveness& end_liveness = liveness_map_.GetLiveness(end_offset);
 
-          int current_offset = iterator.current_offset();
-          Liveness& liveness = liveness_map_.GetLiveness(current_offset);
+    if (!end_liveness.out->UnionIsChanged(*header_liveness.in)) {
+      // Only update the loop body if the loop end liveness changed.
+      continue;
+    }
+    end_liveness.in->CopyFrom(*end_liveness.out);
+    next_bytecode_in_liveness = end_liveness.in;
 
-          UpdateOutLiveness(bytecode, *liveness.out, next_bytecode_in_liveness,
-                            iterator, liveness_map_);
-          liveness.in->CopyFrom(*liveness.out);
-          UpdateInLiveness(bytecode, *liveness.in, iterator);
+    // Advance into the loop body.
+    --iterator;
+    for (; iterator.current_offset() > header_offset; --iterator) {
+      Bytecode bytecode = iterator.current_bytecode();
 
-          next_bytecode_in_liveness = liveness.in;
-        }
-        // Now we are at the loop header. Since the in-liveness of the header
-        // can't change, we need only to update the out-liveness.
-        bytecode = iterator.current_bytecode();
-        UpdateOutLiveness(bytecode, *header_liveness.out,
-                          next_bytecode_in_liveness, iterator, liveness_map_);
-      }
+      int current_offset = iterator.current_offset();
+      Liveness& liveness = liveness_map_.GetLiveness(current_offset);
+
+      UpdateOutLiveness(bytecode, *liveness.out, next_bytecode_in_liveness,
+                        iterator, liveness_map_);
+      liveness.in->CopyFrom(*liveness.out);
+      UpdateInLiveness(bytecode, *liveness.in, iterator);
 
-      // Keep the iterator going so that we can find other loops.
+      next_bytecode_in_liveness = liveness.in;
     }
+    // Now we are at the loop header. Since the in-liveness of the header
+    // can't change, we need only to update the out-liveness.
+    UpdateOutLiveness(iterator.current_bytecode(), *header_liveness.out,
+                      next_bytecode_in_liveness, iterator, liveness_map_);
   }
 
   DCHECK(LivenessIsValid());
@@ -376,7 +352,7 @@ std::ostream& BytecodeAnalysis::PrintLivenessTo(std::ostream& os) const {
 
 #if DEBUG
 bool BytecodeAnalysis::LivenessIsValid() {
-  BytecodeArrayReverseIterator iterator(bytecode_array(), zone());
+  BytecodeArrayRandomIterator iterator(bytecode_array(), zone());
 
   BitVector previous_liveness(bytecode_array()->register_count() + 1, zone());
 
@@ -386,7 +362,7 @@ bool BytecodeAnalysis::LivenessIsValid() {
   BitVector* next_bytecode_in_liveness = nullptr;
 
   // Ensure that there are no liveness changes if we iterate one more time.
-  for (iterator.Reset(); !iterator.done(); iterator.Advance()) {
+  for (iterator.GoToEnd(); iterator.IsValid(); --iterator) {
     Bytecode bytecode = iterator.current_bytecode();
 
     int current_offset = iterator.current_offset();

src/compiler/bytecode-analysis.h

@@ -66,6 +66,7 @@ class V8_EXPORT_PRIVATE BytecodeAnalysis BASE_EMBEDDED {
   Zone* zone_;
 
   ZoneStack<int> loop_stack_;
+  ZoneVector<int> loop_end_index_queue_;
 
   ZoneMap<int, int> end_to_header_;
   ZoneMap<int, int> header_to_parent_;

src/interpreter/bytecode-array-reverse-iterator.cc → src/interpreter/bytecode-array-random-iterator.cc

@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/interpreter/bytecode-array-reverse-iterator.h"
+#include "src/interpreter/bytecode-array-random-iterator.h"
 #include "src/objects-inl.h"
 
 namespace v8 {
 namespace internal {
 namespace interpreter {
 
-BytecodeArrayReverseIterator::BytecodeArrayReverseIterator(
+BytecodeArrayRandomIterator::BytecodeArrayRandomIterator(
     Handle<BytecodeArray> bytecode_array, Zone* zone)
     : BytecodeArrayAccessor(bytecode_array, 0), offsets_(zone) {
   // Run forwards through the bytecode array to determine the offset of each
@@ -18,26 +18,17 @@ BytecodeArrayReverseIterator::BytecodeArrayReverseIterator(
     offsets_.push_back(current_offset());
     SetOffset(current_offset() + current_bytecode_size());
   }
-  Reset();
+  GoToStart();
 }
 
-void BytecodeArrayReverseIterator::Advance() {
-  it_offsets_++;
-  UpdateOffsetFromIterator();
+bool BytecodeArrayRandomIterator::IsValid() const {
+  return current_index_ >= 0 &&
+         static_cast<size_t>(current_index_) < offsets_.size();
 }
 
-void BytecodeArrayReverseIterator::Reset() {
-  it_offsets_ = offsets_.rbegin();
-  UpdateOffsetFromIterator();
-}
-
-bool BytecodeArrayReverseIterator::done() const {
-  return it_offsets_ == offsets_.rend();
-}
-
-void BytecodeArrayReverseIterator::UpdateOffsetFromIterator() {
-  if (it_offsets_ != offsets_.rend()) {
-    SetOffset(*it_offsets_);
+void BytecodeArrayRandomIterator::UpdateOffsetFromIndex() {
+  if (IsValid()) {
+    SetOffset(offsets_[current_index_]);
   }
 }
 

src/interpreter/bytecode-array-random-iterator.h

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_INTERPRETER_BYTECODE_ARRAY_RANDOM_ITERATOR_H_
+#define V8_INTERPRETER_BYTECODE_ARRAY_RANDOM_ITERATOR_H_
+
+#include "src/interpreter/bytecode-array-accessor.h"
+#include "src/zone/zone-containers.h"
+#include "src/zone/zone.h"
+
+namespace v8 {
+namespace internal {
+namespace interpreter {
+
+class V8_EXPORT_PRIVATE BytecodeArrayRandomIterator final
+    : public BytecodeArrayAccessor {
+ public:
+  explicit BytecodeArrayRandomIterator(Handle<BytecodeArray> bytecode_array,
+                                       Zone* zone);
+
+  BytecodeArrayRandomIterator& operator++() {
+    ++current_index_;
+    UpdateOffsetFromIndex();
+    return *this;
+  }
+  BytecodeArrayRandomIterator& operator--() {
+    --current_index_;
+    UpdateOffsetFromIndex();
+    return *this;
+  }
+
+  BytecodeArrayRandomIterator& operator+=(int offset) {
+    current_index_ += offset;
+    UpdateOffsetFromIndex();
+    return *this;
+  }
+
+  BytecodeArrayRandomIterator& operator-=(int offset) {
+    current_index_ -= offset;
+    UpdateOffsetFromIndex();
+    return *this;
+  }
+
+  int current_index() const { return current_index_; }
+
+  size_t size() const { return offsets_.size(); }
+
+  void GoToIndex(int index) {
+    current_index_ = index;
+    UpdateOffsetFromIndex();
+  }
+  void GoToStart() {
+    current_index_ = 0;
+    UpdateOffsetFromIndex();
+  }
+  void GoToEnd() {
+    DCHECK_LT(offsets_.size() - 1, static_cast<size_t>(INT_MAX));
+    current_index_ = static_cast<int>(offsets_.size() - 1);
+    UpdateOffsetFromIndex();
+  }
+
+  bool IsValid() const;
+
+ private:
+  ZoneVector<int> offsets_;
+  int current_index_;
+
+  void UpdateOffsetFromIndex();
+
+  DISALLOW_COPY_AND_ASSIGN(BytecodeArrayRandomIterator);
+};
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_INTERPRETER_BYTECODE_ARRAY_RANDOM_ITERATOR_H_

src/interpreter/bytecode-array-reverse-iterator.h

-// Copyright 2016 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef V8_INTERPRETER_BYTECODE_ARRAY_REVERSE_ITERATOR_H_
-#define V8_INTERPRETER_BYTECODE_ARRAY_REVERSE_ITERATOR_H_
-
-#include "src/interpreter/bytecode-array-accessor.h"
-#include "src/zone/zone-containers.h"
-#include "src/zone/zone.h"
-
-namespace v8 {
-namespace internal {
-namespace interpreter {
-
-class V8_EXPORT_PRIVATE BytecodeArrayReverseIterator final
-    : public BytecodeArrayAccessor {
- public:
-  explicit BytecodeArrayReverseIterator(Handle<BytecodeArray> bytecode_array,
-                                        Zone* zone);
-
-  void Advance();
-  void Reset();
-  bool done() const;
-
- private:
-  ZoneVector<int> offsets_;
-  ZoneVector<int>::const_reverse_iterator it_offsets_;
-
-  void UpdateOffsetFromIterator();
-
-  DISALLOW_COPY_AND_ASSIGN(BytecodeArrayReverseIterator);
-};
-
-}  // namespace interpreter
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_INTERPRETER_BYTECODE_ARRAY_REVERSE_ITERATOR_H_

src/v8.gyp

@@ -982,8 +982,8 @@
         'interpreter/bytecode-array-builder.h',
         'interpreter/bytecode-array-iterator.cc',
         'interpreter/bytecode-array-iterator.h',
-        'interpreter/bytecode-array-reverse-iterator.cc',
-        'interpreter/bytecode-array-reverse-iterator.h',
+        'interpreter/bytecode-array-random-iterator.cc',
+        'interpreter/bytecode-array-random-iterator.h',
         'interpreter/bytecode-array-writer.cc',
         'interpreter/bytecode-array-writer.h',
         'interpreter/bytecode-dead-code-optimizer.cc',

test/unittests/BUILD.gn

@@ -99,7 +99,7 @@ v8_executable("unittests") {
     "heap/slot-set-unittest.cc",
     "interpreter/bytecode-array-builder-unittest.cc",
     "interpreter/bytecode-array-iterator-unittest.cc",
-    "interpreter/bytecode-array-reverse-iterator-unittest.cc",
+    "interpreter/bytecode-array-random-iterator-unittest.cc",
     "interpreter/bytecode-array-writer-unittest.cc",
     "interpreter/bytecode-dead-code-optimizer-unittest.cc",
     "interpreter/bytecode-decoder-unittest.cc",

test/unittests/unittests.gyp

@@ -90,7 +90,7 @@
       'interpreter/bytecodes-unittest.cc',
       'interpreter/bytecode-array-builder-unittest.cc',
       'interpreter/bytecode-array-iterator-unittest.cc',
-      'interpreter/bytecode-array-reverse-iterator-unittest.cc',
+      'interpreter/bytecode-array-random-iterator-unittest.cc',
       'interpreter/bytecode-array-writer-unittest.cc',
       'interpreter/bytecode-dead-code-optimizer-unittest.cc',
       'interpreter/bytecode-decoder-unittest.cc',