Skip to content

V
V8
Commit afc2fb26 authored by bmeurer's avatar bmeurer Committed by Commit bot
Browse files
Options

[turbofan] Remove another ineffective optimization from the ControlReducer. 

The condition of a Branch or Select can never be a NumberConstant,
because the resulting graph would be invalid, so we don't need to
optimize this case. It can only ever be a tagged boolean or an untagged
bit.

Drive-by-fix: Test the interesting cases in the unit tests instead.

R=jarin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#29089}
parent f28f16c9
Hide whitespace changes
Inline Side-by-side
Showing with 44 additions and 47 deletions
src/compiler/control-reducer.cc
View file @ afc2fb26
......@@ -93,8 +93,6 @@ class ControlReducerImpl final : public AdvancedReducer {
return Int32Matcher(cond).Is(0) ? kFalse : kTrue;
case IrOpcode::kInt64Constant:
return Int64Matcher(cond).Is(0) ? kFalse : kTrue;
case IrOpcode::kNumberConstant:
return NumberMatcher(cond).Is(0) ? kFalse : kTrue;
case IrOpcode::kHeapConstant: {
Handle<Object> object =
HeapObjectMatcher<Object>(cond).Value().handle();
......
test/cctest/compiler/test-control-reducer.cc
View file @ afc2fb26
......@@ -902,11 +902,9 @@ TEST(CBranchReduce_none2) {
TEST(CBranchReduce_true) {
ControlReducerTester R;
Node* true_values[] = {
R.one, R.jsgraph.Int32Constant(2),
R.jsgraph.Int32Constant(0x7fffffff), R.jsgraph.Constant(1.0),
R.jsgraph.Constant(22.1), R.jsgraph.TrueConstant()};
Node* true_values[] = {R.jsgraph.Int32Constant(2),
R.jsgraph.Int64Constant(0x7fffffff),
R.jsgraph.TrueConstant()};
for (size_t i = 0; i < arraysize(true_values); i++) {
Diamond d(R, true_values[i]);
R.ReduceBranch(kTrue, d.branch);
......@@ -916,9 +914,9 @@ TEST(CBranchReduce_true) {
TEST(CBranchReduce_false) {
ControlReducerTester R;
Node* false_values[] = {R.zero, R.jsgraph.Constant(0.0),
R.jsgraph.Constant(-0.0), R.jsgraph.FalseConstant()};
Node* false_values[] = {R.jsgraph.Int32Constant(0),
R.jsgraph.Int64Constant(0),
R.jsgraph.FalseConstant()};
for (size_t i = 0; i < arraysize(false_values); i++) {
Diamond d(R, false_values[i]);
R.ReduceBranch(kFalse, d.branch);
......@@ -928,22 +926,22 @@ TEST(CBranchReduce_false) {
TEST(CDiamondReduce_true) {
ControlReducerTester R;
Diamond d1(R, R.one);
Diamond d1(R, R.jsgraph.TrueConstant());
R.ReduceMergeIterative(R.start, d1.merge);
}
TEST(CDiamondReduce_false) {
ControlReducerTester R;
Diamond d2(R, R.zero);
Diamond d2(R, R.jsgraph.FalseConstant());
R.ReduceMergeIterative(R.start, d2.merge);
}
TEST(CChainedDiamondsReduce_true_false) {
ControlReducerTester R;
Diamond d1(R, R.one);
Diamond d2(R, R.zero);
Diamond d1(R, R.jsgraph.TrueConstant());
Diamond d2(R, R.jsgraph.FalseConstant());
d2.chain(d1);
R.ReduceMergeIterative(R.start, d2.merge);
......@@ -953,7 +951,7 @@ TEST(CChainedDiamondsReduce_true_false) {
TEST(CChainedDiamondsReduce_x_false) {
ControlReducerTester R;
Diamond d1(R, R.p0);
Diamond d2(R, R.zero);
Diamond d2(R, R.jsgraph.FalseConstant());
d2.chain(d1);
R.ReduceMergeIterative(R.start, d2.merge);
......@@ -982,7 +980,8 @@ TEST(CChainedDiamondsReduce_phi1) {
TEST(CChainedDiamondsReduce_phi2) {
ControlReducerTester R;
Diamond d1(R, R.p0, R.one, R.one); // redundant phi.
Diamond d1(R, R.p0, R.jsgraph.TrueConstant(),
R.jsgraph.TrueConstant()); // redundant phi.
Diamond d2(R, d1.phi);
d2.chain(d1);
......@@ -992,8 +991,8 @@ TEST(CChainedDiamondsReduce_phi2) {
TEST(CNestedDiamondsReduce_true_true_false) {
ControlReducerTester R;
Diamond d1(R, R.one);
Diamond d2(R, R.zero);
Diamond d1(R, R.jsgraph.TrueConstant());
Diamond d2(R, R.jsgraph.FalseConstant());
d2.nest(d1, true);
R.ReduceMergeIterative(R.start, d1.merge);
......@@ -1002,8 +1001,8 @@ TEST(CNestedDiamondsReduce_true_true_false) {
TEST(CNestedDiamondsReduce_false_true_false) {
ControlReducerTester R;
Diamond d1(R, R.one);
Diamond d2(R, R.zero);
Diamond d1(R, R.jsgraph.TrueConstant());
Diamond d2(R, R.jsgraph.FalseConstant());
d2.nest(d1, false);
R.ReduceMergeIterative(R.start, d1.merge);
......@@ -1077,7 +1076,7 @@ TEST(Return1) {
TEST(Return2) {
ControlReducerTester R;
Diamond d(R, R.one);
Diamond d(R, R.jsgraph.TrueConstant());
Node* ret = R.Return(R.half, R.start, d.merge);
R.ReduceGraph();
......@@ -1094,7 +1093,7 @@ TEST(Return2) {
TEST(Return_true1) {
ControlReducerTester R;
Diamond d(R, R.one, R.half, R.zero);
Diamond d(R, R.jsgraph.TrueConstant(), R.half, R.zero);
Node* ret = R.Return(d.phi, R.start, d.merge);
R.ReduceGraph();
......@@ -1112,7 +1111,7 @@ TEST(Return_true1) {
TEST(Return_false1) {
ControlReducerTester R;
Diamond d(R, R.zero, R.one, R.half);
Diamond d(R, R.jsgraph.FalseConstant(), R.one, R.half);
Node* ret = R.Return(d.phi, R.start, d.merge);
R.ReduceGraph();
......@@ -1130,7 +1129,7 @@ TEST(Return_false1) {
TEST(Return_effect1) {
ControlReducerTester R;
Diamond d(R, R.one);
Diamond d(R, R.jsgraph.TrueConstant());
Node* e1 = R.jsgraph.Float64Constant(-100.1);
Node* e2 = R.jsgraph.Float64Constant(+100.1);
Node* effect = R.graph.NewNode(R.common.EffectPhi(2), e1, e2, d.merge);
......@@ -1218,7 +1217,7 @@ TEST(Return_nested_diamonds1_dead2) {
TEST(Return_nested_diamonds_true1) {
ControlReducerTester R;
Diamond d2(R, R.p0, R.one, R.zero);
Diamond d1(R, R.one, d2.phi, R.zero);
Diamond d1(R, R.jsgraph.TrueConstant(), d2.phi, R.zero);
Diamond d3(R, R.p0);
d2.nest(d1, true);
......@@ -1263,8 +1262,8 @@ TEST(Return_nested_diamonds_false1) {
TEST(Return_nested_diamonds_true_true1) {
ControlReducerTester R;
Diamond d1(R, R.one, R.one, R.zero);
Diamond d2(R, R.one);
Diamond d1(R, R.jsgraph.TrueConstant(), R.one, R.zero);
Diamond d2(R, R.jsgraph.TrueConstant());
Diamond d3(R, R.p0);
d2.nest(d1, true);
......@@ -1285,8 +1284,8 @@ TEST(Return_nested_diamonds_true_true1) {
TEST(Return_nested_diamonds_true_false1) {
ControlReducerTester R;
Diamond d1(R, R.one, R.one, R.zero);
Diamond d2(R, R.zero);
Diamond d1(R, R.jsgraph.TrueConstant(), R.one, R.zero);
Diamond d2(R, R.jsgraph.FalseConstant());
Diamond d3(R, R.p0);
d2.nest(d1, true);
......@@ -1342,7 +1341,7 @@ TEST(Return_nested_diamonds_true2) {
Diamond d2(R, R.p0, x2, y2);
Diamond d3(R, R.p0, x3, y3);
Diamond d1(R, R.one, d2.phi, d3.phi);
Diamond d1(R, R.jsgraph.TrueConstant(), d2.phi, d3.phi);
d2.nest(d1, true);
d3.nest(d1, false);
......@@ -1368,9 +1367,9 @@ TEST(Return_nested_diamonds_true_true2) {
Node* x3 = R.jsgraph.Float64Constant(33.3);
Node* y3 = R.jsgraph.Float64Constant(44.4);
Diamond d2(R, R.one, x2, y2);
Diamond d2(R, R.jsgraph.TrueConstant(), x2, y2);
Diamond d3(R, R.p0, x3, y3);
Diamond d1(R, R.one, d2.phi, d3.phi);
Diamond d1(R, R.jsgraph.TrueConstant(), d2.phi, d3.phi);
d2.nest(d1, true);
d3.nest(d1, false);
......@@ -1395,9 +1394,9 @@ TEST(Return_nested_diamonds_true_false2) {
Node* x3 = R.jsgraph.Float64Constant(33.3);
Node* y3 = R.jsgraph.Float64Constant(44.4);
Diamond d2(R, R.zero, x2, y2);
Diamond d2(R, R.jsgraph.FalseConstant(), x2, y2);
Diamond d3(R, R.p0, x3, y3);
Diamond d1(R, R.one, d2.phi, d3.phi);
Diamond d1(R, R.jsgraph.TrueConstant(), d2.phi, d3.phi);
d2.nest(d1, true);
d3.nest(d1, false);
......
test/cctest/compiler/test-osr.cc
View file @ afc2fb26
......@@ -370,8 +370,8 @@ TEST(Deconstruct_osr_nested1) {
Node* outer_phi = outer.Phi(T.p0, T.p0, nullptr);
outer.branch->ReplaceInput(0, outer_phi);
Node* osr_phi = inner.Phi(T.jsgraph.OneConstant(), T.osr_values[0],
T.jsgraph.ZeroConstant());
Node* osr_phi = inner.Phi(T.jsgraph.TrueConstant(), T.osr_values[0],
T.jsgraph.FalseConstant());
inner.branch->ReplaceInput(0, osr_phi);
outer_phi->ReplaceInput(1, osr_phi);
......@@ -385,7 +385,7 @@ TEST(Deconstruct_osr_nested1) {
// Check structure of deconstructed graph.
// Check inner OSR loop is directly connected to start.
CheckInputs(inner.loop, T.start, inner.if_true);
CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.ZeroConstant(), inner.loop);
CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.FalseConstant(), inner.loop);
// Check control transfer to copy of outer loop.
Node* new_outer_loop = FindSuccessor(inner.exit, IrOpcode::kLoop);
......@@ -412,8 +412,8 @@ TEST(Deconstruct_osr_nested1) {
Node* new_inner_loop = FindSuccessor(new_outer_if_true, IrOpcode::kLoop);
Node* new_inner_phi = FindSuccessor(new_inner_loop, IrOpcode::kPhi);
CheckInputs(new_inner_phi, T.jsgraph.OneConstant(), T.jsgraph.ZeroConstant(),
new_inner_loop);
CheckInputs(new_inner_phi, T.jsgraph.TrueConstant(),
T.jsgraph.FalseConstant(), new_inner_loop);
CheckInputs(new_outer_phi, osr_phi, new_inner_phi, new_outer_loop);
}
......@@ -429,11 +429,11 @@ TEST(Deconstruct_osr_nested2) {
Node* outer_phi = outer.Phi(T.p0, T.p0, T.p0);
outer.branch->ReplaceInput(0, outer_phi);
Node* osr_phi = inner.Phi(T.jsgraph.OneConstant(), T.osr_values[0],
T.jsgraph.ZeroConstant());
Node* osr_phi = inner.Phi(T.jsgraph.TrueConstant(), T.osr_values[0],
T.jsgraph.FalseConstant());
inner.branch->ReplaceInput(0, osr_phi);
outer_phi->ReplaceInput(1, osr_phi);
outer_phi->ReplaceInput(2, T.jsgraph.ZeroConstant());
outer_phi->ReplaceInput(2, T.jsgraph.FalseConstant());
Node* x_branch = T.graph.NewNode(T.common.Branch(), osr_phi, inner.exit);
Node* x_true = T.graph.NewNode(T.common.IfTrue(), x_branch);
......@@ -452,7 +452,7 @@ TEST(Deconstruct_osr_nested2) {
// Check structure of deconstructed graph.
// Check inner OSR loop is directly connected to start.
CheckInputs(inner.loop, T.start, inner.if_true);
CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.ZeroConstant(), inner.loop);
CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.FalseConstant(), inner.loop);
// Check control transfer to copy of outer loop.
Node* new_merge = FindSuccessor(x_true, IrOpcode::kMerge);
......@@ -465,7 +465,7 @@ TEST(Deconstruct_osr_nested2) {
CHECK_NE(new_outer_phi, outer_phi);
Node* new_entry_phi = FindSuccessor(new_merge, IrOpcode::kPhi);
CheckInputs(new_entry_phi, osr_phi, T.jsgraph.ZeroConstant(), new_merge);
CheckInputs(new_entry_phi, osr_phi, T.jsgraph.FalseConstant(), new_merge);
CHECK_EQ(new_merge, new_outer_loop->InputAt(0));
......@@ -486,10 +486,10 @@ TEST(Deconstruct_osr_nested2) {
Node* new_inner_loop = FindSuccessor(new_outer_if_true, IrOpcode::kLoop);
Node* new_inner_phi = FindSuccessor(new_inner_loop, IrOpcode::kPhi);
CheckInputs(new_inner_phi, T.jsgraph.OneConstant(), T.jsgraph.ZeroConstant(),
new_inner_loop);
CheckInputs(new_inner_phi, T.jsgraph.TrueConstant(),
T.jsgraph.FalseConstant(), new_inner_loop);
CheckInputs(new_outer_phi, new_entry_phi, new_inner_phi,
T.jsgraph.ZeroConstant(), new_outer_loop);
T.jsgraph.FalseConstant(), new_outer_loop);
}
......
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