[turbofan] Improve interplay of ControlReducer and CommonOperatorReducer.

This turns the CommonOperatorReducer into an AdvancedReducer and makes
it independent of JSGraph (which was used only because it was convienent),
and let's the CommonOperatorReducer run together with the ControlReducer.

The ControlReducer is still not able to run together with other reducers,
but we're getting closer. The plan is to split the ControlReducer into
two parts: The dead code elimination part and the common operator
reduction part. This separation will help to avoid tricky bugs in the
future and should make testing a *lot* easier.

R=jarin@chromium.org

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

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

src/compiler/common-operator-reducer.cc

@@ -7,7 +7,8 @@
 #include <algorithm>
 
 #include "src/compiler/common-operator.h"
-#include "src/compiler/js-graph.h"
+#include "src/compiler/graph.h"
+#include "src/compiler/machine-operator.h"
 #include "src/compiler/node.h"
 #include "src/compiler/node-matchers.h"
 
@@ -32,43 +33,69 @@ Reduction CommonOperatorReducer::Reduce(Node* node) {
 
 Reduction CommonOperatorReducer::ReduceEffectPhi(Node* node) {
   DCHECK_EQ(IrOpcode::kEffectPhi, node->opcode());
-  int const input_count = node->InputCount();
-  if (input_count > 1) {
-    Node* const replacement = node->InputAt(0);
-    for (int i = 1; i < input_count - 1; ++i) {
-      if (node->InputAt(i) != replacement) return NoChange();
+  int const input_count = node->InputCount() - 1;
+  DCHECK_LE(1, input_count);
+  Node* const merge = node->InputAt(input_count);
+  DCHECK(IrOpcode::IsMergeOpcode(merge->opcode()));
+  DCHECK_EQ(input_count, merge->InputCount());
+  Node* const effect = node->InputAt(0);
+  DCHECK_NE(node, effect);
+  for (int i = 1; i < input_count; ++i) {
+    Node* const input = node->InputAt(i);
+    if (input == node) {
+      // Ignore redundant inputs.
+      DCHECK_EQ(IrOpcode::kLoop, merge->opcode());
+      continue;
     }
-    return Replace(replacement);
+    if (input != effect) return NoChange();
   }
-  return NoChange();
+  // We might now be able to further reduce the {merge} node.
+  Revisit(merge);
+  return Replace(effect);
 }
 
 
 Reduction CommonOperatorReducer::ReducePhi(Node* node) {
   DCHECK_EQ(IrOpcode::kPhi, node->opcode());
-  int const input_count = node->InputCount();
-  if (input_count == 3) {
-    Node* vtrue = NodeProperties::GetValueInput(node, 0);
-    Node* vfalse = NodeProperties::GetValueInput(node, 1);
-    Node* merge = NodeProperties::GetControlInput(node);
-    DiamondMatcher matcher(merge);
-    if (matcher.Matched()) {
-      if (matcher.IfTrue() == merge->InputAt(1)) std::swap(vtrue, vfalse);
-      Node* cond = matcher.Branch()->InputAt(0);
+  int const input_count = node->InputCount() - 1;
+  DCHECK_LE(1, input_count);
+  Node* const merge = node->InputAt(input_count);
+  DCHECK(IrOpcode::IsMergeOpcode(merge->opcode()));
+  DCHECK_EQ(input_count, merge->InputCount());
+  if (input_count == 2) {
+    Node* vtrue = node->InputAt(0);
+    Node* vfalse = node->InputAt(1);
+    Node* if_true = merge->InputAt(0);
+    Node* if_false = merge->InputAt(1);
+    if (if_true->opcode() != IrOpcode::kIfTrue) {
+      std::swap(if_true, if_false);
+      std::swap(vtrue, vfalse);
+    }
+    if (if_true->opcode() == IrOpcode::kIfTrue &&
+        if_false->opcode() == IrOpcode::kIfFalse &&
+        if_true->InputAt(0) == if_false->InputAt(0)) {
+      Node* const branch = if_true->InputAt(0);
+      Node* const cond = branch->InputAt(0);
       if (cond->opcode() == IrOpcode::kFloat32LessThan) {
         Float32BinopMatcher mcond(cond);
         if (mcond.left().Is(0.0) && mcond.right().Equals(vtrue) &&
             vfalse->opcode() == IrOpcode::kFloat32Sub) {
           Float32BinopMatcher mvfalse(vfalse);
           if (mvfalse.left().IsZero() && mvfalse.right().Equals(vtrue)) {
+            // We might now be able to further reduce the {merge} node.
+            Revisit(merge);
             return Change(node, machine()->Float32Abs(), vtrue);
           }
         }
         if (mcond.left().Equals(vtrue) && mcond.right().Equals(vfalse) &&
             machine()->HasFloat32Min()) {
+          // We might now be able to further reduce the {merge} node.
+          Revisit(merge);
           return Change(node, machine()->Float32Min(), vtrue, vfalse);
         } else if (mcond.left().Equals(vfalse) && mcond.right().Equals(vtrue) &&
                    machine()->HasFloat32Max()) {
+          // We might now be able to further reduce the {merge} node.
+          Revisit(merge);
           return Change(node, machine()->Float32Max(), vtrue, vfalse);
         }
       } else if (cond->opcode() == IrOpcode::kFloat64LessThan) {
@@ -77,35 +104,47 @@ Reduction CommonOperatorReducer::ReducePhi(Node* node) {
             vfalse->opcode() == IrOpcode::kFloat64Sub) {
           Float64BinopMatcher mvfalse(vfalse);
           if (mvfalse.left().IsZero() && mvfalse.right().Equals(vtrue)) {
+            // We might now be able to further reduce the {merge} node.
+            Revisit(merge);
             return Change(node, machine()->Float64Abs(), vtrue);
           }
         }
         if (mcond.left().Equals(vtrue) && mcond.right().Equals(vfalse) &&
             machine()->HasFloat64Min()) {
+          // We might now be able to further reduce the {merge} node.
+          Revisit(merge);
           return Change(node, machine()->Float64Min(), vtrue, vfalse);
         } else if (mcond.left().Equals(vfalse) && mcond.right().Equals(vtrue) &&
                    machine()->HasFloat64Max()) {
+          // We might now be able to further reduce the {merge} node.
+          Revisit(merge);
           return Change(node, machine()->Float64Max(), vtrue, vfalse);
         }
       }
     }
   }
-  if (input_count > 1) {
-    Node* const replacement = node->InputAt(0);
-    for (int i = 1; i < input_count - 1; ++i) {
-      if (node->InputAt(i) != replacement) return NoChange();
+  Node* const value = node->InputAt(0);
+  DCHECK_NE(node, value);
+  for (int i = 1; i < input_count; ++i) {
+    Node* const input = node->InputAt(i);
+    if (input == node) {
+      // Ignore redundant inputs.
+      DCHECK_EQ(IrOpcode::kLoop, merge->opcode());
+      continue;
     }
-    return Replace(replacement);
+    if (input != value) return NoChange();
   }
-  return NoChange();
+  // We might now be able to further reduce the {merge} node.
+  Revisit(merge);
+  return Replace(value);
 }
 
 
 Reduction CommonOperatorReducer::ReduceSelect(Node* node) {
   DCHECK_EQ(IrOpcode::kSelect, node->opcode());
-  Node* cond = NodeProperties::GetValueInput(node, 0);
-  Node* vtrue = NodeProperties::GetValueInput(node, 1);
-  Node* vfalse = NodeProperties::GetValueInput(node, 2);
+  Node* const cond = node->InputAt(0);
+  Node* const vtrue = node->InputAt(1);
+  Node* const vfalse = node->InputAt(2);
   if (vtrue == vfalse) return Replace(vtrue);
   switch (cond->opcode()) {
     case IrOpcode::kHeapConstant: {
@@ -173,19 +212,6 @@ Reduction CommonOperatorReducer::Change(Node* node, Operator const* op, Node* a,
   return Changed(node);
 }
 
-
-CommonOperatorBuilder* CommonOperatorReducer::common() const {
-  return jsgraph()->common();
-}
-
-
-Graph* CommonOperatorReducer::graph() const { return jsgraph()->graph(); }
-
-
-MachineOperatorBuilder* CommonOperatorReducer::machine() const {
-  return jsgraph()->machine();
-}
-
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/common-operator-reducer.h

@@ -14,15 +14,20 @@ namespace compiler {
 // Forward declarations.
 class CommonOperatorBuilder;
 class Graph;
-class JSGraph;
 class MachineOperatorBuilder;
 class Operator;
 
 
 // Performs strength reduction on nodes that have common operators.
-class CommonOperatorReducer final : public Reducer {
+class CommonOperatorReducer final : public AdvancedReducer {
  public:
-  explicit CommonOperatorReducer(JSGraph* jsgraph) : jsgraph_(jsgraph) {}
+  CommonOperatorReducer(Editor* editor, Graph* graph,
+                        CommonOperatorBuilder* common,
+                        MachineOperatorBuilder* machine)
+      : AdvancedReducer(editor),
+        graph_(graph),
+        common_(common),
+        machine_(machine) {}
   ~CommonOperatorReducer() final {}
 
   Reduction Reduce(Node* node) final;
@@ -35,12 +40,13 @@ class CommonOperatorReducer final : public Reducer {
   Reduction Change(Node* node, Operator const* op, Node* a);
   Reduction Change(Node* node, Operator const* op, Node* a, Node* b);
 
-  CommonOperatorBuilder* common() const;
-  Graph* graph() const;
-  JSGraph* jsgraph() const { return jsgraph_; }
-  MachineOperatorBuilder* machine() const;
+  Graph* graph() const { return graph_; }
+  CommonOperatorBuilder* common() const { return common_; }
+  MachineOperatorBuilder* machine() const { return machine_; }
 
-  JSGraph* const jsgraph_;
+  Graph* const graph_;
+  CommonOperatorBuilder* const common_;
+  MachineOperatorBuilder* const machine_;
 };
 
 }  // namespace compiler

src/compiler/control-reducer.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/compiler/common-operator.h"
+#include "src/compiler/common-operator-reducer.h"
 #include "src/compiler/control-reducer.h"
 #include "src/compiler/graph.h"
 #include "src/compiler/graph-reducer.h"
@@ -69,13 +70,6 @@ class ControlReducerImpl final : public AdvancedReducer {
       case IrOpcode::kMerge:
         result = ReduceMerge(node);
         break;
-      case IrOpcode::kSelect:
-        result = ReduceSelect(node);
-        break;
-      case IrOpcode::kPhi:
-      case IrOpcode::kEffectPhi:
-        result = ReducePhi(node);
-        break;
       case IrOpcode::kEnd:
         result = ReduceEnd(node);
         break;
@@ -104,38 +98,6 @@ class ControlReducerImpl final : public AdvancedReducer {
     return kUnknown;
   }
 
-  // Reduce redundant selects.
-  Node* ReduceSelect(Node* const node) {
-    Node* const tvalue = node->InputAt(1);
-    Node* const fvalue = node->InputAt(2);
-    if (tvalue == fvalue) return tvalue;
-    Decision result = DecideCondition(node->InputAt(0));
-    if (result == kTrue) return tvalue;
-    if (result == kFalse) return fvalue;
-    return node;
-  }
-
-  // Reduce redundant phis.
-  Node* ReducePhi(Node* node) {
-    int n = node->InputCount();
-    if (n <= 1) return dead();            // No non-control inputs.
-    if (n == 2) return node->InputAt(0);  // Only one non-control input.
-
-    Node* replacement = NULL;
-    auto const inputs = node->inputs();
-    for (auto it = inputs.begin(); n > 1; --n, ++it) {
-      Node* input = *it;
-      // Ignore dead inputs.
-      if (input->opcode() == IrOpcode::kDeadControl) continue;
-      // Non-redundant input.
-      if (input != node && input != replacement) {
-        if (replacement != NULL) return node;
-        replacement = input;
-      }
-    }
-    return replacement == NULL ? dead() : replacement;
-  }
-
   // Reduce branches.
   Node* ReduceBranch(Node* branch) {
     if (DecideCondition(branch->InputAt(0)) != kUnknown) {
@@ -312,9 +274,12 @@ class ControlReducerImpl final : public AdvancedReducer {
 void ControlReducer::ReduceGraph(Zone* zone, JSGraph* jsgraph,
                                  int max_phis_for_select) {
   GraphReducer graph_reducer(zone, jsgraph->graph());
+  CommonOperatorReducer common(&graph_reducer, jsgraph->graph(),
+                               jsgraph->common(), jsgraph->machine());
   ControlReducerImpl impl(&graph_reducer, zone, jsgraph);
   impl.max_phis_for_select_ = max_phis_for_select;
   graph_reducer.AddReducer(&impl);
+  graph_reducer.AddReducer(&common);
   graph_reducer.ReduceGraph();
 }
 
@@ -344,14 +309,6 @@ Node* ControlReducer::ReduceMerge(JSGraph* jsgraph, Node* node,
 }
 
 
-Node* ControlReducer::ReducePhiForTesting(JSGraph* jsgraph, Node* node) {
-  Zone zone;
-  DummyEditor editor;
-  ControlReducerImpl impl(&editor, &zone, jsgraph);
-  return impl.ReducePhi(node);
-}
-
-
 Node* ControlReducer::ReduceIfNodeForTesting(JSGraph* jsgraph, Node* node) {
   Zone zone;
   DummyEditor editor;

src/compiler/control-reducer.h

@@ -30,7 +30,6 @@ class ControlReducer {
                            int max_phis_for_select = 0);
 
   // Testing interface.
-  static Node* ReducePhiForTesting(JSGraph* graph, Node* node);
   static Node* ReduceIfNodeForTesting(JSGraph* graph, Node* node);
 };
 }

src/compiler/pipeline.cc

@@ -575,7 +575,8 @@ struct TypedLoweringPhase {
         data->info()->is_deoptimization_enabled()
             ? JSIntrinsicLowering::kDeoptimizationEnabled
             : JSIntrinsicLowering::kDeoptimizationDisabled);
-    CommonOperatorReducer common_reducer(data->jsgraph());
+    CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
+                                         data->common(), data->machine());
     AddReducer(data, &graph_reducer, &builtin_reducer);
     AddReducer(data, &graph_reducer, &typed_lowering);
     AddReducer(data, &graph_reducer, &intrinsic_lowering);
@@ -593,10 +594,11 @@ struct SimplifiedLoweringPhase {
     SimplifiedLowering lowering(data->jsgraph(), temp_zone,
                                 data->source_positions());
     lowering.LowerAllNodes();
+    JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
     ValueNumberingReducer vn_reducer(temp_zone);
     MachineOperatorReducer machine_reducer(data->jsgraph());
-    CommonOperatorReducer common_reducer(data->jsgraph());
-    JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
+    CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
+                                         data->common(), data->machine());
     AddReducer(data, &graph_reducer, &vn_reducer);
     AddReducer(data, &graph_reducer, &machine_reducer);
     AddReducer(data, &graph_reducer, &common_reducer);
@@ -620,11 +622,12 @@ struct ChangeLoweringPhase {
   static const char* phase_name() { return "change lowering"; }
 
   void Run(PipelineData* data, Zone* temp_zone) {
+    JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
     ValueNumberingReducer vn_reducer(temp_zone);
     ChangeLowering lowering(data->jsgraph());
     MachineOperatorReducer machine_reducer(data->jsgraph());
-    CommonOperatorReducer common_reducer(data->jsgraph());
-    JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
+    CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
+                                         data->common(), data->machine());
     AddReducer(data, &graph_reducer, &vn_reducer);
     AddReducer(data, &graph_reducer, &lowering);
     AddReducer(data, &graph_reducer, &machine_reducer);

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

@@ -143,29 +143,6 @@ class ControlReducerTester : HandleAndZoneScope {
 
   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, 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, phi);
-    CHECK_EQ(expect, result);
-  }
-
-  void ReducePhiIterative(Node* expect, Node* phi) {
-    p0->ReplaceInput(0, start);  // hack: parameters may be trimmed.
-    Node* ret = graph.NewNode(common.Return(), phi, start, start);
-    Node* end = graph.NewNode(common.End(1), ret);
-    graph.SetEnd(end);
-    ControlReducer::ReduceGraph(main_zone(), &jsgraph);
-    CheckInputs(end, ret);
-    CheckInputs(ret, expect, start, start);
-  }
-
   void ReduceMerge(Node* expect, Node* merge) {
     Node* result = ControlReducer::ReduceMerge(&jsgraph, merge);
     CHECK_EQ(expect, result);
@@ -313,271 +290,6 @@ struct DeadChecker {
 };
 
 
-TEST(CReducePhi1) {
-  ControlReducerTester R;
-
-  R.ReducePhi(R.leaf[0], R.Phi(R.leaf[0]));
-  R.ReducePhi(R.leaf[1], R.Phi(R.leaf[1]));
-  R.ReducePhi(R.leaf[2], R.Phi(R.leaf[2]));
-  R.ReducePhi(R.leaf[3], R.Phi(R.leaf[3]));
-}
-
-
-TEST(CReducePhi1_dead) {
-  ControlReducerTester R;
-
-  R.ReducePhi(R.leaf[0], R.Phi(R.leaf[0], R.dead));
-  R.ReducePhi(R.leaf[1], R.Phi(R.leaf[1], R.dead));
-  R.ReducePhi(R.leaf[2], R.Phi(R.leaf[2], R.dead));
-  R.ReducePhi(R.leaf[3], R.Phi(R.leaf[3], R.dead));
-
-  R.ReducePhi(R.leaf[0], R.Phi(R.dead, R.leaf[0]));
-  R.ReducePhi(R.leaf[1], R.Phi(R.dead, R.leaf[1]));
-  R.ReducePhi(R.leaf[2], R.Phi(R.dead, R.leaf[2]));
-  R.ReducePhi(R.leaf[3], R.Phi(R.dead, R.leaf[3]));
-}
-
-
-TEST(CReducePhi1_dead2) {
-  ControlReducerTester R;
-
-  R.ReducePhi(R.leaf[0], R.Phi(R.leaf[0], R.dead, R.dead));
-  R.ReducePhi(R.leaf[0], R.Phi(R.dead, R.leaf[0], R.dead));
-  R.ReducePhi(R.leaf[0], R.Phi(R.dead, R.dead, R.leaf[0]));
-}
-
-
-TEST(CReducePhi2a) {
-  ControlReducerTester R;
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(a, a));
-  }
-}
-
-
-TEST(CReducePhi2b) {
-  ControlReducerTester R;
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(R.self, a));
-    R.ReducePhi(a, R.Phi(a, R.self));
-  }
-}
-
-
-TEST(CReducePhi2c) {
-  ControlReducerTester R;
-
-  for (size_t i = 1; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i], *b = R.leaf[0];
-    Node* phi1 = R.Phi(b, a);
-    R.ReducePhi(phi1, phi1);
-
-    Node* phi2 = R.Phi(a, b);
-    R.ReducePhi(phi2, phi2);
-  }
-}
-
-
-TEST(CReducePhi2_dead) {
-  ControlReducerTester R;
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(a, a, R.dead));
-    R.ReducePhi(a, R.Phi(a, R.dead, a));
-    R.ReducePhi(a, R.Phi(R.dead, a, a));
-  }
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(R.self, a));
-    R.ReducePhi(a, R.Phi(a, R.self));
-    R.ReducePhi(a, R.Phi(R.self, a, R.dead));
-    R.ReducePhi(a, R.Phi(a, R.self, R.dead));
-  }
-
-  for (size_t i = 1; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i], *b = R.leaf[0];
-    Node* phi1 = R.Phi(b, a, R.dead);
-    R.ReducePhiNonIterative(phi1, phi1);
-
-    Node* phi2 = R.Phi(a, b, R.dead);
-    R.ReducePhiNonIterative(phi2, phi2);
-  }
-}
-
-
-TEST(CReducePhi3) {
-  ControlReducerTester R;
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(a, a, a));
-  }
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(R.self, a, a));
-    R.ReducePhi(a, R.Phi(a, R.self, a));
-    R.ReducePhi(a, R.Phi(a, a, R.self));
-  }
-
-  for (size_t i = 1; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i], *b = R.leaf[0];
-    Node* phi1 = R.Phi(b, a, a);
-    R.ReducePhi(phi1, phi1);
-
-    Node* phi2 = R.Phi(a, b, a);
-    R.ReducePhi(phi2, phi2);
-
-    Node* phi3 = R.Phi(a, a, b);
-    R.ReducePhi(phi3, phi3);
-  }
-}
-
-
-TEST(CReducePhi4) {
-  ControlReducerTester R;
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(a, a, a, a));
-  }
-
-  for (size_t i = 0; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i];
-    R.ReducePhi(a, R.Phi(R.self, a, a, a));
-    R.ReducePhi(a, R.Phi(a, R.self, a, a));
-    R.ReducePhi(a, R.Phi(a, a, R.self, a));
-    R.ReducePhi(a, R.Phi(a, a, a, R.self));
-
-    R.ReducePhi(a, R.Phi(R.self, R.self, a, a));
-    R.ReducePhi(a, R.Phi(a, R.self, R.self, a));
-    R.ReducePhi(a, R.Phi(a, a, R.self, R.self));
-    R.ReducePhi(a, R.Phi(R.self, a, a, R.self));
-  }
-
-  for (size_t i = 1; i < kNumLeafs; i++) {
-    Node* a = R.leaf[i], *b = R.leaf[0];
-    Node* phi1 = R.Phi(b, a, a, a);
-    R.ReducePhi(phi1, phi1);
-
-    Node* phi2 = R.Phi(a, b, a, a);
-    R.ReducePhi(phi2, phi2);
-
-    Node* phi3 = R.Phi(a, a, b, a);
-    R.ReducePhi(phi3, phi3);
-
-    Node* phi4 = R.Phi(a, a, a, b);
-    R.ReducePhi(phi4, phi4);
-  }
-}
-
-
-TEST(CReducePhi_iterative1) {
-  ControlReducerTester R;
-
-  R.ReducePhiIterative(R.leaf[0], R.Phi(R.leaf[0], R.Phi(R.leaf[0])));
-  R.ReducePhiIterative(R.leaf[0], R.Phi(R.Phi(R.leaf[0]), R.leaf[0]));
-}
-
-
-TEST(CReducePhi_iterative2) {
-  ControlReducerTester R;
-
-  R.ReducePhiIterative(R.leaf[0], R.Phi(R.Phi(R.leaf[0]), R.Phi(R.leaf[0])));
-}
-
-
-TEST(CReducePhi_iterative3) {
-  ControlReducerTester R;
-
-  R.ReducePhiIterative(R.leaf[0],
-                       R.Phi(R.leaf[0], R.Phi(R.leaf[0], R.leaf[0])));
-  R.ReducePhiIterative(R.leaf[0],
-                       R.Phi(R.Phi(R.leaf[0], R.leaf[0]), R.leaf[0]));
-}
-
-
-TEST(CReducePhi_iterative4) {
-  ControlReducerTester R;
-
-  R.ReducePhiIterative(R.leaf[0], R.Phi(R.Phi(R.leaf[0], R.leaf[0]),
-                                        R.Phi(R.leaf[0], R.leaf[0])));
-
-  Node* p1 = R.Phi(R.leaf[0], R.leaf[0]);
-  R.ReducePhiIterative(R.leaf[0], R.Phi(p1, p1));
-
-  Node* p2 = R.Phi(R.leaf[0], R.leaf[0], R.leaf[0]);
-  R.ReducePhiIterative(R.leaf[0], R.Phi(p2, p2, p2));
-
-  Node* p3 = R.Phi(R.leaf[0], R.leaf[0], R.leaf[0]);
-  R.ReducePhiIterative(R.leaf[0], R.Phi(p3, p3, R.leaf[0]));
-}
-
-
-TEST(CReducePhi_iterative_self1) {
-  ControlReducerTester R;
-
-  R.ReducePhiIterative(R.leaf[0], R.Phi(R.leaf[0], R.Phi(R.leaf[0], R.self)));
-  R.ReducePhiIterative(R.leaf[0], R.Phi(R.Phi(R.leaf[0], R.self), R.leaf[0]));
-}
-
-
-TEST(CReducePhi_iterative_self2) {
-  ControlReducerTester R;
-
-  R.ReducePhiIterative(
-      R.leaf[0], R.Phi(R.Phi(R.leaf[0], R.self), R.Phi(R.leaf[0], R.self)));
-  R.ReducePhiIterative(
-      R.leaf[0], R.Phi(R.Phi(R.self, R.leaf[0]), R.Phi(R.self, R.leaf[0])));
-
-  Node* p1 = R.Phi(R.leaf[0], R.self);
-  R.ReducePhiIterative(R.leaf[0], R.Phi(p1, p1));
-
-  Node* p2 = R.Phi(R.self, R.leaf[0]);
-  R.ReducePhiIterative(R.leaf[0], R.Phi(p2, p2));
-}
-
-
-TEST(EReducePhi1) {
-  ControlReducerTester R;
-
-  R.ReducePhi(R.leaf[0], R.EffectPhi(R.leaf[0]));
-  R.ReducePhi(R.leaf[1], R.EffectPhi(R.leaf[1]));
-  R.ReducePhi(R.leaf[2], R.EffectPhi(R.leaf[2]));
-  R.ReducePhi(R.leaf[3], R.EffectPhi(R.leaf[3]));
-}
-
-
-TEST(EReducePhi1_dead) {
-  ControlReducerTester R;
-
-  R.ReducePhi(R.leaf[0], R.EffectPhi(R.leaf[0], R.dead));
-  R.ReducePhi(R.leaf[1], R.EffectPhi(R.leaf[1], R.dead));
-  R.ReducePhi(R.leaf[2], R.EffectPhi(R.leaf[2], R.dead));
-  R.ReducePhi(R.leaf[3], R.EffectPhi(R.leaf[3], R.dead));
-
-  R.ReducePhi(R.leaf[0], R.EffectPhi(R.dead, R.leaf[0]));
-  R.ReducePhi(R.leaf[1], R.EffectPhi(R.dead, R.leaf[1]));
-  R.ReducePhi(R.leaf[2], R.EffectPhi(R.dead, R.leaf[2]));
-  R.ReducePhi(R.leaf[3], R.EffectPhi(R.dead, R.leaf[3]));
-}
-
-
-TEST(EReducePhi1_dead2) {
-  ControlReducerTester R;
-
-  R.ReducePhi(R.leaf[0], R.EffectPhi(R.leaf[0], R.dead, R.dead));
-  R.ReducePhi(R.leaf[0], R.EffectPhi(R.dead, R.leaf[0], R.dead));
-  R.ReducePhi(R.leaf[0], R.EffectPhi(R.dead, R.dead, R.leaf[0]));
-}
-
-
 TEST(CMergeReduce_simple1) {
   ControlReducerTester R;
 
@@ -982,17 +694,6 @@ TEST(CChainedDiamondsReduce_phi1) {
 }
 
 
-TEST(CChainedDiamondsReduce_phi2) {
-  ControlReducerTester R;
-  Diamond d1(R, R.p0, R.jsgraph.TrueConstant(),
-             R.jsgraph.TrueConstant());  // redundant phi.
-  Diamond d2(R, d1.phi);
-  d2.chain(d1);
-
-  R.ReduceMergeIterative(d1.merge, d2.merge);
-}
-
-
 TEST(CNestedDiamondsReduce_true_true_false) {
   ControlReducerTester R;
   Diamond d1(R, R.jsgraph.TrueConstant());

test/unittests/compiler/common-operator-reducer-unittest.cc

@@ -4,14 +4,16 @@
 
 #include "src/compiler/common-operator.h"
 #include "src/compiler/common-operator-reducer.h"
-#include "src/compiler/js-graph.h"
 #include "src/compiler/js-operator.h"
 #include "src/compiler/machine-operator.h"
 #include "src/compiler/machine-type.h"
 #include "src/compiler/operator.h"
+#include "test/unittests/compiler/graph-reducer-unittest.h"
 #include "test/unittests/compiler/graph-unittest.h"
 #include "test/unittests/compiler/node-test-utils.h"
 
+using testing::StrictMock;
+
 namespace v8 {
 namespace internal {
 namespace compiler {
@@ -23,15 +25,21 @@ class CommonOperatorReducerTest : public GraphTest {
   ~CommonOperatorReducerTest() override {}
 
  protected:
-  Reduction Reduce(Node* node, MachineOperatorBuilder::Flags flags =
-                                   MachineOperatorBuilder::kNoFlags) {
+  Reduction Reduce(
+      AdvancedReducer::Editor* editor, Node* node,
+      MachineOperatorBuilder::Flags flags = MachineOperatorBuilder::kNoFlags) {
     JSOperatorBuilder javascript(zone());
     MachineOperatorBuilder machine(zone(), kMachPtr, flags);
-    JSGraph jsgraph(isolate(), graph(), common(), &javascript, &machine);
-    CommonOperatorReducer reducer(&jsgraph);
+    CommonOperatorReducer reducer(editor, graph(), common(), &machine);
     return reducer.Reduce(node);
   }
 
+  Reduction Reduce(Node* node, MachineOperatorBuilder::Flags flags =
+                                   MachineOperatorBuilder::kNoFlags) {
+    StrictMock<MockAdvancedReducerEditor> editor;
+    return Reduce(&editor, node, flags);
+  }
+
   MachineOperatorBuilder* machine() { return &machine_; }
 
  private:
@@ -61,29 +69,52 @@ const Operator kOp0(0, Operator::kNoProperties, "Op0", 0, 0, 0, 1, 1, 0);
 // EffectPhi
 
 
-TEST_F(CommonOperatorReducerTest, RedundantEffectPhi) {
+TEST_F(CommonOperatorReducerTest, EffectPhiWithMerge) {
   const int kMaxInputs = 64;
   Node* inputs[kMaxInputs];
   Node* const input = graph()->NewNode(&kOp0);
   TRACED_FORRANGE(int, input_count, 2, kMaxInputs - 1) {
     int const value_input_count = input_count - 1;
+    for (int i = 0; i < value_input_count; ++i) {
+      inputs[i] = graph()->start();
+    }
+    Node* const merge = graph()->NewNode(common()->Merge(value_input_count),
+                                         value_input_count, inputs);
     for (int i = 0; i < value_input_count; ++i) {
       inputs[i] = input;
     }
-    inputs[value_input_count] = graph()->start();
-    Reduction r = Reduce(graph()->NewNode(
-        common()->EffectPhi(value_input_count), input_count, inputs));
+    inputs[value_input_count] = merge;
+    StrictMock<MockAdvancedReducerEditor> editor;
+    EXPECT_CALL(editor, Revisit(merge));
+    Reduction r =
+        Reduce(&editor, graph()->NewNode(common()->EffectPhi(value_input_count),
+                                         input_count, inputs));
     ASSERT_TRUE(r.Changed());
     EXPECT_EQ(input, r.replacement());
   }
 }
 
 
+TEST_F(CommonOperatorReducerTest, EffectPhiWithLoop) {
+  Node* const e0 = graph()->NewNode(&kOp0);
+  Node* const loop =
+      graph()->NewNode(common()->Loop(2), graph()->start(), graph()->start());
+  loop->ReplaceInput(1, loop);
+  Node* const ephi = graph()->NewNode(common()->EffectPhi(2), e0, e0, loop);
+  ephi->ReplaceInput(1, ephi);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(loop));
+  Reduction const r = Reduce(&editor, ephi);
+  ASSERT_TRUE(r.Changed());
+  EXPECT_EQ(e0, r.replacement());
+}
+
+
 // -----------------------------------------------------------------------------
 // Phi
 
 
-TEST_F(CommonOperatorReducerTest, RedundantPhi) {
+TEST_F(CommonOperatorReducerTest, PhiWithMerge) {
   const int kMaxInputs = 64;
   Node* inputs[kMaxInputs];
   Node* const input = graph()->NewNode(&kOp0);
@@ -93,14 +124,17 @@ TEST_F(CommonOperatorReducerTest, RedundantPhi) {
       for (int i = 0; i < value_input_count; ++i) {
         inputs[i] = graph()->start();
       }
-      Node* merge = graph()->NewNode(common()->Merge(value_input_count),
-                                     value_input_count, inputs);
+      Node* const merge = graph()->NewNode(common()->Merge(value_input_count),
+                                           value_input_count, inputs);
       for (int i = 0; i < value_input_count; ++i) {
         inputs[i] = input;
       }
       inputs[value_input_count] = merge;
-      Reduction r = Reduce(graph()->NewNode(
-          common()->Phi(type, value_input_count), input_count, inputs));
+      StrictMock<MockAdvancedReducerEditor> editor;
+      EXPECT_CALL(editor, Revisit(merge));
+      Reduction r = Reduce(
+          &editor, graph()->NewNode(common()->Phi(type, value_input_count),
+                                    input_count, inputs));
       ASSERT_TRUE(r.Changed());
       EXPECT_EQ(input, r.replacement());
     }
@@ -108,6 +142,22 @@ TEST_F(CommonOperatorReducerTest, RedundantPhi) {
 }
 
 
+TEST_F(CommonOperatorReducerTest, PhiWithLoop) {
+  Node* const p0 = Parameter(0);
+  Node* const loop =
+      graph()->NewNode(common()->Loop(2), graph()->start(), graph()->start());
+  loop->ReplaceInput(1, loop);
+  Node* const phi =
+      graph()->NewNode(common()->Phi(kMachAnyTagged, 2), p0, p0, loop);
+  phi->ReplaceInput(1, phi);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(loop));
+  Reduction const r = Reduce(&editor, phi);
+  ASSERT_TRUE(r.Changed());
+  EXPECT_EQ(p0, r.replacement());
+}
+
+
 TEST_F(CommonOperatorReducerTest, PhiToFloat32Abs) {
   Node* p0 = Parameter(0);
   Node* c0 = Float32Constant(0.0);
@@ -120,7 +170,9 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat32Abs) {
   Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
   Node* phi =
       graph()->NewNode(common()->Phi(kMachFloat32, 2), vtrue, vfalse, merge);
-  Reduction r = Reduce(phi);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(merge));
+  Reduction r = Reduce(&editor, phi);
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFloat32Abs(p0));
 }
@@ -138,7 +190,9 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat64Abs) {
   Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
   Node* phi =
       graph()->NewNode(common()->Phi(kMachFloat64, 2), vtrue, vfalse, merge);
-  Reduction r = Reduce(phi);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(merge));
+  Reduction r = Reduce(&editor, phi);
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFloat64Abs(p0));
 }
@@ -153,7 +207,9 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat32Max) {
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
   Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
   Node* phi = graph()->NewNode(common()->Phi(kMachFloat32, 2), p1, p0, merge);
-  Reduction r = Reduce(phi, MachineOperatorBuilder::kFloat32Max);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(merge));
+  Reduction r = Reduce(&editor, phi, MachineOperatorBuilder::kFloat32Max);
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFloat32Max(p1, p0));
 }
