Commit a5f1d5d4 authored by Daniel Clifford's avatar Daniel Clifford Committed by Commit Bot

Ensure CSA-generated code can handle one-input phis

In general, TurboFan doesn't encounter phi nodes with only a single
data input in the backend. However, CSA-based builtins (especially
auto-generated ones, e.g. from Torque), may contain single-input phi nodes,
although outside the auto-generated case this doesn't happen much in practice.

Single input phi nodes (i.e. phis in blocks with one predecessor) don't have
any side effects and are essentially useless and harmless, but to avoid problems
in the backend of TurboFan (whose SSA deconstruction disallows control flow
splits that continue to blocks with phis), this CL tweaks the existing
CSA-only control flow and graph sanitization in the CSA path to ensure
no no-op phis.

Change-Id: I109f4dc6cde5ad1794585a09609a230b1848e0d5
Reviewed-on: https://chromium-review.googlesource.com/963711Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Daniel Clifford <danno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52056}
parent f0940a63
...@@ -352,6 +352,25 @@ void Schedule::EnsureCFGWellFormedness() { ...@@ -352,6 +352,25 @@ void Schedule::EnsureCFGWellFormedness() {
if (block->deferred()) { if (block->deferred()) {
EnsureDeferredCodeSingleEntryPoint(block); EnsureDeferredCodeSingleEntryPoint(block);
} }
} else {
EliminateNoopPhiNodes(block);
}
}
}
void Schedule::EliminateNoopPhiNodes(BasicBlock* block) {
// Ensure that useless phi nodes in blocks that only have a single predecessor
// -- which can happen with the automatically generated code in the CSA and
// torque -- are pruned.
if (block->PredecessorCount() == 1) {
for (size_t i = 0; i < block->NodeCount();) {
Node* node = block->NodeAt(i);
if (node->opcode() == IrOpcode::kPhi) {
node->ReplaceUses(node->InputAt(0));
block->RemoveNode(block->begin() + i);
} else {
++i;
}
} }
} }
} }
......
...@@ -274,6 +274,10 @@ class V8_EXPORT_PRIVATE Schedule final : public NON_EXPORTED_BASE(ZoneObject) { ...@@ -274,6 +274,10 @@ class V8_EXPORT_PRIVATE Schedule final : public NON_EXPORTED_BASE(ZoneObject) {
// Ensure properties of the CFG assumed by further stages. // Ensure properties of the CFG assumed by further stages.
void EnsureCFGWellFormedness(); void EnsureCFGWellFormedness();
// Eliminates no-op phi nodes added for blocks that only have a single
// predecessor. This ensures the property required for SSA deconstruction that
// the target block of a control flow split has no phis.
void EliminateNoopPhiNodes(BasicBlock* block);
// Ensure split-edge form for a hand-assembled schedule. // Ensure split-edge form for a hand-assembled schedule.
void EnsureSplitEdgeForm(BasicBlock* block); void EnsureSplitEdgeForm(BasicBlock* block);
// Ensure entry into a deferred block happens from a single hot block. // Ensure entry into a deferred block happens from a single hot block.
......
...@@ -3089,6 +3089,32 @@ TEST(ExtractFixedArraySimpleIntPtrParameters) { ...@@ -3089,6 +3089,32 @@ TEST(ExtractFixedArraySimpleIntPtrParameters) {
CHECK_EQ(double_result->get_scalar(1), 12); CHECK_EQ(double_result->get_scalar(1), 12);
} }
TEST(SingleInputPhiElimination) {
Isolate* isolate(CcTest::InitIsolateOnce());
const int kNumParams = 2;
CodeAssemblerTester asm_tester(isolate, kNumParams);
{
CodeStubAssembler m(asm_tester.state());
Variable temp1(&m, MachineRepresentation::kTagged);
Variable temp2(&m, MachineRepresentation::kTagged);
Label temp_label(&m, {&temp1, &temp2});
Label end_label(&m, {&temp1, &temp2});
temp1.Bind(m.Parameter(1));
temp2.Bind(m.Parameter(1));
m.Branch(m.WordEqual(m.Parameter(0), m.Parameter(1)), &end_label,
&temp_label);
temp1.Bind(m.Parameter(2));
temp2.Bind(m.Parameter(2));
m.BIND(&temp_label);
m.Goto(&end_label);
m.BIND(&end_label);
m.Return(temp1.value());
}
FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
// Generating code without an assert is enough to make sure that the
// single-input phi is properly eliminated.
}
} // namespace compiler } // namespace compiler
} // namespace internal } // namespace internal
} // namespace v8 } // 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