From 7e7e85f32e9336a2c5e38b67bd1bb01fa2915a40 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Wed, 18 Feb 2015 05:55:24 -0800 Subject: [PATCH] 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} --- src/compiler/control-flow-optimizer.cc | 15 ++--- src/compiler/node-properties.cc | 44 ++++++++++++++ src/compiler/node-properties.h | 6 ++ src/compiler/scheduler.cc | 58 +++---------------- .../compiler/node-properties-unittest.cc | 27 +++++++++ 5 files changed, 89 insertions(+), 61 deletions(-) diff --git a/src/compiler/control-flow-optimizer.cc b/src/compiler/control-flow-optimizer.cc index c75d09f0ce..1a2b4cdfd8 100644 --- 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 1575310a7f..47de74e329 100644 --- a/src/compiler/node-properties.cc +++ b/src/compiler/node-properties.cc @@ -181,6 +181,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(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(); diff --git a/src/compiler/node-properties.h b/src/compiler/node-properties.h index b40faf39d4..a13eea3a02 100644 --- 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 a21d650ed3..6e105e3713 100644 --- 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(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(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(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(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 d6d8073816..bb471bd01e 100644 --- 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