Reviewers: jarin,
Description:
[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.
[email protected]
Please review this at https://codereview.chromium.org/1195443004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+44, -47 lines):
M src/compiler/control-reducer.cc
M test/cctest/compiler/test-control-reducer.cc
M test/cctest/compiler/test-osr.cc
Index: src/compiler/control-reducer.cc
diff --git a/src/compiler/control-reducer.cc
b/src/compiler/control-reducer.cc
index
4644440f9c6e1826b11ef940446248437fe907b2..633dfe5abf0b31eb76b13478d43e029edbb0740b
100644
--- a/src/compiler/control-reducer.cc
+++ b/src/compiler/control-reducer.cc
@@ -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();
Index: test/cctest/compiler/test-control-reducer.cc
diff --git a/test/cctest/compiler/test-control-reducer.cc
b/test/cctest/compiler/test-control-reducer.cc
index
713090ff7cf66961dcc5c9c5dc9b12629a36d76d..e34dc9a2045f4a7dfa10c4ac2ee19263dae3c771
100644
--- a/test/cctest/compiler/test-control-reducer.cc
+++ b/test/cctest/compiler/test-control-reducer.cc
@@ -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);
Index: test/cctest/compiler/test-osr.cc
diff --git a/test/cctest/compiler/test-osr.cc
b/test/cctest/compiler/test-osr.cc
index
e60e27acdec56d4df8e7eec8a3bed749d999c3ee..77ade9149e3cf6efe963f802d238c40064a99934
100644
--- a/test/cctest/compiler/test-osr.cc
+++ b/test/cctest/compiler/test-osr.cc
@@ -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);
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
