[turbofan] Optimize BranchElimination

We add a map from condition nodes to respective BranchConditions in
ControlPathConditions for faster lookup.

Bug: v8:11510
Change-Id: I571514beb699b76f2a1a0245c4785f518b9d8b1b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3041676
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76232}

src/compiler/branch-elimination.cc

@@ -101,8 +101,9 @@ void BranchElimination::SimplifyBranchCondition(Node* branch) {
     Node* input = inputs[i];
     ControlPathConditions from_input = node_conditions_.Get(input);
     if (!from_input.LookupCondition(branch_condition, &previous_branch,
-                                    &condition_value))
+                                    &condition_value)) {
       return;
+    }
 
     if (phase_ == kEARLY) {
       phi_inputs.emplace_back(condition_value ? jsgraph()->TrueConstant()
@@ -128,6 +129,7 @@ void BranchElimination::SimplifyBranchCondition(Node* branch) {
 Reduction BranchElimination::ReduceBranch(Node* node) {
   Node* condition = node->InputAt(0);
   Node* control_input = NodeProperties::GetControlInput(node, 0);
+  if (!reduced_.Get(control_input)) return NoChange();
   ControlPathConditions from_input = node_conditions_.Get(control_input);
   Node* branch;
   bool condition_value;
@@ -283,7 +285,7 @@ Reduction BranchElimination::ReduceMerge(Node* node) {
 }
 
 Reduction BranchElimination::ReduceStart(Node* node) {
-  return UpdateConditions(node, {});
+  return UpdateConditions(node, ControlPathConditions(zone_));
 }
 
 Reduction BranchElimination::ReduceOtherControl(Node* node) {
@@ -315,7 +317,7 @@ Reduction BranchElimination::UpdateConditions(
   // The control path for the node is the path obtained by appending the
   // current_condition to the prev_conditions. Use the original control path as
   // a hint to avoid allocations.
-  if (in_new_block || prev_conditions.Size() == 0) {
+  if (in_new_block || prev_conditions.blocks_.Size() == 0) {
     prev_conditions.AddConditionInNewBlock(zone_, current_condition,
                                            current_branch, is_true_branch);
   } else {
@@ -330,14 +332,17 @@ void BranchElimination::ControlPathConditions::AddCondition(
     Zone* zone, Node* condition, Node* branch, bool is_true,
     ControlPathConditions hint) {
   if (!LookupCondition(condition)) {
-    FunctionalList<BranchCondition> prev_front = Front();
-    if (hint.Size() > 0) {
-      prev_front.PushFront({condition, branch, is_true}, zone, hint.Front());
+    BranchCondition branch_condition(condition, branch, is_true);
+    FunctionalList<BranchCondition> prev_front = blocks_.Front();
+    if (hint.blocks_.Size() > 0) {
+      prev_front.PushFront(branch_condition, zone, hint.blocks_.Front());
     } else {
-      prev_front.PushFront({condition, branch, is_true}, zone);
+      prev_front.PushFront(branch_condition, zone);
     }
-    DropFront();
-    PushFront(prev_front, zone);
+    blocks_.DropFront();
+    blocks_.PushFront(prev_front, zone);
+    conditions_.Set(condition, branch_condition);
+    SLOW_DCHECK(BlocksAndConditionsInvariant());
   }
 }
 
@@ -345,35 +350,66 @@ void BranchElimination::ControlPathConditions::AddConditionInNewBlock(
     Zone* zone, Node* condition, Node* branch, bool is_true) {
   FunctionalList<BranchCondition> new_block;
   if (!LookupCondition(condition)) {
-    new_block.PushFront({condition, branch, is_true}, zone);
+    BranchCondition branch_condition(condition, branch, is_true);
+    new_block.PushFront(branch_condition, zone);
+    conditions_.Set(condition, branch_condition);
   }
-  PushFront(new_block, zone);
+  blocks_.PushFront(new_block, zone);
+  SLOW_DCHECK(BlocksAndConditionsInvariant());
 }
 
 bool BranchElimination::ControlPathConditions::LookupCondition(
     Node* condition) const {
-  for (auto block : *this) {
-    for (BranchCondition element : block) {
-      if (element.condition == condition) return true;
-    }
-  }
-  return false;
+  return conditions_.Get(condition).IsSet();
 }
 
 bool BranchElimination::ControlPathConditions::LookupCondition(
     Node* condition, Node** branch, bool* is_true) const {
-  for (auto block : *this) {
-    for (BranchCondition element : block) {
-      if (element.condition == condition) {
-        *is_true = element.is_true;
-        *branch = element.branch;
-        return true;
-      }
-    }
+  const BranchCondition& element = conditions_.Get(condition);
+  if (element.IsSet()) {
+    *is_true = element.is_true;
+    *branch = element.branch;
+    return true;
   }
   return false;
 }
 
+void BranchElimination::ControlPathConditions::ResetToCommonAncestor(
+    ControlPathConditions other) {
+  while (other.blocks_.Size() > blocks_.Size()) other.blocks_.DropFront();
+  while (blocks_.Size() > other.blocks_.Size()) {
+    for (BranchCondition branch_condition : blocks_.Front()) {
+      conditions_.Set(branch_condition.condition, {});
+    }
+    blocks_.DropFront();
+  }
+  while (blocks_ != other.blocks_) {
+    for (BranchCondition branch_condition : blocks_.Front()) {
+      conditions_.Set(branch_condition.condition, {});
+    }
+    blocks_.DropFront();
+    other.blocks_.DropFront();
+  }
+  SLOW_DCHECK(BlocksAndConditionsInvariant());
+}
+
+#if DEBUG
+bool BranchElimination::ControlPathConditions::BlocksAndConditionsInvariant() {
+  PersistentMap<Node*, BranchCondition> conditions_copy(conditions_);
+  for (auto block : blocks_) {
+    for (BranchCondition condition : block) {
+      // Every element of blocks_ has to be in conditions_.
+      if (conditions_copy.Get(condition.condition) != condition) return false;
+      conditions_copy.Set(condition.condition, {});
+    }
+  }
+  // Every element of {conditions_} has to be in {blocks_}. We removed all
+  // elements of blocks_ from condition_copy, so if it is not empty, the
+  // invariant fails.
+  return conditions_copy.begin() == conditions_copy.end();
+}
+#endif
+
 void BranchElimination::MarkAsSafetyCheckIfNeeded(Node* branch, Node* node) {
   // Check if {branch} is dead because we might have a stale side-table entry.
   if (!branch->IsDead() && branch->opcode() != IrOpcode::kDead &&

src/compiler/branch-elimination.h

@@ -10,6 +10,7 @@
 #include "src/compiler/functional-list.h"
 #include "src/compiler/graph-reducer.h"
 #include "src/compiler/node-aux-data.h"
+#include "src/compiler/persistent-map.h"
 
 namespace v8 {
 namespace internal {
@@ -38,6 +39,9 @@ class V8_EXPORT_PRIVATE BranchElimination final
   // Represents a condition along with its value in the current control path.
   // Also stores the node that branched on this condition.
   struct BranchCondition {
+    BranchCondition() : condition(nullptr), branch(nullptr), is_true(false) {}
+    BranchCondition(Node* condition, Node* branch, bool is_true)
+        : condition(condition), branch(branch), is_true(is_true) {}
     Node* condition;
     Node* branch;
     bool is_true;
@@ -47,15 +51,17 @@ class V8_EXPORT_PRIVATE BranchElimination final
              is_true == other.is_true;
     }
     bool operator!=(BranchCondition other) const { return !(*this == other); }
+
+    bool IsSet() const { return branch != nullptr; }
   };
 
   // Class for tracking information about branch conditions. It is represented
   // as a linked list of condition blocks, each of which corresponds to a block
   // of code bewteen an IfTrue/IfFalse and a Merge. Each block is in turn
   // represented as a linked list of {BranchCondition}s.
-  class ControlPathConditions
-      : public FunctionalList<FunctionalList<BranchCondition>> {
+  class ControlPathConditions {
    public:
+    explicit ControlPathConditions(Zone* zone) : conditions_(zone) {}
     // Checks if {condition} is present in this {ControlPathConditions}.
     bool LookupCondition(Node* condition) const;
     // Checks if {condition} is present in this {ControlPathConditions} and
@@ -68,9 +74,29 @@ class V8_EXPORT_PRIVATE BranchElimination final
     // Adds a condition in a new block.
     void AddConditionInNewBlock(Zone* zone, Node* condition, Node* branch,
                                 bool is_true);
+    // Reset this {ControlPathConditions} to the longest prefix that is common
+    // with {other}.
+    void ResetToCommonAncestor(ControlPathConditions other);
+
+    bool operator==(const ControlPathConditions& other) const {
+      return blocks_ == other.blocks_;
+    }
+    bool operator!=(const ControlPathConditions& other) const {
+      return blocks_ != other.blocks_;
+    }
+
+    friend class BranchElimination;
 
    private:
-    using FunctionalList<FunctionalList<BranchCondition>>::PushFront;
+    FunctionalList<FunctionalList<BranchCondition>> blocks_;
+    // This is an auxilliary data structure that provides fast lookups in the
+    // set of conditions. It should hold at any point that the contents of
+    // {blocks_} and {conditions_} is the same, which is implemented in
+    // {BlocksAndConditionsInvariant}.
+    PersistentMap<Node*, BranchCondition> conditions_;
+#if DEBUG
+    bool BlocksAndConditionsInvariant();
+#endif
   };
 
   Reduction ReduceBranch(Node* node);
@@ -101,7 +127,9 @@ class V8_EXPORT_PRIVATE BranchElimination final
   // Maps each control node to the condition information known about the node.
   // If the information is nullptr, then we have not calculated the information
   // yet.
-  NodeAuxData<ControlPathConditions> node_conditions_;
+
+  NodeAuxData<ControlPathConditions, ZoneConstruct<ControlPathConditions>>
+      node_conditions_;
   NodeAuxData<bool> reduced_;
   Zone* zone_;
   Node* dead_;

src/compiler/compiler-source-position-table.h

@@ -62,10 +62,14 @@ class V8_EXPORT_PRIVATE SourcePositionTable final
  private:
   class Decorator;
 
+  static SourcePosition UnknownSourcePosition(Zone* zone) {
+    return SourcePosition::Unknown();
+  }
+
   Graph* const graph_;
   Decorator* decorator_;
   SourcePosition current_position_;
-  NodeAuxData<SourcePosition, SourcePosition::Unknown> table_;
+  NodeAuxData<SourcePosition, UnknownSourcePosition> table_;
 };
 
 }  // namespace compiler

src/compiler/node-aux-data.h

@@ -16,21 +16,26 @@ namespace compiler {
 class Node;
 
 template <class T>
-T DefaultConstruct() {
+T DefaultConstruct(Zone* zone) {
   return T();
 }
 
-template <class T, T def() = DefaultConstruct<T>>
+template <class T>
+T ZoneConstruct(Zone* zone) {
+  return T(zone);
+}
+
+template <class T, T def(Zone*) = DefaultConstruct<T>>
 class NodeAuxData {
  public:
-  explicit NodeAuxData(Zone* zone) : aux_data_(zone) {}
+  explicit NodeAuxData(Zone* zone) : zone_(zone), aux_data_(zone) {}
   explicit NodeAuxData(size_t initial_size, Zone* zone)
-      : aux_data_(initial_size, zone) {}
+      : zone_(zone), aux_data_(initial_size, def(zone), zone) {}
 
   // Update entry. Returns true iff entry was changed.
   bool Set(Node* node, T const& data) {
     size_t const id = node->id();
-    if (id >= aux_data_.size()) aux_data_.resize(id + 1, def());
+    if (id >= aux_data_.size()) aux_data_.resize(id + 1, def(zone_));
     if (aux_data_[id] != data) {
       aux_data_[id] = data;
       return true;
@@ -40,7 +45,7 @@ class NodeAuxData {
 
   T Get(Node* node) const {
     size_t const id = node->id();
-    return (id < aux_data_.size()) ? aux_data_[id] : def();
+    return (id < aux_data_.size()) ? aux_data_[id] : def(zone_);
   }
 
   class const_iterator;
@@ -50,10 +55,11 @@ class NodeAuxData {
   const_iterator end() const;
 
  private:
+  Zone* zone_;
   ZoneVector<T> aux_data_;
 };
 
-template <class T, T def()>
+template <class T, T def(Zone*)>
 class NodeAuxData<T, def>::const_iterator {
  public:
   using iterator_category = std::forward_iterator_tag;
@@ -87,13 +93,13 @@ class NodeAuxData<T, def>::const_iterator {
   size_t current_;
 };
 
-template <class T, T def()>
+template <class T, T def(Zone*)>
 typename NodeAuxData<T, def>::const_iterator NodeAuxData<T, def>::begin()
     const {
   return typename NodeAuxData<T, def>::const_iterator(&aux_data_, 0);
 }
 
-template <class T, T def()>
+template <class T, T def(Zone*)>
 typename NodeAuxData<T, def>::const_iterator NodeAuxData<T, def>::end() const {
   return typename NodeAuxData<T, def>::const_iterator(&aux_data_,
                                                       aux_data_.size());

src/compiler/node-origin-table.h

@@ -136,7 +136,10 @@ class V8_EXPORT_PRIVATE NodeOriginTable final
   NodeOrigin current_origin_;
 
   const char* current_phase_name_;
-  NodeAuxData<NodeOrigin, NodeOrigin::Unknown> table_;
+  static NodeOrigin UnknownNodeOrigin(Zone* zone) {
+    return NodeOrigin::Unknown();
+  }
+  NodeAuxData<NodeOrigin, UnknownNodeOrigin> table_;
 };
 
 }  // namespace compiler