[turbofan] Introduce BranchMatcher and DiamondMatcher helpers.

R=bmeurer@chromium.org
BUG=

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

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

diff --git a/src/compiler/common-operator-reducer.cc b/src/compiler/common-operator-reducer.cc
index b07383d21c5ed7b554273d24f98d933ab8808d98..c66cce0538794f8019fba27966bc3181b088113d 100644 (file)
--- a/src/compiler/common-operator-reducer.cc
+++ b/src/compiler/common-operator-reducer.cc
@@ -51,17 +51,10 @@ Reduction CommonOperatorReducer::ReducePhi(Node* node) {
     Node* vtrue = NodeProperties::GetValueInput(node, 0);
     Node* vfalse = NodeProperties::GetValueInput(node, 1);
     Node* merge = NodeProperties::GetControlInput(node);
-    Node* if_true = NodeProperties::GetControlInput(merge, 0);
-    Node* if_false = NodeProperties::GetControlInput(merge, 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* branch = if_true->InputAt(0);
-      Node* cond = branch->InputAt(0);
+    DiamondMatcher matcher(merge);
+    if (matcher.Matched()) {
+      if (matcher.IfTrue() == merge->InputAt(1)) std::swap(vtrue, vfalse);
+      Node* cond = matcher.Branch()->InputAt(0);
       if (cond->opcode() == IrOpcode::kFloat64LessThan) {
         if (cond->InputAt(0) == vtrue && cond->InputAt(1) == vfalse &&
             machine()->HasFloat64Min()) {

diff --git a/src/compiler/control-flow-optimizer.cc b/src/compiler/control-flow-optimizer.cc
index f074f72602af2b1995013c25bbff7c0073c11e64..c2198046e340c7968870a453aa11472082c4f450 100644 (file)
--- a/src/compiler/control-flow-optimizer.cc
+++ b/src/compiler/control-flow-optimizer.cc
@@ -123,13 +123,7 @@ bool ControlFlowOptimizer::TryCloneBranch(Node* node) {
     return false;
   }
   // Grab the IfTrue/IfFalse projections of the Branch.
-  Node* control_projections[2];
-  NodeProperties::CollectControlProjections(branch, control_projections,
-                                            arraysize(control_projections));
-  Node* if_true = control_projections[0];
-  Node* if_false = control_projections[1];
-  DCHECK_EQ(IrOpcode::kIfTrue, if_true->opcode());
-  DCHECK_EQ(IrOpcode::kIfFalse, if_false->opcode());
+  BranchMatcher matcher(branch);
   // Check/collect other Phi/EffectPhi nodes hanging off the Merge.
   NodeVector phis(zone());
   for (Node* const use : merge->uses()) {
@@ -150,7 +144,8 @@ bool ControlFlowOptimizer::TryCloneBranch(Node* node) {
       if (NodeProperties::IsPhi(edge.from())) {
         control = NodeProperties::GetControlInput(control, edge.index());
       }
-      if (control != if_true && control != if_false) return false;
+      if (control != matcher.IfTrue() && control != matcher.IfFalse())
+        return false;
     }
     phis.push_back(use);
   }
@@ -185,16 +180,16 @@ bool ControlFlowOptimizer::TryCloneBranch(Node* node) {
       if (NodeProperties::IsPhi(edge.from())) {
         control = NodeProperties::GetControlInput(control, edge.index());
       }
-      DCHECK(control == if_true || control == if_false);
-      edge.UpdateTo((control == if_true) ? phi_true : phi_false);
+      DCHECK(control == matcher.IfTrue() || control == matcher.IfFalse());
+      edge.UpdateTo((control == matcher.IfTrue()) ? phi_true : phi_false);
     }
     phi->Kill();
   }
   // Fix up IfTrue and IfFalse and kill all dead nodes.
-  if_false->ReplaceUses(merge_false);
-  if_true->ReplaceUses(merge_true);
-  if_false->Kill();
-  if_true->Kill();
+  matcher.IfFalse()->ReplaceUses(merge_false);
+  matcher.IfTrue()->ReplaceUses(merge_true);
+  matcher.IfFalse()->Kill();
+  matcher.IfTrue()->Kill();
   branch->Kill();
   cond->Kill();
   merge->Kill();
@@ -219,12 +214,11 @@ bool ControlFlowOptimizer::TryBuildSwitch(Node* node) {
   Node* if_false;
   Node* if_true;
   while (true) {
-    Node* control_projections[2];
-    NodeProperties::CollectControlProjections(branch, control_projections, 2);
-    if_true = control_projections[0];
-    if_false = control_projections[1];
-    DCHECK_EQ(IrOpcode::kIfTrue, if_true->opcode());
-    DCHECK_EQ(IrOpcode::kIfFalse, if_false->opcode());
+    BranchMatcher matcher(branch);
+    DCHECK(matcher.Matched());
+
+    if_true = matcher.IfTrue();
+    if_false = matcher.IfFalse();
 
     auto it = if_false->uses().begin();
     if (it == if_false->uses().end()) break;
@@ -256,8 +250,6 @@ bool ControlFlowOptimizer::TryBuildSwitch(Node* node) {
 
   DCHECK_EQ(IrOpcode::kBranch, node->opcode());
   DCHECK_EQ(IrOpcode::kBranch, branch->opcode());
-  DCHECK_EQ(IrOpcode::kIfTrue, if_true->opcode());
-  DCHECK_EQ(IrOpcode::kIfFalse, if_false->opcode());
   if (branch == node) {
     DCHECK_EQ(1u, values.size());
     return false;

diff --git a/src/compiler/control-reducer.cc b/src/compiler/control-reducer.cc
index 238b510671d7e79af7a128fae9626193a283790a..08182703b4bac886e4112ae517f17c822c738df4 100644 (file)
--- a/src/compiler/control-reducer.cc
+++ b/src/compiler/control-reducer.cc
@@ -566,25 +566,17 @@ class ControlReducerImpl {
 
     // Check if it's an unused diamond.
     if (live == 2 && phis.empty()) {
-      Node* node0 = node->InputAt(0);
-      Node* node1 = node->InputAt(1);
-      if (((node0->opcode() == IrOpcode::kIfTrue &&
-            node1->opcode() == IrOpcode::kIfFalse) ||
-           (node1->opcode() == IrOpcode::kIfTrue &&
-            node0->opcode() == IrOpcode::kIfFalse)) &&
-          node0->OwnedBy(node) && node1->OwnedBy(node)) {
-        Node* branch0 = NodeProperties::GetControlInput(node0);
-        Node* branch1 = NodeProperties::GetControlInput(node1);
-        if (branch0 == branch1) {
-          // It's a dead diamond, i.e. neither the IfTrue nor the IfFalse nodes
-          // have users except for the Merge and the Merge has no Phi or
-          // EffectPhi uses, so replace the Merge with the control input of the
-          // diamond.
-          TRACE("  DeadDiamond: #%d:%s #%d:%s #%d:%s\n", node0->id(),
-                node0->op()->mnemonic(), node1->id(), node1->op()->mnemonic(),
-                branch0->id(), branch0->op()->mnemonic());
-          return NodeProperties::GetControlInput(branch0);
-        }
+      DiamondMatcher matcher(node);
+      if (matcher.Matched() && matcher.IfProjectionsAreOwned()) {
+        // It's a dead diamond, i.e. neither the IfTrue nor the IfFalse nodes
+        // have uses except for the Merge and the Merge has no Phi or
+        // EffectPhi uses, so replace the Merge with the control input of the
+        // diamond.
+        TRACE("  DeadDiamond: #%d:Branch #%d:IfTrue #%d:IfFalse\n",
+              matcher.Branch()->id(), matcher.IfTrue()->id(),
+              matcher.IfFalse()->id());
+        // TODO(turbofan): replace single-phi diamonds with selects.
+        return NodeProperties::GetControlInput(matcher.Branch());
       }
     }
 

diff --git a/src/compiler/node-matchers.cc b/src/compiler/node-matchers.cc
index c6ae39000c025ae98f6f1b8a0ad67faf148c185e..1627b88219181892e638ec84457a9248e6972bf1 100644 (file)
--- a/src/compiler/node-matchers.cc
+++ b/src/compiler/node-matchers.cc
@@ -12,6 +12,49 @@ bool NodeMatcher::IsComparison() const {
   return IrOpcode::IsComparisonOpcode(opcode());
 }
 
+
+BranchMatcher::BranchMatcher(Node* branch)
+    : NodeMatcher(branch), if_true_(nullptr), if_false_(nullptr) {
+  if (branch->opcode() != IrOpcode::kBranch) return;
+  for (Node* use : branch->uses()) {
+    if (use->opcode() == IrOpcode::kIfTrue) {
+      DCHECK_NULL(if_true_);
+      if_true_ = use;
+    } else if (use->opcode() == IrOpcode::kIfFalse) {
+      DCHECK_NULL(if_false_);
+      if_false_ = use;
+    }
+  }
+}
+
+
+DiamondMatcher::DiamondMatcher(Node* merge)
+    : NodeMatcher(merge),
+      branch_(nullptr),
+      if_true_(nullptr),
+      if_false_(nullptr) {
+  if (merge->InputCount() != 2) return;
+  if (merge->opcode() != IrOpcode::kMerge) return;
+  Node* input0 = merge->InputAt(0);
+  if (input0->InputCount() != 1) return;
+  Node* input1 = merge->InputAt(1);
+  if (input1->InputCount() != 1) return;
+  Node* branch = input0->InputAt(0);
+  if (branch != input1->InputAt(0)) return;
+  if (branch->opcode() != IrOpcode::kBranch) return;
+  if (input0->opcode() == IrOpcode::kIfTrue &&
+      input1->opcode() == IrOpcode::kIfFalse) {
+    branch_ = branch;
+    if_true_ = input0;
+    if_false_ = input1;
+  } else if (input0->opcode() == IrOpcode::kIfFalse &&
+             input1->opcode() == IrOpcode::kIfTrue) {
+    branch_ = branch;
+    if_true_ = input1;
+    if_false_ = input0;
+  }
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

diff --git a/src/compiler/node-matchers.h b/src/compiler/node-matchers.h
index 1cdb6547f4e7d831c616d8ec5c337d64a25c7449..627829f4b56976a99e35a9ed23767995e1e821e9 100644 (file)
--- a/src/compiler/node-matchers.h
+++ b/src/compiler/node-matchers.h
@@ -542,6 +542,40 @@ typedef BaseWithIndexAndDisplacementMatcher<Int32AddMatcher>
 typedef BaseWithIndexAndDisplacementMatcher<Int64AddMatcher>
     BaseWithIndexAndDisplacement64Matcher;
 
+struct BranchMatcher : public NodeMatcher {
+  explicit BranchMatcher(Node* branch);
+
+  bool Matched() const { return if_true_ && if_false_; }
+
+  Node* Branch() const { return node(); }
+  Node* IfTrue() const { return if_true_; }
+  Node* IfFalse() const { return if_false_; }
+
+ private:
+  Node* if_true_;
+  Node* if_false_;
+};
+
+
+struct DiamondMatcher : public NodeMatcher {
+  explicit DiamondMatcher(Node* merge);
+
+  bool Matched() const { return branch_; }
+  bool IfProjectionsAreOwned() const {
+    return if_true_->OwnedBy(node()) && if_false_->OwnedBy(node());
+  }
+
+  Node* Branch() const { return branch_; }
+  Node* IfTrue() const { return if_true_; }
+  Node* IfFalse() const { return if_false_; }
+  Node* Merge() const { return node(); }
+
+ private:
+  Node* branch_;
+  Node* if_true_;
+  Node* if_false_;
+};
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

diff --git a/test/unittests/compiler/node-matchers-unittest.cc b/test/unittests/compiler/node-matchers-unittest.cc
index 85db9dbdca6f2a580c0ce72d719cad5d814677e3..a6239085dd75065a6072aa554f3eda0505b90500 100644 (file)
--- a/test/unittests/compiler/node-matchers-unittest.cc
+++ b/test/unittests/compiler/node-matchers-unittest.cc
@@ -728,6 +728,224 @@ TEST_F(NodeMatcherTest, ScaledWithOffset64Matcher) {
 }
 
 
+TEST_F(NodeMatcherTest, BranchMatcher_match) {
+  Node* zero = graph()->NewNode(common()->Int32Constant(0));
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    BranchMatcher matcher(branch);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    BranchMatcher matcher(branch);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* other = graph()->NewNode(common()->IfValue(33), branch);
+    BranchMatcher matcher(branch);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+    USE(other);
+  }
+}
+
+
+TEST_F(NodeMatcherTest, BranchMatcher_fail) {
+  Node* zero = graph()->NewNode(common()->Int32Constant(0));
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    BranchMatcher matcher(branch);
+    EXPECT_FALSE(matcher.Matched());
+    USE(if_true);
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    BranchMatcher matcher(branch);
+    EXPECT_FALSE(matcher.Matched());
+    USE(if_false);
+  }
+
+  {
+    BranchMatcher matcher(zero);
+    EXPECT_FALSE(matcher.Matched());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    EXPECT_TRUE(BranchMatcher(branch).Matched());
+    EXPECT_FALSE(BranchMatcher(if_true).Matched());
+    EXPECT_FALSE(BranchMatcher(if_false).Matched());
+  }
+
+  {
+    Node* sw = graph()->NewNode(common()->Switch(5), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), sw);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), sw);
+    EXPECT_FALSE(BranchMatcher(sw).Matched());
+    EXPECT_FALSE(BranchMatcher(if_true).Matched());
+    EXPECT_FALSE(BranchMatcher(if_false).Matched());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_value = graph()->NewNode(common()->IfValue(2), branch);
+    BranchMatcher matcher(branch);
+    EXPECT_FALSE(matcher.Matched());
+    EXPECT_FALSE(BranchMatcher(if_true).Matched());
+    EXPECT_FALSE(BranchMatcher(if_value).Matched());
+  }
+}
+
+
+TEST_F(NodeMatcherTest, DiamondMatcher_match) {
+  Node* zero = graph()->NewNode(common()->Int32Constant(0));
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    DiamondMatcher matcher(merge);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+    EXPECT_EQ(merge, matcher.Merge());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    DiamondMatcher matcher(merge);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+    EXPECT_EQ(merge, matcher.Merge());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_false, if_true);
+    DiamondMatcher matcher(merge);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+    EXPECT_EQ(merge, matcher.Merge());
+  }
+}
+
+
+TEST_F(NodeMatcherTest, DiamondMatcher_fail) {
+  Node* zero = graph()->NewNode(common()->Int32Constant(0));
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_value = graph()->NewNode(common()->IfValue(1), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_value);
+    DiamondMatcher matcher(merge);
+    EXPECT_FALSE(matcher.Matched());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* if_value = graph()->NewNode(common()->IfValue(1), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_false, if_value);
+    DiamondMatcher matcher(merge);
+    EXPECT_FALSE(matcher.Matched());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    DiamondMatcher matcher(merge);
+    EXPECT_TRUE(matcher.Matched());
+    EXPECT_EQ(branch, matcher.Branch());
+    EXPECT_EQ(if_true, matcher.IfTrue());
+    EXPECT_EQ(if_false, matcher.IfFalse());
+    EXPECT_EQ(merge, matcher.Merge());
+
+    EXPECT_FALSE(DiamondMatcher(branch).Matched());  // Must be the merge.
+    EXPECT_FALSE(DiamondMatcher(if_true).Matched());
+    EXPECT_FALSE(DiamondMatcher(if_false).Matched());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* merge = graph()->NewNode(common()->Merge(3), if_true, if_false,
+                                   graph()->start());
+    DiamondMatcher matcher(merge);
+    EXPECT_FALSE(matcher.Matched());  // Too many inputs to merge.
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* if_false = graph()->start();
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    DiamondMatcher matcher(merge);
+    EXPECT_FALSE(matcher.Matched());
+  }
+
+  {
+    Node* branch = graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->start();
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    DiamondMatcher matcher(merge);
+    EXPECT_FALSE(matcher.Matched());
+  }
+
+  {
+    Node* branch1 =
+        graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* branch2 =
+        graph()->NewNode(common()->Branch(), zero, graph()->start());
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch1);
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch2);
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    DiamondMatcher matcher(merge);
+    EXPECT_FALSE(matcher.Matched());
+  }
+}
+
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8