[turbofan] Add support for advanced reducers.

An AdvancedReducer is basically a regular Reducer with an editor
that can perform graph editing operations beyond changing or
replacing the node that is currently being reduced. The GraphReducer
is the default implementation of the AdvancedReducer::Editor interface.

The ControlReducerImpl is now just an AdvancedReducer, which
temporarily requires a Finish method in the reducer to implement
the dead node trimming until we move that to the GraphReducer
(which in turn requires that all loops are connected to End).

Review URL: https://codereview.chromium.org/1122423003

Cr-Commit-Position: refs/heads/master@{#28251}

src/compiler/control-reducer.cc

@@ -5,6 +5,7 @@
 #include "src/compiler/common-operator.h"
 #include "src/compiler/control-reducer.h"
 #include "src/compiler/graph.h"
+#include "src/compiler/graph-reducer.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/node-marker.h"
 #include "src/compiler/node-matchers.h"
@@ -47,56 +48,25 @@ class ReachabilityMarker : public NodeMarker<uint8_t> {
 };
 
 
-class ControlReducerImpl {
+class ControlReducerImpl final : public AdvancedReducer {
  public:
-  ControlReducerImpl(Zone* zone, JSGraph* jsgraph,
-                     CommonOperatorBuilder* common)
-      : zone_(zone),
-        jsgraph_(jsgraph),
-        common_(common),
-        state_(jsgraph->graph()->NodeCount(), kUnvisited, zone_),
-        stack_(zone_),
-        revisit_(zone_),
-        max_phis_for_select_(0) {}
-
   Zone* zone_;
   JSGraph* jsgraph_;
-  CommonOperatorBuilder* common_;
-  ZoneVector<VisitState> state_;
-  ZoneDeque<Node*> stack_;
-  ZoneDeque<Node*> revisit_;
   int max_phis_for_select_;
 
-  void Reduce() {
-    Push(graph()->end());
-    do {
-      // Process the node on the top of the stack, potentially pushing more
-      // or popping the node off the stack.
-      ReduceTop();
-      // If the stack becomes empty, revisit any nodes in the revisit queue.
-      // If no nodes in the revisit queue, try removing dead loops.
-      // If no dead loops, then finish.
-    } while (!stack_.empty() || TryRevisit() || RepairAndRemoveLoops());
-  }
-
-  bool TryRevisit() {
-    while (!revisit_.empty()) {
-      Node* n = revisit_.back();
-      revisit_.pop_back();
-      if (state_[n->id()] == kRevisit) {  // state can change while in queue.
-        Push(n);
-        return true;
-      }
-    }
-    return false;
-  }
+  ControlReducerImpl(Editor* editor, Zone* zone, JSGraph* jsgraph)
+      : AdvancedReducer(editor),
+        zone_(zone),
+        jsgraph_(jsgraph),
+        max_phis_for_select_(0) {}
 
-  // Repair the graph after the possible creation of non-terminating or dead
-  // loops. Removing dead loops can produce more opportunities for reduction.
-  bool RepairAndRemoveLoops() {
-    // TODO(turbofan): we can skip this if the graph has no loops, but
-    // we have to be careful about proper loop detection during reduction.
+  Graph* graph() { return jsgraph_->graph(); }
+  CommonOperatorBuilder* common() { return jsgraph_->common(); }
+  Node* dead() { return jsgraph_->DeadControl(); }
 
+  // Finish reducing the graph by trimming nodes and/or connecting NTLs.
+  bool Finish() final {
+    bool done = true;
     // Gather all nodes backwards-reachable from end (through inputs).
     ReachabilityMarker marked(graph());
     NodeVector nodes(zone_);
@@ -164,11 +134,15 @@ class ControlReducerImpl {
     for (size_t i = 0; i < nodes.size(); i++) {
       Node* node = nodes[i];
       if (node->op()->ControlOutputCount() > 0 &&
-          !marked.IsReachableFromStart(node)) {
-        ReplaceNode(node, dead());  // uses will be added to revisit queue.
+          !marked.IsReachableFromStart(node) &&
+          node->opcode() != IrOpcode::kDead) {
+        TRACE("Dead: #%d:%s\n", node->id(), node->op()->mnemonic());
+        node->ReplaceUses(dead());
+        done = false;
       }
     }
-    return TryRevisit();  // try to push a node onto the stack.
+
+    return done;
   }
 
   // Connect {loop}, the header of a non-terminating loop, to the end node.
@@ -176,23 +150,12 @@ class ControlReducerImpl {
     TRACE("ConnectNTL: #%d:%s\n", loop->id(), loop->op()->mnemonic());
     DCHECK_EQ(IrOpcode::kLoop, loop->opcode());
 
-    Node* always = graph()->NewNode(common_->Always());
-    // Mark the node as visited so that we can revisit later.
-    MarkAsVisited(always);
-
-    Node* branch = graph()->NewNode(common_->Branch(), always, loop);
-    // Mark the node as visited so that we can revisit later.
-    MarkAsVisited(branch);
-
-    Node* if_true = graph()->NewNode(common_->IfTrue(), branch);
-    // Mark the node as visited so that we can revisit later.
-    MarkAsVisited(if_true);
-
-    Node* if_false = graph()->NewNode(common_->IfFalse(), branch);
-    // Mark the node as visited so that we can revisit later.
-    MarkAsVisited(if_false);
+    Node* always = graph()->NewNode(common()->Always());
+    Node* branch = graph()->NewNode(common()->Branch(), always, loop);
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
 
-    // Hook up the branch into the loop and collect all loop effects.
+    // Insert the branch into the loop and collect all loop effects.
     NodeVector effects(zone_);
     for (auto edge : loop->use_edges()) {
       DCHECK_EQ(loop, edge.to());
@@ -217,35 +180,35 @@ class ControlReducerImpl {
     if (effects_count == 1) {
       effect = effects[0];
     } else if (effects_count > 1) {
-      effect = graph()->NewNode(common_->EffectSet(effects_count),
+      effect = graph()->NewNode(common()->EffectSet(effects_count),
                                 effects_count, &effects.front());
-      // Mark the node as visited so that we can revisit later.
-      MarkAsVisited(effect);
     }
 
     // Add a return to connect the NTL to the end.
     Node* ret = graph()->NewNode(
-        common_->Return(), jsgraph_->UndefinedConstant(), effect, if_false);
-    // Mark the node as visited so that we can revisit later.
-    MarkAsVisited(ret);
+        common()->Return(), jsgraph_->UndefinedConstant(), effect, if_false);
 
     Node* end = graph()->end();
-    CHECK_EQ(IrOpcode::kEnd, end->opcode());
+    if (end->opcode() == IrOpcode::kDead) {
+      // End is actually the dead node. Make a new end.
+      end = graph()->NewNode(common()->End(), ret);
+      graph()->SetEnd(end);
+      return end;
+    }
+    // End is not dead.
     Node* merge = end->InputAt(0);
     if (merge == NULL || merge->opcode() == IrOpcode::kDead) {
       // The end node died; just connect end to {ret}.
       end->ReplaceInput(0, ret);
     } else if (merge->opcode() != IrOpcode::kMerge) {
       // Introduce a final merge node for {end->InputAt(0)} and {ret}.
-      merge = graph()->NewNode(common_->Merge(2), merge, ret);
+      merge = graph()->NewNode(common()->Merge(2), merge, ret);
       end->ReplaceInput(0, merge);
       ret = merge;
-      // Mark the node as visited so that we can revisit later.
-      MarkAsVisited(merge);
     } else {
       // Append a new input to the final merge at the end.
       merge->AppendInput(graph()->zone(), ret);
-      merge->set_op(common_->Merge(merge->InputCount()));
+      merge->set_op(common()->Merge(merge->InputCount()));
     }
     return ret;
   }
@@ -320,116 +283,48 @@ class ControlReducerImpl {
 #endif
   }
 
-  // Reduce the node on the top of the stack.
-  // If an input {i} is not yet visited or needs to be revisited, push {i} onto
-  // the stack and return. Otherwise, all inputs are visited, so apply
-  // reductions for {node} and pop it off the stack.
-  void ReduceTop() {
-    size_t height = stack_.size();
-    Node* node = stack_.back();
-
-    if (node->IsDead()) return Pop();  // Node was killed while on stack.
-
-    TRACE("ControlReduce: #%d:%s\n", node->id(), node->op()->mnemonic());
-
-    // Recurse on an input if necessary.
-    for (Node* const input : node->inputs()) {
-      DCHECK(input);
-      if (Recurse(input)) return;
-    }
-
-    // All inputs should be visited or on stack. Apply reductions to node.
-    Node* replacement = ReduceNode(node);
-    if (replacement != node) ReplaceNode(node, replacement);
-
-    // After reducing the node, pop it off the stack.
-    CHECK_EQ(static_cast<int>(height), static_cast<int>(stack_.size()));
-    Pop();
-
-    // If there was a replacement, reduce it after popping {node}.
-    if (replacement != node) Recurse(replacement);
-  }
-
-  void EnsureStateSize(size_t id) {
-    if (id >= state_.size()) {
-      state_.resize((3 * id) / 2, kUnvisited);
-    }
-  }
-
-  // Push a node onto the stack if its state is {kUnvisited} or {kRevisit}.
-  bool Recurse(Node* node) {
-    size_t id = static_cast<size_t>(node->id());
-    EnsureStateSize(id);
-    if (state_[id] != kRevisit && state_[id] != kUnvisited) return false;
-    Push(node);
-    return true;
-  }
-
-  void Push(Node* node) {
-    state_[node->id()] = kOnStack;
-    stack_.push_back(node);
-  }
-
-  void Pop() {
-    int pos = static_cast<int>(stack_.size()) - 1;
-    DCHECK_GE(pos, 0);
-    DCHECK_EQ(kOnStack, state_[stack_[pos]->id()]);
-    state_[stack_[pos]->id()] = kVisited;
-    stack_.pop_back();
-  }
-
-  // Queue a node to be revisited if it has been visited once already.
-  void Revisit(Node* node) {
-    size_t id = static_cast<size_t>(node->id());
-    if (id < state_.size() && state_[id] == kVisited) {
-      TRACE("  Revisit #%d:%s\n", node->id(), node->op()->mnemonic());
-      state_[id] = kRevisit;
-      revisit_.push_back(node);
-    }
-  }
-
-  // Mark {node} as visited.
-  void MarkAsVisited(Node* node) {
-    size_t id = static_cast<size_t>(node->id());
-    EnsureStateSize(id);
-    state_[id] = kVisited;
-  }
-
-  Node* dead() { return jsgraph_->DeadControl(); }
-
   //===========================================================================
   // Reducer implementation: perform reductions on a node.
   //===========================================================================
-  Node* ReduceNode(Node* node) {
+  Reduction Reduce(Node* node) override {
     if (node->op()->ControlInputCount() == 1 ||
         node->opcode() == IrOpcode::kLoop) {
       // If a node has only one control input and it is dead, replace with dead.
       Node* control = NodeProperties::GetControlInput(node);
       if (control->opcode() == IrOpcode::kDead) {
         TRACE("ControlDead: #%d:%s\n", node->id(), node->op()->mnemonic());
-        return control;
+        return Replace(control);
       }
     }
 
+    Node* result = node;
     // Reduce branches, phis, and merges.
     switch (node->opcode()) {
       case IrOpcode::kBranch:
-        return ReduceBranch(node);
+        result = ReduceBranch(node);
+        break;
       case IrOpcode::kIfTrue:
-        return ReduceIfProjection(node, kTrue);
+        result = ReduceIfProjection(node, kTrue);
+        break;
       case IrOpcode::kIfFalse:
-        return ReduceIfProjection(node, kFalse);
-      case IrOpcode::kLoop:
+        result = ReduceIfProjection(node, kFalse);
+        break;
+      case IrOpcode::kLoop:  // fallthrough
       case IrOpcode::kMerge:
-        return ReduceMerge(node);
+        result = ReduceMerge(node);
+        break;
       case IrOpcode::kSelect:
-        return ReduceSelect(node);
+        result = ReduceSelect(node);
+        break;
       case IrOpcode::kPhi:
       case IrOpcode::kEffectPhi:
-        return ReducePhi(node);
+        result = ReducePhi(node);
+        break;
       default:
-        return node;
+        break;
     }
+
+    return result == node ? NoChange() : Replace(result);
   }
 
   // Try to statically fold a condition.
@@ -543,7 +438,9 @@ class ControlReducerImpl {
 
     if (live == 1) {
       // All phis are redundant. Replace them with their live input.
-      for (Node* const phi : phis) ReplaceNode(phi, phi->InputAt(live_index));
+      for (Node* const phi : phis) {
+        Replace(phi, phi->InputAt(live_index));
+      }
       // The merge itself is redundant.
       return node->InputAt(live_index);
     }
@@ -583,13 +480,13 @@ class ControlReducerImpl {
           Node* cond = matcher.Branch()->InputAt(0);
           for (Node* phi : phis) {
             Node* select = graph()->NewNode(
-                common_->Select(OpParameter<MachineType>(phi),
+                common()->Select(OpParameter<MachineType>(phi),
                                  BranchHintOf(matcher.Branch()->op())),
                 cond, matcher.TrueInputOf(phi), matcher.FalseInputOf(phi));
             TRACE("  MatchSelect: #%d:Branch #%d:IfTrue #%d:IfFalse -> #%d\n",
                   matcher.Branch()->id(), matcher.IfTrue()->id(),
                   matcher.IfFalse()->id(), select->id());
-            ReplaceNode(phi, select);
+            Replace(phi, select);
           }
           return control;
         }
@@ -641,65 +538,63 @@ class ControlReducerImpl {
     DCHECK_EQ(total, live + node->InputCount() - merge->InputCount());
     DCHECK_NE(total, node->InputCount());
     node->TrimInputCount(total);
-    node->set_op(common_->ResizeMergeOrPhi(node->op(), live));
+    node->set_op(common()->ResizeMergeOrPhi(node->op(), live));
   }
-
-  // Replace uses of {node} with {replacement} and revisit the uses.
-  void ReplaceNode(Node* node, Node* replacement) {
-    if (node == replacement) return;
-    TRACE("  Replace: #%d:%s with #%d:%s\n", node->id(), node->op()->mnemonic(),
-          replacement->id(), replacement->op()->mnemonic());
-    for (Node* const use : node->uses()) {
-      // Don't revisit this node if it refers to itself.
-      if (use != node) Revisit(use);
-    }
-    node->ReplaceUses(replacement);
-    node->Kill();
-  }
-
-  Graph* graph() { return jsgraph_->graph(); }
 };
 
 
 void ControlReducer::ReduceGraph(Zone* zone, JSGraph* jsgraph,
-                                 CommonOperatorBuilder* common,
                                  int max_phis_for_select) {
-  ControlReducerImpl impl(zone, jsgraph, common);
+  GraphReducer graph_reducer(jsgraph->graph(), zone);
+  ControlReducerImpl impl(&graph_reducer, zone, jsgraph);
   impl.max_phis_for_select_ = max_phis_for_select;
-  impl.Reduce();
+  graph_reducer.AddReducer(&impl);
+  graph_reducer.ReduceGraph();
 }
 
 
+namespace {
+
+class DummyEditor final : public AdvancedReducer::Editor {
+ public:
+  void Replace(Node* node, Node* replacement) final {
+    node->ReplaceUses(replacement);
+  }
+  void Revisit(Node* node) final {}
+};
+
+}  // namespace
+
+
 void ControlReducer::TrimGraph(Zone* zone, JSGraph* jsgraph) {
-  ControlReducerImpl impl(zone, jsgraph, NULL);
+  DummyEditor editor;
+  ControlReducerImpl impl(&editor, zone, jsgraph);
   impl.Trim();
 }
 
 
-Node* ControlReducer::ReduceMerge(JSGraph* jsgraph,
-                                  CommonOperatorBuilder* common, Node* node,
+Node* ControlReducer::ReduceMerge(JSGraph* jsgraph, Node* node,
                                   int max_phis_for_select) {
   Zone zone;
-  ControlReducerImpl impl(&zone, jsgraph, common);
+  DummyEditor editor;
+  ControlReducerImpl impl(&editor, &zone, jsgraph);
   impl.max_phis_for_select_ = max_phis_for_select;
   return impl.ReduceMerge(node);
 }
 
 
-Node* ControlReducer::ReducePhiForTesting(JSGraph* jsgraph,
-                                          CommonOperatorBuilder* common,
-                                          Node* node) {
+Node* ControlReducer::ReducePhiForTesting(JSGraph* jsgraph, Node* node) {
   Zone zone;
-  ControlReducerImpl impl(&zone, jsgraph, common);
+  DummyEditor editor;
+  ControlReducerImpl impl(&editor, &zone, jsgraph);
   return impl.ReducePhi(node);
 }
 
 
-Node* ControlReducer::ReduceIfNodeForTesting(JSGraph* jsgraph,
-                                             CommonOperatorBuilder* common,
-                                             Node* node) {
+Node* ControlReducer::ReduceIfNodeForTesting(JSGraph* jsgraph, Node* node) {
   Zone zone;
-  ControlReducerImpl impl(&zone, jsgraph, common);
+  DummyEditor editor;
+  ControlReducerImpl impl(&editor, &zone, jsgraph);
   switch (node->opcode()) {
     case IrOpcode::kIfTrue:
       return impl.ReduceIfProjection(node, kTrue);
@@ -709,6 +604,7 @@ Node* ControlReducer::ReduceIfNodeForTesting(JSGraph* jsgraph,
       return node;
   }
 }
-}
-}
-}  // namespace v8::internal::compiler
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8

src/compiler/control-reducer.h

@@ -23,22 +23,18 @@ class ControlReducer {
  public:
   // Perform branch folding and dead code elimination on the graph.
   static void ReduceGraph(Zone* zone, JSGraph* graph,
-                          CommonOperatorBuilder* builder,
                           int max_phis_for_select = 0);
 
   // Trim nodes in the graph that are not reachable from end.
   static void TrimGraph(Zone* zone, JSGraph* graph);
 
   // Reduces a single merge node and attached phis.
-  static Node* ReduceMerge(JSGraph* graph, CommonOperatorBuilder* builder,
-                           Node* node, int max_phis_for_select = 0);
+  static Node* ReduceMerge(JSGraph* graph, Node* node,
+                           int max_phis_for_select = 0);
 
   // Testing interface.
-  static Node* ReducePhiForTesting(JSGraph* graph,
-                                   CommonOperatorBuilder* builder, Node* node);
-  static Node* ReduceIfNodeForTesting(JSGraph* graph,
-                                      CommonOperatorBuilder* builder,
-                                      Node* node);
+  static Node* ReducePhiForTesting(JSGraph* graph, Node* node);
+  static Node* ReduceIfNodeForTesting(JSGraph* graph, Node* node);
 };
 }
 }

src/compiler/graph-reducer.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <functional>
+#include <limits>
 
 #include "src/compiler/graph.h"
 #include "src/compiler/graph-reducer.h"
@@ -12,6 +13,9 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+bool Reducer::Finish() { return true; }
+
+
 enum class GraphReducer::State : uint8_t {
   kUnvisited,
   kRevisit,
@@ -28,6 +32,9 @@ GraphReducer::GraphReducer(Graph* graph, Zone* zone)
       stack_(zone) {}
 
 
+GraphReducer::~GraphReducer() {}
+
+
 void GraphReducer::AddReducer(Reducer* reducer) {
   reducers_.push_back(reducer);
 }
@@ -59,7 +66,23 @@ void GraphReducer::ReduceNode(Node* node) {
 }
 
 
-void GraphReducer::ReduceGraph() { ReduceNode(graph()->end()); }
+void GraphReducer::ReduceGraph() {
+  for (;;) {
+    ReduceNode(graph()->end());
+    // TODO(turbofan): Remove this once the dead node trimming is in the
+    // GraphReducer.
+    bool done = true;
+    for (Reducer* const reducer : reducers_) {
+      if (!reducer->Finish()) {
+        done = false;
+        break;
+      }
+    }
+    if (done) break;
+    // Reset all marks on the graph in preparation to re-reduce the graph.
+    state_.Reset(graph());
+  }
+}
 
 
 Reduction GraphReducer::Reduce(Node* const node) {
@@ -112,8 +135,8 @@ void GraphReducer::ReduceTop() {
     if (input != node && Recurse(input)) return;
   }
 
-  // Remember the node count before reduction.
-  const int node_count = graph()->NodeCount();
+  // Remember the max node id before reduction.
+  NodeId const max_id = graph()->NodeCount() - 1;
 
   // All inputs should be visited or on stack. Apply reductions to node.
   Reduction reduction = Reduce(node);
@@ -135,39 +158,54 @@ void GraphReducer::ReduceTop() {
   // After reducing the node, pop it off the stack.
   Pop();
 
+  // Check if we have a new replacement.
+  if (replacement != node) {
+    Replace(node, replacement, max_id);
+  } else {
     // Revisit all uses of the node.
-  for (Node* const use : node->uses()) {
+    for (Node* const user : node->uses()) {
       // Don't revisit this node if it refers to itself.
-    if (use != node) Revisit(use);
+      if (user != node) Revisit(user);
+    }
   }
+}
 
-  // Check if we have a new replacement.
-  if (replacement != node) {
+
+void GraphReducer::Replace(Node* node, Node* replacement) {
+  Replace(node, replacement, std::numeric_limits<NodeId>::max());
+}
+
+
+void GraphReducer::Replace(Node* node, Node* replacement, NodeId max_id) {
   if (node == graph()->start()) graph()->SetStart(replacement);
   if (node == graph()->end()) graph()->SetEnd(replacement);
-    // If {node} was replaced by an old node, unlink {node} and assume that
+  if (replacement->id() <= max_id) {
+    // {replacement} is an old node, so unlink {node} and assume that
     // {replacement} was already reduced and finish.
-    if (replacement->id() < node_count) {
-      node->ReplaceUses(replacement);
+    for (Edge edge : node->use_edges()) {
+      Node* const user = edge.from();
+      edge.UpdateTo(replacement);
+      // Don't revisit this node if it refers to itself.
+      if (user != node) Revisit(user);
+    }
     node->Kill();
   } else {
-      // Otherwise {node} was replaced by a new node. Replace all old uses of
-      // {node} with {replacement}. New nodes created by this reduction can
-      // use {node}.
+    // Replace all old uses of {node} with {replacement}, but allow new nodes
+    // created by this reduction to use {node}.
     for (Edge edge : node->use_edges()) {
-        if (edge.from()->id() < node_count) {
+      Node* const user = edge.from();
+      if (user->id() <= max_id) {
         edge.UpdateTo(replacement);
+        // Don't revisit this node if it refers to itself.
+        if (user != node) Revisit(user);
       }
     }
     // Unlink {node} if it's no longer used.
-      if (node->uses().empty()) {
-        node->Kill();
-      }
+    if (node->uses().empty()) node->Kill();
 
     // If there was a replacement, reduce it after popping {node}.
     Recurse(replacement);
   }
-  }
 }
 
 

src/compiler/graph-reducer.h

@@ -17,13 +17,18 @@ class Graph;
 class Node;
 
 
+// NodeIds are identifying numbers for nodes that can be used to index auxiliary
+// out-of-line data associated with each node.
+typedef int32_t NodeId;
+
+
 // Represents the result of trying to reduce a node in the graph.
 class Reduction final {
  public:
-  explicit Reduction(Node* replacement = NULL) : replacement_(replacement) {}
+  explicit Reduction(Node* replacement = nullptr) : replacement_(replacement) {}
 
   Node* replacement() const { return replacement_; }
-  bool Changed() const { return replacement() != NULL; }
+  bool Changed() const { return replacement() != nullptr; }
 
  private:
   Node* replacement_;
@@ -37,26 +42,66 @@ class Reduction final {
 // phase.
 class Reducer {
  public:
-  Reducer() {}
   virtual ~Reducer() {}
 
   // Try to reduce a node if possible.
   virtual Reduction Reduce(Node* node) = 0;
 
+  // Ask this reducer to finish operation, returns {true} if the reducer is
+  // done, while {false} indicates that the graph might need to be reduced
+  // again.
+  // TODO(turbofan): Remove this once the dead node trimming is in the
+  // GraphReducer.
+  virtual bool Finish();
+
   // Helper functions for subclasses to produce reductions for a node.
   static Reduction NoChange() { return Reduction(); }
   static Reduction Replace(Node* node) { return Reduction(node); }
   static Reduction Changed(Node* node) { return Reduction(node); }
+};
+
+
+// An advanced reducer can also edit the graphs by changing and replacing nodes
+// other than the one currently being reduced.
+class AdvancedReducer : public Reducer {
+ public:
+  // Observe the actions of this reducer.
+  class Editor {
+   public:
+    virtual ~Editor() {}
+
+    // Replace {node} with {replacement}.
+    virtual void Replace(Node* node, Node* replacement) = 0;
+    // Revisit the {node} again later.
+    virtual void Revisit(Node* node) = 0;
+  };
+
+  explicit AdvancedReducer(Editor* editor) : editor_(editor) {}
+
+ protected:
+  // Helper functions for subclasses to produce reductions for a node.
+  static Reduction Replace(Node* node) { return Reducer::Replace(node); }
+
+  // Helper functions for subclasses to edit the graph.
+  void Replace(Node* node, Node* replacement) {
+    DCHECK_NOT_NULL(editor_);
+    editor_->Replace(node, replacement);
+  }
+  void Revisit(Node* node) {
+    DCHECK_NOT_NULL(editor_);
+    editor_->Revisit(node);
+  }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(Reducer);
+  Editor* const editor_;
 };
 
 
 // Performs an iterative reduction of a node graph.
-class GraphReducer final {
+class GraphReducer final : public AdvancedReducer::Editor {
  public:
   GraphReducer(Graph* graph, Zone* zone);
+  ~GraphReducer() final;
 
   Graph* graph() const { return graph_; }
 
@@ -79,15 +124,22 @@ class GraphReducer final {
   // Reduce the node on top of the stack.
   void ReduceTop();
 
+  // Replace {node} with {replacement}.
+  void Replace(Node* node, Node* replacement) final;
+  // Replace all uses of {node} with {replacement} if the id of {replacement} is
+  // less than or equal to {max_id}. Otherwise, replace all uses of {node} whose
+  // id is less than or equal to {max_id} with the {replacement}.
+  void Replace(Node* node, Node* replacement, NodeId max_id);
+
   // Node stack operations.
   void Pop();
   void Push(Node* node);
 
   // Revisit queue operations.
   bool Recurse(Node* node);
-  void Revisit(Node* node);
+  void Revisit(Node* node) final;
 
-  Graph* graph_;
+  Graph* const graph_;
   NodeMarker<State> state_;
   ZoneVector<Reducer*> reducers_;
   ZoneStack<Node*> revisit_;

src/compiler/node-marker.cc

@@ -13,8 +13,8 @@ namespace compiler {
 
 NodeMarkerBase::NodeMarkerBase(Graph* graph, uint32_t num_states)
     : mark_min_(graph->mark_max_), mark_max_(graph->mark_max_ += num_states) {
-  DCHECK(num_states > 0);         // user error!
-  DCHECK(mark_max_ > mark_min_);  // check for wraparound.
+  DCHECK_NE(0u, num_states);        // user error!
+  DCHECK_LT(mark_min_, mark_max_);  // check for wraparound.
 }
 
 
@@ -35,6 +35,14 @@ void NodeMarkerBase::Set(Node* node, Mark mark) {
   node->set_mark(mark + mark_min_);
 }
 
+
+void NodeMarkerBase::Reset(Graph* graph) {
+  uint32_t const num_states = mark_max_ - mark_min_;
+  mark_min_ = graph->mark_max_;
+  mark_max_ = graph->mark_max_ += num_states;
+  DCHECK_LT(mark_min_, mark_max_);  // check for wraparound.
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/node-marker.h

@@ -29,6 +29,7 @@ class NodeMarkerBase {
 
   Mark Get(Node* node);
   void Set(Node* node, Mark mark);
+  void Reset(Graph* graph);
 
  private:
   Mark mark_min_;

src/compiler/osr.cc

@@ -339,12 +339,12 @@ void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
   // but we need to avoid that because the osr_loop is reachable through
   // the second input, so reduce it and its phis manually.
   osr_loop->ReplaceInput(0, dead);
-  Node* node = ControlReducer::ReduceMerge(jsgraph, common, osr_loop);
+  Node* node = ControlReducer::ReduceMerge(jsgraph, osr_loop);
   if (node != osr_loop) osr_loop->ReplaceUses(node);
 
   // Run the normal control reduction, which naturally trims away the dead
   // parts of the graph.
-  ControlReducer::ReduceGraph(tmp_zone, jsgraph, common);
+  ControlReducer::ReduceGraph(tmp_zone, jsgraph);
 }
 
 

src/compiler/pipeline.cc

@@ -647,8 +647,7 @@ struct EarlyControlReductionPhase {
   void Run(PipelineData* data, Zone* temp_zone) {
     SourcePositionTable::Scope pos(data->source_positions(),
                                    SourcePosition::Unknown());
-    // TODO(turbofan): enable select matching in early control reduction.
-    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), data->common(), 0);
+    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), 0);
   }
 };
 
@@ -658,7 +657,7 @@ struct LateControlReductionPhase {
   void Run(PipelineData* data, Zone* temp_zone) {
     SourcePositionTable::Scope pos(data->source_positions(),
                                    SourcePosition::Unknown());
-    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), data->common(), 0);
+    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), 0);
   }
 };
 

test/cctest/compiler/test-control-reducer.cc

@@ -6,6 +6,7 @@
 #include "test/cctest/cctest.h"
 
 #include "src/base/bits.h"
+#include "src/compiler/all-nodes.h"
 #include "src/compiler/common-operator.h"
 #include "src/compiler/control-reducer.h"
 #include "src/compiler/graph.h"
@@ -142,20 +143,18 @@ class ControlReducerTester : HandleAndZoneScope {
 
   void Trim() { ControlReducer::TrimGraph(main_zone(), &jsgraph); }
 
-  void ReduceGraph() {
-    ControlReducer::ReduceGraph(main_zone(), &jsgraph, &common);
-  }
+  void ReduceGraph() { ControlReducer::ReduceGraph(main_zone(), &jsgraph); }
 
   // Checks one-step reduction of a phi.
   void ReducePhi(Node* expect, Node* phi) {
-    Node* result = ControlReducer::ReducePhiForTesting(&jsgraph, &common, phi);
+    Node* result = ControlReducer::ReducePhiForTesting(&jsgraph, phi);
     CHECK_EQ(expect, result);
     ReducePhiIterative(expect, phi);  // iterative should give the same result.
   }
 
   // Checks one-step reduction of a phi.
   void ReducePhiNonIterative(Node* expect, Node* phi) {
-    Node* result = ControlReducer::ReducePhiForTesting(&jsgraph, &common, phi);
+    Node* result = ControlReducer::ReducePhiForTesting(&jsgraph, phi);
     CHECK_EQ(expect, result);
   }
 
@@ -164,13 +163,13 @@ class ControlReducerTester : HandleAndZoneScope {
     Node* ret = graph.NewNode(common.Return(), phi, start, start);
     Node* end = graph.NewNode(common.End(), ret);
     graph.SetEnd(end);
-    ControlReducer::ReduceGraph(main_zone(), &jsgraph, &common);
+    ControlReducer::ReduceGraph(main_zone(), &jsgraph);
     CheckInputs(end, ret);
     CheckInputs(ret, expect, start, start);
   }
 
   void ReduceMerge(Node* expect, Node* merge) {
-    Node* result = ControlReducer::ReduceMerge(&jsgraph, &common, merge);
+    Node* result = ControlReducer::ReduceMerge(&jsgraph, merge);
     CHECK_EQ(expect, result);
   }
 
@@ -186,14 +185,12 @@ class ControlReducerTester : HandleAndZoneScope {
     Node* control = branch->InputAt(1);
     for (Node* use : branch->uses()) {
       if (use->opcode() == IrOpcode::kIfTrue) {
-        Node* result =
-            ControlReducer::ReduceIfNodeForTesting(&jsgraph, &common, use);
+        Node* result = ControlReducer::ReduceIfNodeForTesting(&jsgraph, use);
         if (expected == kTrue) CHECK_EQ(control, result);
         if (expected == kFalse) CHECK_EQ(IrOpcode::kDead, result->opcode());
         if (expected == kUnknown) CHECK_EQ(use, result);
       } else if (use->opcode() == IrOpcode::kIfFalse) {
-        Node* result =
-            ControlReducer::ReduceIfNodeForTesting(&jsgraph, &common, use);
+        Node* result = ControlReducer::ReduceIfNodeForTesting(&jsgraph, use);
         if (expected == kFalse) CHECK_EQ(control, result);
         if (expected == kTrue) CHECK_EQ(IrOpcode::kDead, result->opcode());
         if (expected == kUnknown) CHECK_EQ(use, result);
@@ -211,6 +208,109 @@ class ControlReducerTester : HandleAndZoneScope {
 };
 
 
+struct Branch {
+  Node* branch;
+  Node* if_true;
+  Node* if_false;
+
+  Branch(ControlReducerTester& R, Node* cond, Node* control = NULL) {
+    if (control == NULL) control = R.start;
+    branch = R.graph.NewNode(R.common.Branch(), cond, control);
+    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
+    if_false = R.graph.NewNode(R.common.IfFalse(), branch);
+  }
+};
+
+
+// TODO(titzer): use the diamonds from src/compiler/diamond.h here.
+struct Diamond {
+  Node* branch;
+  Node* if_true;
+  Node* if_false;
+  Node* merge;
+  Node* phi;
+
+  Diamond(ControlReducerTester& R, Node* cond) {
+    branch = R.graph.NewNode(R.common.Branch(), cond, R.start);
+    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
+    if_false = R.graph.NewNode(R.common.IfFalse(), branch);
+    merge = R.graph.NewNode(R.common.Merge(2), if_true, if_false);
+    phi = NULL;
+  }
+
+  Diamond(ControlReducerTester& R, Node* cond, Node* tv, Node* fv) {
+    branch = R.graph.NewNode(R.common.Branch(), cond, R.start);
+    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
+    if_false = R.graph.NewNode(R.common.IfFalse(), branch);
+    merge = R.graph.NewNode(R.common.Merge(2), if_true, if_false);
+    phi = R.graph.NewNode(R.common.Phi(kMachAnyTagged, 2), tv, fv, merge);
+  }
+
+  void chain(Diamond& that) { branch->ReplaceInput(1, that.merge); }
+
+  // Nest {this} into either the if_true or if_false branch of {that}.
+  void nest(Diamond& that, bool if_true) {
+    if (if_true) {
+      branch->ReplaceInput(1, that.if_true);
+      that.merge->ReplaceInput(0, merge);
+    } else {
+      branch->ReplaceInput(1, that.if_false);
+      that.merge->ReplaceInput(1, merge);
+    }
+  }
+};
+
+
+struct While {
+  Node* branch;
+  Node* if_true;
+  Node* exit;
+  Node* loop;
+
+  While(ControlReducerTester& R, Node* cond) {
+    loop = R.graph.NewNode(R.common.Loop(2), R.start, R.start);
+    branch = R.graph.NewNode(R.common.Branch(), cond, loop);
+    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
+    exit = R.graph.NewNode(R.common.IfFalse(), branch);
+    loop->ReplaceInput(1, if_true);
+  }
+
+  void chain(Node* control) { loop->ReplaceInput(0, control); }
+};
+
+
+// Helper for checking that nodes are *not* reachable from end.
+struct DeadChecker {
+  Zone zone;
+  AllNodes nodes;
+  explicit DeadChecker(Graph* graph) : zone(), nodes(&zone, graph) {}
+
+  void Check(Node* node) { CHECK(!nodes.IsLive(node)); }
+
+  void Check(Diamond& d) {
+    Check(d.branch);
+    Check(d.if_true);
+    Check(d.if_false);
+    Check(d.merge);
+    if (d.phi != NULL) Check(d.phi);
+  }
+
+  void CheckLive(Diamond& d, bool live_phi = true) {
+    CheckInputs(d.merge, d.if_true, d.if_false);
+    CheckInputs(d.if_true, d.branch);
+    CheckInputs(d.if_false, d.branch);
+    if (d.phi != NULL) {
+      if (live_phi) {
+        CHECK_EQ(3, d.phi->InputCount());
+        CHECK_EQ(d.merge, d.phi->InputAt(2));
+      } else {
+        Check(d.phi);
+      }
+    }
+  }
+};
+
+
 TEST(Trim1_live) {
   ControlReducerTester T;
   CHECK(IsUsedBy(T.start, T.p0));
@@ -897,7 +997,7 @@ TEST(CMergeReduce_exhaustive_4) {
       if (!selector.is_selected(i)) merge->ReplaceInput(i, R.dead);
     }
 
-    Node* result = ControlReducer::ReduceMerge(&R.jsgraph, &R.common, merge);
+    Node* result = ControlReducer::ReduceMerge(&R.jsgraph, merge);
 
     int count = selector.count;
     if (count == 0) {
@@ -993,77 +1093,6 @@ TEST(CMergeReduce_dead_chain2) {
 }
 
 
-struct Branch {
-  Node* branch;
-  Node* if_true;
-  Node* if_false;
-
-  Branch(ControlReducerTester& R, Node* cond, Node* control = NULL) {
-    if (control == NULL) control = R.start;
-    branch = R.graph.NewNode(R.common.Branch(), cond, control);
-    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
-    if_false = R.graph.NewNode(R.common.IfFalse(), branch);
-  }
-};
-
-
-// TODO(titzer): use the diamonds from src/compiler/diamond.h here.
-struct Diamond {
-  Node* branch;
-  Node* if_true;
-  Node* if_false;
-  Node* merge;
-  Node* phi;
-
-  Diamond(ControlReducerTester& R, Node* cond) {
-    branch = R.graph.NewNode(R.common.Branch(), cond, R.start);
-    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
-    if_false = R.graph.NewNode(R.common.IfFalse(), branch);
-    merge = R.graph.NewNode(R.common.Merge(2), if_true, if_false);
-    phi = NULL;
-  }
-
-  Diamond(ControlReducerTester& R, Node* cond, Node* tv, Node* fv) {
-    branch = R.graph.NewNode(R.common.Branch(), cond, R.start);
-    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
-    if_false = R.graph.NewNode(R.common.IfFalse(), branch);
-    merge = R.graph.NewNode(R.common.Merge(2), if_true, if_false);
-    phi = R.graph.NewNode(R.common.Phi(kMachAnyTagged, 2), tv, fv, merge);
-  }
-
-  void chain(Diamond& that) { branch->ReplaceInput(1, that.merge); }
-
-  // Nest {this} into either the if_true or if_false branch of {that}.
-  void nest(Diamond& that, bool if_true) {
-    if (if_true) {
-      branch->ReplaceInput(1, that.if_true);
-      that.merge->ReplaceInput(0, merge);
-    } else {
-      branch->ReplaceInput(1, that.if_false);
-      that.merge->ReplaceInput(1, merge);
-    }
-  }
-};
-
-
-struct While {
-  Node* branch;
-  Node* if_true;
-  Node* exit;
-  Node* loop;
-
-  While(ControlReducerTester& R, Node* cond) {
-    loop = R.graph.NewNode(R.common.Loop(2), R.start, R.start);
-    branch = R.graph.NewNode(R.common.Branch(), cond, loop);
-    if_true = R.graph.NewNode(R.common.IfTrue(), branch);
-    exit = R.graph.NewNode(R.common.IfFalse(), branch);
-    loop->ReplaceInput(1, if_true);
-  }
-
-  void chain(Node* control) { loop->ReplaceInput(0, control); }
-};
-
-
 TEST(CBranchReduce_none1) {
   ControlReducerTester R;
   Diamond d(R, R.p0);
@@ -1237,8 +1266,12 @@ TEST(CDeadLoop1) {
   loop->ReplaceInput(0, b.if_true);  // loop is not connected to start.
   Node* merge = R.graph.NewNode(R.common.Merge(2), R.start, b.if_false);
   R.ReduceMergeIterative(R.start, merge);
-  CHECK(b.if_true->IsDead());
-  CHECK(b.if_false->IsDead());
+
+  DeadChecker dead(&R.graph);
+  dead.Check(b.if_true);
+  dead.Check(b.if_false);
+  dead.Check(b.branch);
+  dead.Check(loop);
 }
 
 
@@ -1252,8 +1285,10 @@ TEST(CDeadLoop2) {
   d.merge->ReplaceInput(0, w.exit);
 
   R.ReduceMergeIterative(R.start, d.merge);
-  CHECK(d.if_true->IsDead());
-  CHECK(d.if_false->IsDead());
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d);
+  dead.Check(w.loop);
 }
 
 
@@ -1271,10 +1306,12 @@ TEST(Return2) {
   Diamond d(R, R.one);
   Node* ret = R.Return(R.half, R.start, d.merge);
   R.ReduceGraph();
-  CHECK(d.branch->IsDead());
-  CHECK(d.if_true->IsDead());
-  CHECK(d.if_false->IsDead());
-  CHECK(d.merge->IsDead());
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d.branch);
+  dead.Check(d.if_true);
+  dead.Check(d.if_false);
+  dead.Check(d.merge);
 
   CheckInputs(R.graph.end(), ret);
   CheckInputs(ret, R.half, R.start, R.start);
@@ -1286,11 +1323,13 @@ TEST(Return_true1) {
   Diamond d(R, R.one, R.half, R.zero);
   Node* ret = R.Return(d.phi, R.start, d.merge);
   R.ReduceGraph();
-  CHECK(d.branch->IsDead());
-  CHECK(d.if_true->IsDead());
-  CHECK(d.if_false->IsDead());
-  CHECK(d.merge->IsDead());
-  CHECK(d.phi->IsDead());
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d.branch);
+  dead.Check(d.if_true);
+  dead.Check(d.if_false);
+  dead.Check(d.merge);
+  dead.Check(d.phi);
 
   CheckInputs(R.graph.end(), ret);
   CheckInputs(ret, R.half, R.start, R.start);
@@ -1302,41 +1341,19 @@ TEST(Return_false1) {
   Diamond d(R, R.zero, R.one, R.half);
   Node* ret = R.Return(d.phi, R.start, d.merge);
   R.ReduceGraph();
-  CHECK(d.branch->IsDead());
-  CHECK(d.if_true->IsDead());
-  CHECK(d.if_false->IsDead());
-  CHECK(d.merge->IsDead());
-  CHECK(d.phi->IsDead());
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d.branch);
+  dead.Check(d.if_true);
+  dead.Check(d.if_false);
+  dead.Check(d.merge);
+  dead.Check(d.phi);
 
   CheckInputs(R.graph.end(), ret);
   CheckInputs(ret, R.half, R.start, R.start);
 }
 
 
-void CheckDeadDiamond(Diamond& d) {
-  CHECK(d.branch->IsDead());
-  CHECK(d.if_true->IsDead());
-  CHECK(d.if_false->IsDead());
-  CHECK(d.merge->IsDead());
-  if (d.phi != NULL) CHECK(d.phi->IsDead());
-}
-
-
-void CheckLiveDiamond(Diamond& d, bool live_phi = true) {
-  CheckInputs(d.merge, d.if_true, d.if_false);
-  CheckInputs(d.if_true, d.branch);
-  CheckInputs(d.if_false, d.branch);
-  if (d.phi != NULL) {
-    if (live_phi) {
-      CHECK_EQ(3, d.phi->InputCount());
-      CHECK_EQ(d.merge, d.phi->InputAt(2));
-    } else {
-      CHECK(d.phi->IsDead());
-    }
-  }
-}
-
-
 TEST(Return_effect1) {
   ControlReducerTester R;
   Diamond d(R, R.one);
@@ -1345,8 +1362,10 @@ TEST(Return_effect1) {
   Node* effect = R.graph.NewNode(R.common.EffectPhi(2), e1, e2, d.merge);
   Node* ret = R.Return(R.p0, effect, d.merge);
   R.ReduceGraph();
-  CheckDeadDiamond(d);
-  CHECK(effect->IsDead());
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d);
+  dead.Check(effect);
 
   CheckInputs(R.graph.end(), ret);
   CheckInputs(ret, R.p0, e1, R.start);
@@ -1355,9 +1374,9 @@ TEST(Return_effect1) {
 
 TEST(Return_nested_diamonds1) {
   ControlReducerTester R;
-  Diamond d1(R, R.p0, R.one, R.zero);
-  Diamond d2(R, R.p0);
-  Diamond d3(R, R.p0);
+  Diamond d2(R, R.p0, R.one, R.zero);
+  Diamond d3(R, R.p0, R.one, R.zero);
+  Diamond d1(R, R.p0, d2.phi, d3.phi);
 
   d2.nest(d1, true);
   d3.nest(d1, false);
@@ -1366,16 +1385,66 @@ TEST(Return_nested_diamonds1) {
 
   R.ReduceGraph();  // nothing should happen.
 
+  CheckInputs(ret, d1.phi, R.start, d1.merge);
+  CheckInputs(d1.phi, d2.phi, d3.phi, d1.merge);
+  CheckInputs(d1.merge, d2.merge, d3.merge);
+
+  DeadChecker dead(&R.graph);
+  dead.CheckLive(d2);
+  dead.CheckLive(d3);
+}
+
+
+TEST(Return_nested_diamonds1_dead1) {
+  ControlReducerTester R;
+  Diamond d2(R, R.p0);  // dead diamond
+  Diamond d3(R, R.p0, R.one, R.zero);
+  Diamond d1(R, R.p0, R.one, d3.phi);
+
+  d2.nest(d1, true);
+  d3.nest(d1, false);
+
+  Node* ret = R.Return(d1.phi, R.start, d1.merge);
+
+  R.ReduceGraph();
+
+  CheckInputs(ret, d1.phi, R.start, d1.merge);
+  CheckInputs(d1.phi, R.one, d3.phi, d1.merge);
+  CheckInputs(d1.merge, d1.if_true, d3.merge);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d2);  // d2 was a dead diamond.
+  dead.CheckLive(d3);
+}
+
+
+TEST(Return_nested_diamonds1_dead2) {
+  ControlReducerTester R;
+  Diamond d2(R, R.p0);  // dead diamond
+  Diamond d3(R, R.p0);  // dead diamond
+  Diamond d1(R, R.p0, R.one, R.zero);
+
+  d2.nest(d1, true);
+  d3.nest(d1, false);
+
+  Node* ret = R.Return(d1.phi, R.start, d1.merge);
+
+  R.ReduceGraph();
+
   CheckInputs(ret, d1.phi, R.start, d1.merge);
   CheckInputs(d1.phi, R.one, R.zero, d1.merge);
   CheckInputs(d1.merge, d1.if_true, d1.if_false);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d2);
+  dead.Check(d3);
 }
 
 
 TEST(Return_nested_diamonds_true1) {
   ControlReducerTester R;
-  Diamond d1(R, R.one, R.one, R.zero);
-  Diamond d2(R, R.p0);
+  Diamond d2(R, R.p0, R.one, R.zero);
+  Diamond d1(R, R.one, d2.phi, R.zero);
   Diamond d3(R, R.p0);
 
   d2.nest(d1, true);
@@ -1385,15 +1454,21 @@ TEST(Return_nested_diamonds_true1) {
 
   R.ReduceGraph();  // d1 gets folded true.
 
-  CheckInputs(ret, R.one, R.start, R.start);
+  CheckInputs(ret, d2.phi, R.start, d2.merge);
+  CheckInputs(d2.branch, R.p0, R.start);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.CheckLive(d2);
+  dead.Check(d3);
 }
 
 
 TEST(Return_nested_diamonds_false1) {
   ControlReducerTester R;
-  Diamond d1(R, R.zero, R.one, R.zero);
+  Diamond d3(R, R.p0, R.one, R.zero);
+  Diamond d1(R, R.zero, R.one, d3.phi);
   Diamond d2(R, R.p0);
-  Diamond d3(R, R.p0);
 
   d2.nest(d1, true);
   d3.nest(d1, false);
@@ -1402,7 +1477,13 @@ TEST(Return_nested_diamonds_false1) {
 
   R.ReduceGraph();  // d1 gets folded false.
 
-  CheckInputs(ret, R.zero, R.start, R.start);
+  CheckInputs(ret, d3.phi, R.start, d3.merge);
+  CheckInputs(d3.branch, R.p0, R.start);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.Check(d2);
+  dead.CheckLive(d3);
 }
 
 
@@ -1420,9 +1501,11 @@ TEST(Return_nested_diamonds_true_true1) {
   R.ReduceGraph();  // d1 and d2 both get folded true.
 
   CheckInputs(ret, R.one, R.start, R.start);
-  CheckDeadDiamond(d1);
-  CheckDeadDiamond(d2);
-  CheckDeadDiamond(d3);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.Check(d2);
+  dead.Check(d3);
 }
 
 
@@ -1440,9 +1523,11 @@ TEST(Return_nested_diamonds_true_false1) {
   R.ReduceGraph();  // d1 gets folded true and d2 gets folded false.
 
   CheckInputs(ret, R.one, R.start, R.start);
-  CheckDeadDiamond(d1);
-  CheckDeadDiamond(d2);
-  CheckDeadDiamond(d3);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.Check(d2);
+  dead.Check(d3);
 }
 
 
@@ -1467,8 +1552,10 @@ TEST(Return_nested_diamonds2) {
   CheckInputs(ret, d1.phi, R.start, d1.merge);
   CheckInputs(d1.phi, d2.phi, d3.phi, d1.merge);
   CheckInputs(d1.merge, d2.merge, d3.merge);
-  CheckLiveDiamond(d2);
-  CheckLiveDiamond(d3);
+
+  DeadChecker dead(&R.graph);
+  dead.CheckLive(d2);
+  dead.CheckLive(d3);
 }
 
 
@@ -1492,9 +1579,11 @@ TEST(Return_nested_diamonds_true2) {
 
   CheckInputs(ret, d2.phi, R.start, d2.merge);
   CheckInputs(d2.branch, R.p0, R.start);
-  CheckDeadDiamond(d1);
-  CheckLiveDiamond(d2);
-  CheckDeadDiamond(d3);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.CheckLive(d2);
+  dead.Check(d3);
 }
 
 
@@ -1517,9 +1606,11 @@ TEST(Return_nested_diamonds_true_true2) {
   R.ReduceGraph();  // d1 gets folded true.
 
   CheckInputs(ret, x2, R.start, R.start);
-  CheckDeadDiamond(d1);
-  CheckDeadDiamond(d2);
-  CheckDeadDiamond(d3);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.Check(d2);
+  dead.Check(d3);
 }
 
 
@@ -1542,7 +1633,9 @@ TEST(Return_nested_diamonds_true_false2) {
   R.ReduceGraph();  // d1 gets folded true.
 
   CheckInputs(ret, y2, R.start, R.start);
-  CheckDeadDiamond(d1);
-  CheckDeadDiamond(d2);
-  CheckDeadDiamond(d3);
+
+  DeadChecker dead(&R.graph);
+  dead.Check(d1);
+  dead.Check(d2);
+  dead.Check(d3);
 }

test/unittests/compiler/control-reducer-unittest.cc

@@ -41,8 +41,7 @@ class ControlReducerTest : public TypedGraphTest {
       os << "-- Graph before control reduction" << std::endl;
       os << AsRPO(*graph());
     }
-    ControlReducer::ReduceGraph(zone(), jsgraph(), common(),
-                                max_phis_for_select);
+    ControlReducer::ReduceGraph(zone(), jsgraph(), max_phis_for_select);
     if (FLAG_trace_turbo_graph) {
       OFStream os(stdout);
       os << "-- Graph after control reduction" << std::endl;

test/unittests/compiler/graph-reducer-unittest.cc

@@ -3,11 +3,10 @@
 // found in the LICENSE file.
 
 #include "src/compiler/graph.h"
-#include "src/compiler/graph-reducer.h"
 #include "src/compiler/node.h"
 #include "src/compiler/operator.h"
+#include "test/unittests/compiler/graph-reducer-unittest.h"
 #include "test/unittests/test-utils.h"
-#include "testing/gmock/include/gmock/gmock.h"
 
 using testing::_;
 using testing::DefaultValue;
@@ -55,9 +54,9 @@ struct MockReducer : public Reducer {
 
 
 // Replaces all "A" operators with "B" operators without creating new nodes.
-class InPlaceABReducer : public Reducer {
+class InPlaceABReducer final : public Reducer {
  public:
-  virtual Reduction Reduce(Node* node) {
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeA0:
         EXPECT_EQ(0, node->InputCount());
@@ -78,10 +77,11 @@ class InPlaceABReducer : public Reducer {
 
 
 // Replaces all "A" operators with "B" operators by allocating new nodes.
-class NewABReducer : public Reducer {
+class NewABReducer final : public Reducer {
  public:
   explicit NewABReducer(Graph* graph) : graph_(graph) {}
-  virtual Reduction Reduce(Node* node) {
+
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeA0:
         EXPECT_EQ(0, node->InputCount());
@@ -96,7 +96,9 @@ class NewABReducer : public Reducer {
     }
     return NoChange();
   }
-  Graph* graph_;
+
+ private:
+  Graph* const graph_;
 };
 
 
@@ -104,7 +106,8 @@ class NewABReducer : public Reducer {
 class A0Wrapper final : public Reducer {
  public:
   explicit A0Wrapper(Graph* graph) : graph_(graph) {}
-  virtual Reduction Reduce(Node* node) override {
+
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeA0:
         EXPECT_EQ(0, node->InputCount());
@@ -112,7 +115,9 @@ class A0Wrapper final : public Reducer {
     }
     return NoChange();
   }
-  Graph* graph_;
+
+ private:
+  Graph* const graph_;
 };
 
 
@@ -120,7 +125,8 @@ class A0Wrapper final : public Reducer {
 class B0Wrapper final : public Reducer {
  public:
   explicit B0Wrapper(Graph* graph) : graph_(graph) {}
-  virtual Reduction Reduce(Node* node) override {
+
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeB0:
         EXPECT_EQ(0, node->InputCount());
@@ -128,13 +134,16 @@ class B0Wrapper final : public Reducer {
     }
     return NoChange();
   }
-  Graph* graph_;
+
+ private:
+  Graph* const graph_;
 };
 
 
 // Replaces all "kOpA1" nodes with the first input.
-class A1Forwarder : public Reducer {
-  virtual Reduction Reduce(Node* node) {
+class A1Forwarder final : public Reducer {
+ public:
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeA1:
         EXPECT_EQ(1, node->InputCount());
@@ -146,8 +155,9 @@ class A1Forwarder : public Reducer {
 
 
 // Replaces all "kOpB1" nodes with the first input.
-class B1Forwarder : public Reducer {
-  virtual Reduction Reduce(Node* node) {
+class B1Forwarder final : public Reducer {
+ public:
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeB1:
         EXPECT_EQ(1, node->InputCount());
@@ -159,9 +169,9 @@ class B1Forwarder : public Reducer {
 
 
 // Replaces all "B" operators with "C" operators without creating new nodes.
-class InPlaceBCReducer : public Reducer {
+class InPlaceBCReducer final : public Reducer {
  public:
-  virtual Reduction Reduce(Node* node) {
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeB0:
         EXPECT_EQ(0, node->InputCount());
@@ -182,8 +192,9 @@ class InPlaceBCReducer : public Reducer {
 
 
 // Swaps the inputs to "kOp2A" and "kOp2B" nodes based on ids.
-class AB2Sorter : public Reducer {
-  virtual Reduction Reduce(Node* node) {
+class AB2Sorter final : public Reducer {
+ public:
+  Reduction Reduce(Node* node) final {
     switch (node->op()->opcode()) {
       case kOpcodeA2:
       case kOpcodeB2:
@@ -200,10 +211,59 @@ class AB2Sorter : public Reducer {
   }
 };
 
-
 }  // namespace
 
 
+class AdvancedReducerTest : public TestWithZone {
+ public:
+  AdvancedReducerTest() : graph_(zone()) {}
+
+ protected:
+  Graph* graph() { return &graph_; }
+
+ private:
+  Graph graph_;
+};
+
+
+TEST_F(AdvancedReducerTest, Replace) {
+  struct DummyReducer final : public AdvancedReducer {
+    explicit DummyReducer(Editor* editor) : AdvancedReducer(editor) {}
+    Reduction Reduce(Node* node) final {
+      Replace(node, node);
+      return NoChange();
+    }
+  };
+  StrictMock<MockAdvancedReducerEditor> e;
+  DummyReducer r(&e);
+  Node* node0 = graph()->NewNode(&kOpA0);
+  Node* node1 = graph()->NewNode(&kOpA1, node0);
+  EXPECT_CALL(e, Replace(node0, node0));
+  EXPECT_CALL(e, Replace(node1, node1));
+  EXPECT_FALSE(r.Reduce(node0).Changed());
+  EXPECT_FALSE(r.Reduce(node1).Changed());
+}
+
+
+TEST_F(AdvancedReducerTest, Revisit) {
+  struct DummyReducer final : public AdvancedReducer {
+    explicit DummyReducer(Editor* editor) : AdvancedReducer(editor) {}
+    Reduction Reduce(Node* node) final {
+      Revisit(node);
+      return NoChange();
+    }
+  };
+  StrictMock<MockAdvancedReducerEditor> e;
+  DummyReducer r(&e);
+  Node* node0 = graph()->NewNode(&kOpA0);
+  Node* node1 = graph()->NewNode(&kOpA1, node0);
+  EXPECT_CALL(e, Revisit(node0));
+  EXPECT_CALL(e, Revisit(node1));
+  EXPECT_FALSE(r.Reduce(node0).Changed());
+  EXPECT_FALSE(r.Reduce(node1).Changed());
+}
+
+
 class GraphReducerTest : public TestWithZone {
  public:
   GraphReducerTest() : graph_(zone()) {}
@@ -573,6 +633,8 @@ TEST_F(GraphReducerTest, Sorter1) {
 }
 
 
+namespace {
+
 // Generate a node graph with the given permutations.
 void GenDAG(Graph* graph, int* p3, int* p2, int* p1) {
   Node* level4 = graph->NewNode(&kOpA0);
@@ -591,6 +653,8 @@ void GenDAG(Graph* graph, int* p3, int* p2, int* p1) {
   graph->SetEnd(end);
 }
 
+}  // namespace
+
 
 TEST_F(GraphReducerTest, SortForwardReduce) {
   // Tests combined reductions on a series of DAGs.
@@ -667,7 +731,6 @@ TEST_F(GraphReducerTest, Order) {
   }
 }
 
-
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

test/unittests/compiler/graph-reducer-unittest.h

+// Copyright 2015 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_UNITTESTS_COMPILER_GRAPH_REDUCER_UNITTEST_H_
+#define V8_UNITTESTS_COMPILER_GRAPH_REDUCER_UNITTEST_H_
+
+#include "src/compiler/graph-reducer.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace v8 {
+namespace internal {
+namespace compiler {
+
+struct MockAdvancedReducerEditor : public AdvancedReducer::Editor {
+  MOCK_METHOD1(Revisit, void(Node*));
+  MOCK_METHOD2(Replace, void(Node*, Node*));
+};
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_UNITTESTS_COMPILER_GRAPH_REDUCER_UNITTEST_H_

test/unittests/unittests.gyp

@@ -47,6 +47,7 @@
         'compiler/control-reducer-unittest.cc',
         'compiler/diamond-unittest.cc',
         'compiler/graph-reducer-unittest.cc',
+        'compiler/graph-reducer-unittest.h',
         'compiler/graph-unittest.cc',
         'compiler/graph-unittest.h',
         'compiler/instruction-selector-unittest.cc',