Skip to content

V
V8
Commit 60c95d85 authored by epertoso's avatar epertoso Committed by Commit bot
Browse files
Options

[turbofan] Move TryCloneBranch in the EffectControlLinearizer pass. 

When trying to clone a branch, the ControlFlowOptimizer gave up as soon as it found a Phi/EffectPhi node that could not be placed directly below the IfTrue or IfFalse control paths.

Moving the step in the EffectControlLinearizer phase, after the first schedule, works around the problem by looking at the successor blocks.

BUG=

Review-Url: https://codereview.chromium.org/2139593002
Cr-Commit-Position: refs/heads/master@{#37687}
parent 8f1f1cb1
Expand all Show whitespace changes
Inline Side-by-side
Showing with 288 additions and 195 deletions
src/compiler/control-flow-optimizer.cc
View file @ 60c95d85
......@@ -63,146 +63,10 @@ void ControlFlowOptimizer::VisitNode(Node* node) {
void ControlFlowOptimizer::VisitBranch(Node* node) {
DCHECK_EQ(IrOpcode::kBranch, node->opcode());
if (TryBuildSwitch(node)) return;
if (TryCloneBranch(node)) return;
VisitNode(node);
}
bool ControlFlowOptimizer::TryCloneBranch(Node* node) {
DCHECK_EQ(IrOpcode::kBranch, node->opcode());
// This optimization is a special case of (super)block cloning. It takes an
// input graph as shown below and clones the Branch node for every predecessor
// to the Merge, essentially removing the Merge completely. This avoids
// materializing the bit for the Phi and may offer potential for further
// branch folding optimizations (i.e. because one or more inputs to the Phi is
// a constant). Note that there may be more Phi nodes hanging off the Merge,
// but we can only a certain subset of them currently (actually only Phi and
// EffectPhi nodes whose uses have either the IfTrue or IfFalse as control
// input).
// Control1 ... ControlN
// ^ ^
// | | Cond1 ... CondN
// +----+ +----+ ^ ^
// | | | |
// | | +----+ |
// Merge<--+ | +------------+
// ^ \|/
// | Phi
// | |
// Branch----+
// ^
// |
// +-----+-----+
// | |
// IfTrue IfFalse
// ^ ^
// | |
// The resulting graph (modulo the Phi and EffectPhi nodes) looks like this:
// Control1 Cond1 ... ControlN CondN
// ^ ^ ^ ^
// \ / \ /
// Branch ... Branch
// ^ ^
// | |
// +---+---+ +---+----+
// | | | |
// IfTrue IfFalse ... IfTrue IfFalse
// ^ ^ ^ ^
// | | | |
// +--+ +-------------+ |
// | | +--------------+ +--+
// | | | |
// Merge Merge
// ^ ^
// | |
Node* branch = node;
Node* cond = NodeProperties::GetValueInput(branch, 0);
if (!cond->OwnedBy(branch) || cond->opcode() != IrOpcode::kPhi) return false;
Node* merge = NodeProperties::GetControlInput(branch);
if (merge->opcode() != IrOpcode::kMerge ||
NodeProperties::GetControlInput(cond) != merge) {
return false;
}
// Grab the IfTrue/IfFalse projections of the Branch.
BranchMatcher matcher(branch);
// Check/collect other Phi/EffectPhi nodes hanging off the Merge.
NodeVector phis(zone());
for (Node* const use : merge->uses()) {
if (use == branch || use == cond) continue;
// We cannot currently deal with non-Phi/EffectPhi nodes hanging off the
// Merge. Ideally, we would just clone the nodes (and everything that
// depends on it to some distant join point), but that requires knowledge
// about dominance/post-dominance.
if (!NodeProperties::IsPhi(use)) return false;
for (Edge edge : use->use_edges()) {
// Right now we can only handle Phi/EffectPhi nodes whose uses are
// directly control-dependend on either the IfTrue or the IfFalse
// successor, because we know exactly how to update those uses.
// TODO(turbofan): Generalize this to all Phi/EffectPhi nodes using
// dominance/post-dominance on the sea of nodes.
if (edge.from()->op()->ControlInputCount() != 1) return false;
Node* control = NodeProperties::GetControlInput(edge.from());
if (NodeProperties::IsPhi(edge.from())) {
control = NodeProperties::GetControlInput(control, edge.index());
}
if (control != matcher.IfTrue() && control != matcher.IfFalse())
return false;
}
phis.push_back(use);
}
BranchHint const hint = BranchHintOf(branch->op());
int const input_count = merge->op()->ControlInputCount();
DCHECK_LE(1, input_count);
Node** const inputs = zone()->NewArray<Node*>(2 * input_count);
Node** const merge_true_inputs = &inputs[0];
Node** const merge_false_inputs = &inputs[input_count];
for (int index = 0; index < input_count; ++index) {
Node* cond1 = NodeProperties::GetValueInput(cond, index);
Node* control1 = NodeProperties::GetControlInput(merge, index);
Node* branch1 = graph()->NewNode(common()->Branch(hint), cond1, control1);
merge_true_inputs[index] = graph()->NewNode(common()->IfTrue(), branch1);
merge_false_inputs[index] = graph()->NewNode(common()->IfFalse(), branch1);
Enqueue(branch1);
}
Node* const merge_true = graph()->NewNode(common()->Merge(input_count),
input_count, merge_true_inputs);
Node* const merge_false = graph()->NewNode(common()->Merge(input_count),
input_count, merge_false_inputs);
for (Node* const phi : phis) {
for (int index = 0; index < input_count; ++index) {
inputs[index] = phi->InputAt(index);
}
inputs[input_count] = merge_true;
Node* phi_true = graph()->NewNode(phi->op(), input_count + 1, inputs);
inputs[input_count] = merge_false;
Node* phi_false = graph()->NewNode(phi->op(), input_count + 1, inputs);
for (Edge edge : phi->use_edges()) {
Node* control = NodeProperties::GetControlInput(edge.from());
if (NodeProperties::IsPhi(edge.from())) {
control = NodeProperties::GetControlInput(control, edge.index());
}
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.
matcher.IfFalse()->ReplaceUses(merge_false);
matcher.IfTrue()->ReplaceUses(merge_true);
matcher.IfFalse()->Kill();
matcher.IfTrue()->Kill();
branch->Kill();
cond->Kill();
merge->Kill();
return true;
}
bool ControlFlowOptimizer::TryBuildSwitch(Node* node) {
DCHECK_EQ(IrOpcode::kBranch, node->opcode());
......
src/compiler/effect-control-linearizer.cc
View file @ 60c95d85
This diff is collapsed.
test/unittests/compiler/control-flow-optimizer-unittest.cc
View file @ 60c95d85
......@@ -96,34 +96,6 @@ TEST_F(ControlFlowOptimizerTest, BuildSwitch2) {
IsSwitch(index, IsIfSuccess(index)))))));
}
TEST_F(ControlFlowOptimizerTest, CloneBranch) {
Node* cond0 = Parameter(0);
Node* cond1 = Parameter(1);
Node* cond2 = Parameter(2);
Node* branch0 = graph()->NewNode(common()->Branch(), cond0, start());
Node* control1 = graph()->NewNode(common()->IfTrue(), branch0);
Node* control2 = graph()->NewNode(common()->IfFalse(), branch0);
Node* merge0 = graph()->NewNode(common()->Merge(2), control1, control2);
Node* phi0 = graph()->NewNode(common()->Phi(MachineRepresentation::kBit, 2),
cond1, cond2, merge0);
Node* branch = graph()->NewNode(common()->Branch(), phi0, merge0);
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);
graph()->SetEnd(graph()->NewNode(common()->End(1), merge));
Optimize();
Capture<Node*> branch1_capture, branch2_capture;
EXPECT_THAT(
end(),
IsEnd(IsMerge(IsMerge(IsIfTrue(CaptureEq(&branch1_capture)),
IsIfTrue(CaptureEq(&branch2_capture))),
IsMerge(IsIfFalse(AllOf(CaptureEq(&branch1_capture),
IsBranch(cond1, control1))),
IsIfFalse(AllOf(CaptureEq(&branch2_capture),
IsBranch(cond2, control2)))))));
}
} // namespace compiler
} // namespace internal
} // namespace v8
test/unittests/compiler/effect-control-linearizer-unittest.cc
View file @ 60c95d85
......@@ -12,12 +12,15 @@
#include "test/unittests/compiler/graph-unittest.h"
#include "test/unittests/compiler/node-test-utils.h"
#include "test/unittests/test-utils.h"
#include "testing/gmock-support.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace v8 {
namespace internal {
namespace compiler {
using testing::Capture;
class EffectControlLinearizerTest : public TypedGraphTest {
public:
EffectControlLinearizerTest()
......@@ -323,6 +326,74 @@ TEST_F(EffectControlLinearizerTest, LoopLoad) {
heap_number, effect_phi, loop));
}
TEST_F(EffectControlLinearizerTest, CloneBranch) {
Schedule schedule(zone());
Node* cond0 = Parameter(0);
Node* cond1 = Parameter(1);
Node* cond2 = Parameter(2);
Node* branch0 = graph()->NewNode(common()->Branch(), cond0, start());
Node* control1 = graph()->NewNode(common()->IfTrue(), branch0);
Node* control2 = graph()->NewNode(common()->IfFalse(), branch0);
Node* merge0 = graph()->NewNode(common()->Merge(2), control1, control2);
Node* phi0 = graph()->NewNode(common()->Phi(MachineRepresentation::kBit, 2),
cond1, cond2, merge0);
Node* branch = graph()->NewNode(common()->Branch(), phi0, merge0);
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);
graph()->SetEnd(graph()->NewNode(common()->End(1), merge));
BasicBlock* start = schedule.start();
schedule.rpo_order()->push_back(start);
start->set_rpo_number(0);
BasicBlock* f1block = AddBlockToSchedule(&schedule);
BasicBlock* t1block = AddBlockToSchedule(&schedule);
BasicBlock* bblock = AddBlockToSchedule(&schedule);
BasicBlock* f2block = AddBlockToSchedule(&schedule);
BasicBlock* t2block = AddBlockToSchedule(&schedule);
BasicBlock* mblock = AddBlockToSchedule(&schedule);
// Populate the basic blocks with nodes.
schedule.AddNode(start, graph()->start());
schedule.AddBranch(start, branch0, t1block, f1block);
schedule.AddNode(t1block, control1);
schedule.AddGoto(t1block, bblock);
schedule.AddNode(f1block, control2);
schedule.AddGoto(f1block, bblock);
schedule.AddNode(bblock, merge0);
schedule.AddNode(bblock, phi0);
schedule.AddBranch(bblock, branch, t2block, f2block);
schedule.AddNode(t2block, if_true);
schedule.AddGoto(t2block, mblock);
schedule.AddNode(f2block, if_false);
schedule.AddGoto(f2block, mblock);
schedule.AddNode(mblock, merge);
schedule.AddNode(mblock, graph()->end());
EffectControlLinearizer introducer(jsgraph(), &schedule, zone());
introducer.Run();
Capture<Node *> branch1_capture, branch2_capture;
EXPECT_THAT(
end(),
IsEnd(IsMerge(IsMerge(IsIfTrue(CaptureEq(&branch1_capture)),
IsIfTrue(CaptureEq(&branch2_capture))),
IsMerge(IsIfFalse(AllOf(CaptureEq(&branch1_capture),
IsBranch(cond1, control1))),
IsIfFalse(AllOf(CaptureEq(&branch2_capture),
IsBranch(cond2, control2)))))));
}
} // namespace compiler
} // namespace internal
} // namespace v8
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment