Introduce and test NodeProperties::CollectControlProjections.

R=bmeurer@chromium.org
TEST=unittests/NodePropertiesTest.CollectControlProjections

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

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

diff --git a/src/compiler/control-flow-optimizer.cc b/src/compiler/control-flow-optimizer.cc
index c75d09f..1a2b4cd 100644 (file)
--- a/src/compiler/control-flow-optimizer.cc
+++ b/src/compiler/control-flow-optimizer.cc
@@ -68,19 +68,14 @@ void ControlFlowOptimizer::VisitBranch(Node* node) {
   Node* if_false;
   Node* if_true;
   while (true) {
-    // TODO(turbofan): use NodeProperties::CollectSuccessorProjections() here
-    // once available.
-    auto it = branch->uses().begin();
-    DCHECK(it != branch->uses().end());
-    if_true = *it++;
-    DCHECK(it != branch->uses().end());
-    if_false = *it++;
-    DCHECK(it == branch->uses().end());
-    if (if_true->opcode() != IrOpcode::kIfTrue) std::swap(if_true, if_false);
+    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());
 
-    it = if_false->uses().begin();
+    auto it = if_false->uses().begin();
     if (it == if_false->uses().end()) break;
     Node* branch1 = *it++;
     if (branch1->opcode() != IrOpcode::kBranch) break;

diff --git a/src/compiler/node-properties.cc b/src/compiler/node-properties.cc
index 1575310..47de74e 100644 (file)
--- a/src/compiler/node-properties.cc
+++ b/src/compiler/node-properties.cc
@@ -182,6 +182,50 @@ Node* NodeProperties::FindProjection(Node* node, size_t projection_index) {
 
 
 // static
+void NodeProperties::CollectControlProjections(Node* node, Node** projections,
+                                               size_t projection_count) {
+#ifdef DEBUG
+  DCHECK_EQ(static_cast<int>(projection_count), node->UseCount());
+  std::memset(projections, 0, sizeof(*projections) * projection_count);
+#endif
+  size_t if_value_index = 0;
+  for (Node* const use : node->uses()) {
+    size_t index;
+    switch (use->opcode()) {
+      default:
+        UNREACHABLE();
+      // Fall through.
+      case IrOpcode::kIfTrue:
+        DCHECK_EQ(IrOpcode::kBranch, node->opcode());
+        index = 0;
+        break;
+      case IrOpcode::kIfFalse:
+        DCHECK_EQ(IrOpcode::kBranch, node->opcode());
+        index = 1;
+        break;
+      case IrOpcode::kIfValue:
+        DCHECK_EQ(IrOpcode::kSwitch, node->opcode());
+        index = if_value_index++;
+        break;
+      case IrOpcode::kIfDefault:
+        DCHECK_EQ(IrOpcode::kSwitch, node->opcode());
+        index = projection_count - 1;
+        break;
+    }
+    DCHECK_LT(if_value_index, projection_count);
+    DCHECK_LT(index, projection_count);
+    DCHECK_NULL(projections[index]);
+    projections[index] = use;
+  }
+#ifdef DEBUG
+  for (size_t index = 0; index < projection_count; ++index) {
+    DCHECK_NOT_NULL(projections[index]);
+  }
+#endif
+}
+
+
+// static
 bool NodeProperties::AllValueInputsAreTyped(Node* node) {
   int input_count = node->op()->ValueInputCount();
   for (int index = 0; index < input_count; ++index) {

diff --git a/src/compiler/node-properties.h b/src/compiler/node-properties.h
index b40faf3..a13eea3 100644 (file)
--- a/src/compiler/node-properties.h
+++ b/src/compiler/node-properties.h
@@ -90,6 +90,12 @@ class NodeProperties FINAL {
 
   static Node* FindProjection(Node* node, size_t projection_index);
 
+  // Collect the branch-related projections from a node, such as IfTrue,
+  // IfFalse, IfValue and IfDefault.
+  //  - Branch: [ IfTrue, IfFalse ]
+  //  - Switch: [ IfValue, ..., IfDefault ]
+  static void CollectControlProjections(Node* node, Node** proj, size_t count);
+
 
   // ---------------------------------------------------------------------------
   // Type Bounds.

diff --git a/src/compiler/scheduler.cc b/src/compiler/scheduler.cc
index a21d650..6e105e3 100644 (file)
--- a/src/compiler/scheduler.cc
+++ b/src/compiler/scheduler.cc
@@ -364,63 +364,19 @@ class CFGBuilder : public ZoneObject {
   }
 
   void BuildBlocksForSuccessors(Node* node) {
-    size_t const successor_count = node->op()->ControlOutputCount();
-    Node** successors = zone_->NewArray<Node*>(successor_count);
-    CollectSuccessorProjections(node, successors, successor_count);
-    for (size_t index = 0; index < successor_count; ++index) {
+    size_t const successor_cnt = node->op()->ControlOutputCount();
+    Node** successors = zone_->NewArray<Node*>(successor_cnt);
+    NodeProperties::CollectControlProjections(node, successors, successor_cnt);
+    for (size_t index = 0; index < successor_cnt; ++index) {
       BuildBlockForNode(successors[index]);
     }
   }
 
-  // Collect the branch-related projections from a node, such as IfTrue,
-  // IfFalse, Case and Default.
-  void CollectSuccessorProjections(Node* node, Node** successors,
-                                   size_t successor_count) {
-#ifdef DEBUG
-    DCHECK_EQ(static_cast<int>(successor_count), node->UseCount());
-    std::memset(successors, 0, sizeof(*successors) * successor_count);
-#endif
-    size_t if_value_index = 0;
-    for (Node* const use : node->uses()) {
-      size_t index;
-      switch (use->opcode()) {
-        default:
-          UNREACHABLE();
-        // Fall through.
-        case IrOpcode::kIfTrue:
-          DCHECK_EQ(IrOpcode::kBranch, node->opcode());
-          index = 0;
-          break;
-        case IrOpcode::kIfFalse:
-          DCHECK_EQ(IrOpcode::kBranch, node->opcode());
-          index = 1;
-          break;
-        case IrOpcode::kIfValue:
-          DCHECK_EQ(IrOpcode::kSwitch, node->opcode());
-          index = if_value_index++;
-          break;
-        case IrOpcode::kIfDefault:
-          DCHECK_EQ(IrOpcode::kSwitch, node->opcode());
-          index = successor_count - 1;
-          break;
-      }
-      DCHECK_LT(if_value_index, successor_count);
-      DCHECK_LT(index, successor_count);
-      DCHECK_NULL(successors[index]);
-      successors[index] = use;
-    }
-#ifdef DEBUG
-    for (size_t index = 0; index < successor_count; ++index) {
-      DCHECK_NOT_NULL(successors[index]);
-    }
-#endif
-  }
-
   void CollectSuccessorBlocks(Node* node, BasicBlock** successor_blocks,
-                              size_t successor_count) {
+                              size_t successor_cnt) {
     Node** successors = reinterpret_cast<Node**>(successor_blocks);
-    CollectSuccessorProjections(node, successors, successor_count);
-    for (size_t index = 0; index < successor_count; ++index) {
+    NodeProperties::CollectControlProjections(node, successors, successor_cnt);
+    for (size_t index = 0; index < successor_cnt; ++index) {
       successor_blocks[index] = schedule_->block(successors[index]);
     }
   }

diff --git a/test/unittests/compiler/node-properties-unittest.cc b/test/unittests/compiler/node-properties-unittest.cc
index d6d8073..bb471bd 100644 (file)
--- a/test/unittests/compiler/node-properties-unittest.cc
+++ b/test/unittests/compiler/node-properties-unittest.cc
@@ -7,6 +7,7 @@
 #include "test/unittests/test-utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+using testing::AnyOf;
 using testing::IsNull;
 
 namespace v8 {
@@ -27,6 +28,32 @@ TEST_F(NodePropertiesTest, FindProjection) {
   EXPECT_THAT(NodeProperties::FindProjection(start, 1234567890), IsNull());
 }
 
+
+TEST_F(NodePropertiesTest, CollectControlProjections_Branch) {
+  Node* result[2];
+  CommonOperatorBuilder common(zone());
+  Node* branch = Node::New(zone(), 1, common.Branch(), 0, nullptr, false);
+  Node* if_false = Node::New(zone(), 2, common.IfFalse(), 1, &branch, false);
+  Node* if_true = Node::New(zone(), 3, common.IfTrue(), 1, &branch, false);
+  NodeProperties::CollectControlProjections(branch, result, arraysize(result));
+  EXPECT_EQ(if_true, result[0]);
+  EXPECT_EQ(if_false, result[1]);
+}
+
+
+TEST_F(NodePropertiesTest, CollectControlProjections_Switch) {
+  Node* result[3];
+  CommonOperatorBuilder common(zone());
+  Node* sw = Node::New(zone(), 1, common.Switch(3), 0, nullptr, false);
+  Node* if_default = Node::New(zone(), 2, common.IfDefault(), 1, &sw, false);
+  Node* if_value1 = Node::New(zone(), 3, common.IfValue(1), 1, &sw, false);
+  Node* if_value2 = Node::New(zone(), 4, common.IfValue(2), 1, &sw, false);
+  NodeProperties::CollectControlProjections(sw, result, arraysize(result));
+  EXPECT_THAT(result[0], AnyOf(if_value1, if_value2));
+  EXPECT_THAT(result[1], AnyOf(if_value1, if_value2));
+  EXPECT_EQ(if_default, result[2]);
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8