@@ -168,7 +224,9 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat64Max) {
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
   Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
   Node* phi = graph()->NewNode(common()->Phi(kMachFloat64, 2), p1, p0, merge);
-  Reduction r = Reduce(phi, MachineOperatorBuilder::kFloat64Max);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(merge));
+  Reduction r = Reduce(&editor, phi, MachineOperatorBuilder::kFloat64Max);
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFloat64Max(p1, p0));
 }
@@ -183,7 +241,9 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat32Min) {
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
   Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
   Node* phi = graph()->NewNode(common()->Phi(kMachFloat32, 2), p0, p1, merge);
-  Reduction r = Reduce(phi, MachineOperatorBuilder::kFloat32Min);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(merge));
+  Reduction r = Reduce(&editor, phi, MachineOperatorBuilder::kFloat32Min);
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFloat32Min(p0, p1));
 }
@@ -198,7 +258,9 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat64Min) {
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
   Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
   Node* phi = graph()->NewNode(common()->Phi(kMachFloat64, 2), p0, p1, merge);
-  Reduction r = Reduce(phi, MachineOperatorBuilder::kFloat64Min);
+  StrictMock<MockAdvancedReducerEditor> editor;
+  EXPECT_CALL(editor, Revisit(merge));
+  Reduction r = Reduce(&editor, phi, MachineOperatorBuilder::kFloat64Min);
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFloat64Min(p0, p1));
 }
@@ -208,7 +270,7 @@ TEST_F(CommonOperatorReducerTest, PhiToFloat64Min) {
 // Select
 
 
-TEST_F(CommonOperatorReducerTest, RedundantSelect) {
+TEST_F(CommonOperatorReducerTest, SelectWithSameThenAndElse) {
   Node* const input = graph()->NewNode(&kOp0);
   TRACED_FOREACH(BranchHint, hint, kBranchHints) {
     TRACED_FOREACH(MachineType, type, kMachineTypes) {