[compiler] Optimize AddInputsToFrameStateDescriptor

Optimizes InstructionSelector::AddInputsToFrameStateDescriptor by
taking advantage of SparseInputMask data structure to more quickly
handle empty inputs and insert all the OptimizedOut entries in one go.
The number of empty inputs is now determined using CountTrailingZeros
rather than iterating over them one at a time.

Gives a 9% improvement to SelectInstructions runtime call stat for
Octane in turboprop.

Bug: v8:10051
Change-Id: Ib13d6f9644b4c89ba0546a19fe0ed623d69fec99
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2037443
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66591}

src/compiler/backend/instruction-selector.cc

@@ -574,11 +574,7 @@ size_t InstructionSelector::AddOperandToStateValueDescriptor(
     StateValueList* values, InstructionOperandVector* inputs,
     OperandGenerator* g, StateObjectDeduplicator* deduplicator, Node* input,
     MachineType type, FrameStateInputKind kind, Zone* zone) {
-  if (input == nullptr) {
-    values->PushOptimizedOut();
-    return 0;
-  }
-
+  DCHECK_NOT_NULL(input);
   switch (input->opcode()) {
     case IrOpcode::kArgumentsElementsState: {
       values->PushArgumentsElements(ArgumentsStateTypeOf(input->op()));
@@ -636,6 +632,26 @@ size_t InstructionSelector::AddOperandToStateValueDescriptor(
   }
 }
 
+size_t InstructionSelector::AddInputsToFrameStateDescriptor(
+    StateValueList* values, InstructionOperandVector* inputs,
+    OperandGenerator* g, StateObjectDeduplicator* deduplicator, Node* node,
+    FrameStateInputKind kind, Zone* zone) {
+  size_t entries = 0;
+  StateValuesAccess::iterator it = StateValuesAccess(node).begin();
+  // Take advantage of sparse nature of StateValuesAccess to skip over multiple
+  // empty nodes at once pushing repeated OptimizedOuts all in one go.
+  while (!it.done()) {
+    values->PushOptimizedOut(it.AdvanceTillNotEmpty());
+    if (it.done()) break;
+    StateValuesAccess::TypedNode input_node = *it;
+    entries += AddOperandToStateValueDescriptor(values, inputs, g, deduplicator,
+                                                input_node.node,
+                                                input_node.type, kind, zone);
+    ++it;
+  }
+  return entries;
+}
+
 // Returns the number of instruction operands added to inputs.
 size_t InstructionSelector::AddInputsToFrameStateDescriptor(
     FrameStateDescriptor* descriptor, Node* state, OperandGenerator* g,
@@ -669,30 +685,25 @@ size_t InstructionSelector::AddInputsToFrameStateDescriptor(
   DCHECK_EQ(values_descriptor->size(), 0u);
   values_descriptor->ReserveSize(descriptor->GetSize());
 
+  DCHECK_NOT_NULL(function);
   entries += AddOperandToStateValueDescriptor(
       values_descriptor, inputs, g, deduplicator, function,
       MachineType::AnyTagged(), FrameStateInputKind::kStackSlot, zone);
-  for (StateValuesAccess::TypedNode input_node :
-       StateValuesAccess(parameters)) {
-    entries += AddOperandToStateValueDescriptor(values_descriptor, inputs, g,
-                                                deduplicator, input_node.node,
-                                                input_node.type, kind, zone);
-  }
+
+  entries += AddInputsToFrameStateDescriptor(
+      values_descriptor, inputs, g, deduplicator, parameters, kind, zone);
+
   if (descriptor->HasContext()) {
+    DCHECK_NOT_NULL(context);
     entries += AddOperandToStateValueDescriptor(
         values_descriptor, inputs, g, deduplicator, context,
         MachineType::AnyTagged(), FrameStateInputKind::kStackSlot, zone);
   }
-  for (StateValuesAccess::TypedNode input_node : StateValuesAccess(locals)) {
-    entries += AddOperandToStateValueDescriptor(values_descriptor, inputs, g,
-                                                deduplicator, input_node.node,
-                                                input_node.type, kind, zone);
-  }
-  for (StateValuesAccess::TypedNode input_node : StateValuesAccess(stack)) {
-    entries += AddOperandToStateValueDescriptor(values_descriptor, inputs, g,
-                                                deduplicator, input_node.node,
-                                                input_node.type, kind, zone);
-  }
+
+  entries += AddInputsToFrameStateDescriptor(values_descriptor, inputs, g,
+                                             deduplicator, locals, kind, zone);
+  entries += AddInputsToFrameStateDescriptor(values_descriptor, inputs, g,
+                                             deduplicator, stack, kind, zone);
   DCHECK_EQ(initial_size + entries, inputs->size());
   return entries;
 }

src/compiler/backend/instruction-selector.h

@@ -576,6 +576,12 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
                                          StateObjectDeduplicator* deduplicator,
                                          InstructionOperandVector* inputs,
                                          FrameStateInputKind kind, Zone* zone);
+  size_t AddInputsToFrameStateDescriptor(StateValueList* values,
+                                         InstructionOperandVector* inputs,
+                                         OperandGenerator* g,
+                                         StateObjectDeduplicator* deduplicator,
+                                         Node* node, FrameStateInputKind kind,
+                                         Zone* zone);
   size_t AddOperandToStateValueDescriptor(StateValueList* values,
                                           InstructionOperandVector* inputs,
                                           OperandGenerator* g,

src/compiler/backend/instruction.h

@@ -1212,8 +1212,8 @@ class StateValueList {
   void PushPlain(MachineType type) {
     fields_.push_back(StateValueDescriptor::Plain(type));
   }
-  void PushOptimizedOut() {
-    fields_.push_back(StateValueDescriptor::OptimizedOut());
+  void PushOptimizedOut(size_t num = 1) {
+    fields_.insert(fields_.end(), num, StateValueDescriptor::OptimizedOut());
   }
 
   iterator begin() { return iterator(fields_.begin(), nested_.begin()); }

src/compiler/common-operator.cc

@@ -282,6 +282,15 @@ void SparseInputMask::InputIterator::Advance() {
   bit_mask_ >>= 1;
 }
 
+size_t SparseInputMask::InputIterator::AdvanceToNextRealOrEnd() {
+  DCHECK_NE(bit_mask_, SparseInputMask::kDenseBitMask);
+
+  size_t count = base::bits::CountTrailingZeros(bit_mask_);
+  bit_mask_ >>= count;
+  DCHECK(IsReal() || IsEnd());
+  return count;
+}
+
 Node* SparseInputMask::InputIterator::GetReal() const {
   DCHECK(IsReal());
   return parent_->InputAt(real_index_);

src/compiler/common-operator.h

@@ -273,6 +273,11 @@ class SparseInputMask final {
     // current sparse input is real.
     Node* GetReal() const;
 
+    // Advance to the next real value or the end. Only valid if the iterator is
+    // not dense. Returns the number of empty values that were skipped. This can
+    // return 0 and in that case, it does not advance.
+    size_t AdvanceToNextRealOrEnd();
+
     // Get the current sparse input, returning either a real input node if
     // the current sparse input is real, or the given {empty_value} if the
     // current sparse input is empty.

src/compiler/js-create-lowering.cc

@@ -1421,9 +1421,9 @@ Node* JSCreateLowering::AllocateArguments(Node* effect, Node* control,
   a.AllocateArray(argument_count,
                   MapRef(broker(), factory()->fixed_array_map()));
   for (int i = 0; i < argument_count; ++i, ++parameters_it) {
-    DCHECK_NOT_NULL((*parameters_it).node);
+    DCHECK_NOT_NULL(parameters_it.node());
     a.Store(AccessBuilder::ForFixedArrayElement(), jsgraph()->Constant(i),
-            (*parameters_it).node);
+            parameters_it.node());
   }
   return a.Finish();
 }
@@ -1452,9 +1452,9 @@ Node* JSCreateLowering::AllocateRestArguments(Node* effect, Node* control,
   AllocationBuilder a(jsgraph(), effect, control);
   a.AllocateArray(num_elements, MapRef(broker(), factory()->fixed_array_map()));
   for (int i = 0; i < num_elements; ++i, ++parameters_it) {
-    DCHECK_NOT_NULL((*parameters_it).node);
+    DCHECK_NOT_NULL(parameters_it.node());
     a.Store(AccessBuilder::ForFixedArrayElement(), jsgraph()->Constant(i),
-            (*parameters_it).node);
+            parameters_it.node());
   }
   return a.Finish();
 }
@@ -1496,9 +1496,9 @@ Node* JSCreateLowering::AllocateAliasedArguments(
              jsgraph()->TheHoleConstant());
   }
   for (int i = mapped_count; i < argument_count; ++i, ++parameters_it) {
-    DCHECK_NOT_NULL((*parameters_it).node);
+    DCHECK_NOT_NULL(parameters_it.node());
     aa.Store(AccessBuilder::ForFixedArrayElement(), jsgraph()->Constant(i),
-             (*parameters_it).node);
+             parameters_it.node());
   }
   Node* arguments = aa.Finish();
 

src/compiler/state-values-utils.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/state-values-utils.h"
 
+#include "src/compiler/common-operator.h"
 #include "src/utils/bit-vector.h"
 
 namespace v8 {
@@ -240,9 +241,9 @@ void CheckTreeContainsValues(Node* tree, Node** values, size_t count,
   auto itend = access.end();
   for (i = 0; it != itend; ++it, ++i) {
     if (liveness == nullptr || liveness->Contains(liveness_offset + i)) {
-      DCHECK_EQ((*it).node, values[i]);
+      DCHECK_EQ(it.node(), values[i]);
     } else {
-      DCHECK_NULL((*it).node);
+      DCHECK_NULL(it.node());
     }
   }
   DCHECK_EQ(static_cast<size_t>(i), count);
@@ -329,13 +330,20 @@ void StateValuesAccess::iterator::Pop() {
   current_depth_--;
 }
 
-bool StateValuesAccess::iterator::done() const { return current_depth_ < 0; }
-
 void StateValuesAccess::iterator::Advance() {
   Top()->Advance();
   EnsureValid();
 }
 
+size_t StateValuesAccess::iterator::AdvanceTillNotEmpty() {
+  size_t count = 0;
+  while (!done() && Top()->IsEmpty()) {
+    count += Top()->AdvanceToNextRealOrEnd();
+    EnsureValid();
+  }
+  return count;
+}
+
 void StateValuesAccess::iterator::EnsureValid() {
   while (true) {
     SparseInputMask::InputIterator* top = Top();
@@ -376,21 +384,18 @@ Node* StateValuesAccess::iterator::node() { return Top()->Get(nullptr); }
 
 MachineType StateValuesAccess::iterator::type() {
   Node* parent = Top()->parent();
+  DCHECK(!Top()->IsEmpty());
   if (parent->opcode() == IrOpcode::kStateValues) {
     return MachineType::AnyTagged();
   } else {
     DCHECK_EQ(IrOpcode::kTypedStateValues, parent->opcode());
 
-    if (Top()->IsEmpty()) {
-      return MachineType::None();
-    } else {
-      ZoneVector<MachineType> const* types = MachineTypesOf(parent->op());
-      return (*types)[Top()->real_index()];
-    }
+    ZoneVector<MachineType> const* types = MachineTypesOf(parent->op());
+    return (*types)[Top()->real_index()];
   }
 }
 
-bool StateValuesAccess::iterator::operator!=(iterator const& other) {
+bool StateValuesAccess::iterator::operator!=(iterator const& other) const {
   // We only allow comparison with end().
   CHECK(other.done());
   return !done();
@@ -406,8 +411,7 @@ StateValuesAccess::TypedNode StateValuesAccess::iterator::operator*() {
   return TypedNode(node(), type());
 }
 
-
-size_t StateValuesAccess::size() {
+size_t StateValuesAccess::size() const {
   size_t count = 0;
   SparseInputMask mask = SparseInputMaskOf(node_->op());
 

src/compiler/state-values-utils.h

@@ -6,6 +6,7 @@
 #define V8_COMPILER_STATE_VALUES_UTILS_H_
 
 #include <array>
+
 #include "src/common/globals.h"
 #include "src/compiler/common-operator.h"
 #include "src/compiler/js-graph.h"
@@ -91,20 +92,23 @@ class V8_EXPORT_PRIVATE StateValuesAccess {
 
   class V8_EXPORT_PRIVATE iterator {
    public:
-    // Bare minimum of operators needed for range iteration.
-    bool operator!=(iterator const& other);
+    bool operator!=(iterator const& other) const;
     iterator& operator++();
     TypedNode operator*();
 
+    Node* node();
+    bool done() const { return current_depth_ < 0; }
+
+    // Returns the number of empty nodes that were skipped over.
+    size_t AdvanceTillNotEmpty();
+
    private:
     friend class StateValuesAccess;
 
     iterator() : current_depth_(-1) {}
     explicit iterator(Node* node);
 
-    Node* node();
     MachineType type();
-    bool done() const;
     void Advance();
     void EnsureValid();
 
@@ -119,9 +123,9 @@ class V8_EXPORT_PRIVATE StateValuesAccess {
 
   explicit StateValuesAccess(Node* node) : node_(node) {}
 
-  size_t size();
-  iterator begin() { return iterator(node_); }
-  iterator end() { return iterator(); }
+  size_t size() const;
+  iterator begin() const { return iterator(node_); }
+  iterator end() const { return iterator(); }
 
  private:
   Node* node_;

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

@@ -153,11 +153,13 @@ TEST_F(StateValuesIteratorTest, TreeFromVectorWithLiveness) {
 
     // Check the tree contents with vector.
     int i = 0;
-    for (StateValuesAccess::TypedNode node : StateValuesAccess(values_node)) {
+    for (StateValuesAccess::iterator it =
+             StateValuesAccess(values_node).begin();
+         !it.done(); ++it) {
       if (liveness.Contains(i)) {
-        EXPECT_THAT(node.node, IsInt32Constant(i));
+        EXPECT_THAT(it.node(), IsInt32Constant(i));
       } else {
-        EXPECT_EQ(node.node, nullptr);
+        EXPECT_EQ(it.node(), nullptr);
       }
       i++;
     }