From 0798746cb389d676634c7fc2ed83a40852910e07 Mon Sep 17 00:00:00 2001 From: Georg Neis Date: Mon, 16 Mar 2020 12:15:28 +0000 Subject: [PATCH] Revert "[turbofan] Clean up ConstantFoldingReducer" This reverts commit 2c834c53641550bfc0257ee6656953795cfc16de. Reason for revert: several clusterfuzz issues, e.g. 1061805 Original change's description: > [turbofan] Clean up ConstantFoldingReducer > > Change-Id: Iaf7f83cc157a6f6680da8933560347f7f3503d56 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2098736 > Reviewed-by: Tobias Tebbi > Commit-Queue: Georg Neis > Cr-Commit-Position: refs/heads/master@{#66706} TBR=neis@chromium.org,tebbi@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I6e5b655bb465087a50ebaa2088795c6f920c2e51 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2104892 Reviewed-by: Georg Neis Commit-Queue: Georg Neis Cr-Commit-Position: refs/heads/master@{#66717} --- src/compiler/constant-folding-reducer.cc | 73 +++++++------ src/compiler/js-graph.cc | 2 - src/compiler/js-graph.h | 3 +- .../constant-folding-reducer-unittest.cc | 101 ++++++------------ 4 files changed, 70 insertions(+), 109 deletions(-) diff --git a/src/compiler/constant-folding-reducer.cc b/src/compiler/constant-folding-reducer.cc index 93b38841ab..5a903273ed 100644 --- a/src/compiler/constant-folding-reducer.cc +++ b/src/compiler/constant-folding-reducer.cc @@ -11,36 +11,6 @@ namespace v8 { namespace internal { namespace compiler { -namespace { -Node* TryGetConstant(JSGraph* jsgraph, Node* node) { - Type type = NodeProperties::GetType(node); - Node* result; - if (type.IsNone()) { - result = nullptr; - } else if (type.Is(Type::Null())) { - result = jsgraph->NullConstant(); - } else if (type.Is(Type::Undefined())) { - result = jsgraph->UndefinedConstant(); - } else if (type.Is(Type::MinusZero())) { - result = jsgraph->MinusZeroConstant(); - } else if (type.Is(Type::NaN())) { - result = jsgraph->NaNConstant(); - } else if (type.Is(Type::Hole())) { - result = jsgraph->TheHoleConstant(); - } else if (type.IsHeapConstant()) { - result = jsgraph->Constant(type.AsHeapConstant()->Ref()); - } else if (type.Is(Type::PlainNumber()) && type.Min() == type.Max()) { - result = jsgraph->Constant(type.Min()); - } else { - result = nullptr; - } - DCHECK_EQ(result != nullptr, type.IsSingleton()); - DCHECK_IMPLIES(result != nullptr, - NodeProperties::GetType(result).Equals(type)); - return result; -} -} // namespace - ConstantFoldingReducer::ConstantFoldingReducer(Editor* editor, JSGraph* jsgraph, JSHeapBroker* broker) : AdvancedReducer(editor), jsgraph_(jsgraph), broker_(broker) {} @@ -49,15 +19,44 @@ ConstantFoldingReducer::~ConstantFoldingReducer() = default; Reduction ConstantFoldingReducer::Reduce(Node* node) { DisallowHeapAccess no_heap_access; + // Check if the output type is a singleton. In that case we already know the + // result value and can simply replace the node if it's eliminable. if (!NodeProperties::IsConstant(node) && NodeProperties::IsTyped(node) && - node->opcode() != IrOpcode::kFinishRegion) { - Node* constant = TryGetConstant(jsgraph(), node); - if (constant != nullptr) { - if (node->op()->HasProperty(Operator::kEliminatable)) { - RelaxEffectsAndControls(node); + node->op()->HasProperty(Operator::kEliminatable)) { + // TODO(v8:5303): We must not eliminate FinishRegion here. This special + // case can be removed once we have separate operators for value and + // effect regions. + if (node->opcode() == IrOpcode::kFinishRegion) return NoChange(); + // We can only constant-fold nodes here, that are known to not cause any + // side-effect, may it be a JavaScript observable side-effect or a possible + // eager deoptimization exit (i.e. {node} has an operator that doesn't have + // the Operator::kNoDeopt property). + Type upper = NodeProperties::GetType(node); + if (!upper.IsNone()) { + Node* replacement = nullptr; + if (upper.IsHeapConstant()) { + replacement = jsgraph()->Constant(upper.AsHeapConstant()->Ref()); + } else if (upper.Is(Type::MinusZero())) { + Factory* factory = jsgraph()->isolate()->factory(); + ObjectRef minus_zero(broker(), factory->minus_zero_value()); + replacement = jsgraph()->Constant(minus_zero); + } else if (upper.Is(Type::NaN())) { + replacement = jsgraph()->NaNConstant(); + } else if (upper.Is(Type::Null())) { + replacement = jsgraph()->NullConstant(); + } else if (upper.Is(Type::PlainNumber()) && upper.Min() == upper.Max()) { + replacement = jsgraph()->Constant(upper.Min()); + } else if (upper.Is(Type::Undefined())) { + replacement = jsgraph()->UndefinedConstant(); + } + if (replacement) { + // Make sure the node has a type. + if (!NodeProperties::IsTyped(replacement)) { + NodeProperties::SetType(replacement, upper); + } + ReplaceWithValue(node, replacement); + return Changed(replacement); } - ReplaceWithValue(node, constant, node, node); - return Changed(node); } } return NoChange(); diff --git a/src/compiler/js-graph.cc b/src/compiler/js-graph.cc index bd733ae413..beed7820b4 100644 --- a/src/compiler/js-graph.cc +++ b/src/compiler/js-graph.cc @@ -160,8 +160,6 @@ DEFINE_GETTER(NullConstant, HeapConstant(factory()->null_value())) DEFINE_GETTER(ZeroConstant, NumberConstant(0.0)) -DEFINE_GETTER(MinusZeroConstant, NumberConstant(-0.0)) - DEFINE_GETTER(OneConstant, NumberConstant(1.0)) DEFINE_GETTER(MinusOneConstant, NumberConstant(-1.0)) diff --git a/src/compiler/js-graph.h b/src/compiler/js-graph.h index f965451e35..83c81b1010 100644 --- a/src/compiler/js-graph.h +++ b/src/compiler/js-graph.h @@ -99,10 +99,9 @@ class V8_EXPORT_PRIVATE JSGraph : public MachineGraph { V(FalseConstant) \ V(NullConstant) \ V(ZeroConstant) \ - V(MinusZeroConstant) \ V(OneConstant) \ - V(MinusOneConstant) \ V(NaNConstant) \ + V(MinusOneConstant) \ V(EmptyStateValues) \ V(SingleDeadTypedStateValues) diff --git a/test/unittests/compiler/constant-folding-reducer-unittest.cc b/test/unittests/compiler/constant-folding-reducer-unittest.cc index 4f3d05173d..09cd26006b 100644 --- a/test/unittests/compiler/constant-folding-reducer-unittest.cc +++ b/test/unittests/compiler/constant-folding-reducer-unittest.cc @@ -80,12 +80,6 @@ class ConstantFoldingReducerTest : public TypedGraphTest { return reducer.Reduce(node); } - Node* UseValue(Node* node) { - Node* start = graph()->NewNode(common()->Start(1)); - Node* zero = graph()->NewNode(common()->NumberConstant(0)); - return graph()->NewNode(common()->Return(), zero, node, start, start); - } - SimplifiedOperatorBuilder* simplified() { return &simplified_; } JSHeapBroker* broker() { return &broker_; } @@ -97,26 +91,20 @@ class ConstantFoldingReducerTest : public TypedGraphTest { TEST_F(ConstantFoldingReducerTest, ParameterWithMinusZero) { { - Node* node = Parameter( - Type::Constant(broker(), factory()->minus_zero_value(), zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter( + Type::Constant(broker(), factory()->minus_zero_value(), zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(-0.0)); + EXPECT_THAT(r.replacement(), IsNumberConstant(-0.0)); } { - Node* node = Parameter(Type::MinusZero()); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::MinusZero())); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(-0.0)); + EXPECT_THAT(r.replacement(), IsNumberConstant(-0.0)); } { - Node* node = Parameter(Type::Union( + Reduction r = Reduce(Parameter(Type::Union( Type::MinusZero(), - Type::Constant(broker(), factory()->NewNumber(0), zone()), zone())); - UseValue(node); - Reduction r = Reduce(node); + Type::Constant(broker(), factory()->NewNumber(0), zone()), zone()))); EXPECT_FALSE(r.Changed()); } } @@ -124,18 +112,14 @@ TEST_F(ConstantFoldingReducerTest, ParameterWithMinusZero) { TEST_F(ConstantFoldingReducerTest, ParameterWithNull) { Handle null = factory()->null_value(); { - Node* node = Parameter(Type::Constant(broker(), null, zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::Constant(broker(), null, zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsHeapConstant(null)); + EXPECT_THAT(r.replacement(), IsHeapConstant(null)); } { - Node* node = Parameter(Type::Null()); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::Null())); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsHeapConstant(null)); + EXPECT_THAT(r.replacement(), IsHeapConstant(null)); } } @@ -145,62 +129,49 @@ TEST_F(ConstantFoldingReducerTest, ParameterWithNaN) { std::numeric_limits::signaling_NaN()}; TRACED_FOREACH(double, nan, kNaNs) { Handle constant = factory()->NewNumber(nan); - Node* node = Parameter(Type::Constant(broker(), constant, zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::Constant(broker(), constant, zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(IsNaN())); + EXPECT_THAT(r.replacement(), IsNumberConstant(IsNaN())); } { - Node* node = - Parameter(Type::Constant(broker(), factory()->nan_value(), zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce( + Parameter(Type::Constant(broker(), factory()->nan_value(), zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(IsNaN())); + EXPECT_THAT(r.replacement(), IsNumberConstant(IsNaN())); } { - Node* node = Parameter(Type::NaN()); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::NaN())); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(IsNaN())); + EXPECT_THAT(r.replacement(), IsNumberConstant(IsNaN())); } } TEST_F(ConstantFoldingReducerTest, ParameterWithPlainNumber) { TRACED_FOREACH(double, value, kFloat64Values) { Handle constant = factory()->NewNumber(value); - Node* node = Parameter(Type::Constant(broker(), constant, zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::Constant(broker(), constant, zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(value)); + EXPECT_THAT(r.replacement(), IsNumberConstant(value)); } TRACED_FOREACH(double, value, kIntegerValues) { - Node* node = Parameter(Type::Range(value, value, zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::Range(value, value, zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(value)); + EXPECT_THAT(r.replacement(), IsNumberConstant(value)); } } TEST_F(ConstantFoldingReducerTest, ParameterWithUndefined) { Handle undefined = factory()->undefined_value(); { - Node* node = Parameter(Type::Undefined()); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(Parameter(Type::Undefined())); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsUndefinedConstant()); + EXPECT_THAT(r.replacement(), IsHeapConstant(undefined)); } { - Node* node = Parameter(Type::Constant(broker(), undefined, zone())); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = + Reduce(Parameter(Type::Constant(broker(), undefined, zone()))); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsUndefinedConstant()); + EXPECT_THAT(r.replacement(), IsHeapConstant(undefined)); } } @@ -229,11 +200,9 @@ TEST_F(ConstantFoldingReducerTest, ToBooleanWithFalsish) { zone()), zone()), 0); - Node* node = graph()->NewNode(simplified()->ToBoolean(), input); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(graph()->NewNode(simplified()->ToBoolean(), input)); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsFalseConstant()); + EXPECT_THAT(r.replacement(), IsFalseConstant()); } TEST_F(ConstantFoldingReducerTest, ToBooleanWithTruish) { @@ -243,20 +212,16 @@ TEST_F(ConstantFoldingReducerTest, ToBooleanWithTruish) { Type::Union(Type::DetectableReceiver(), Type::Symbol(), zone()), zone()), 0); - Node* node = graph()->NewNode(simplified()->ToBoolean(), input); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(graph()->NewNode(simplified()->ToBoolean(), input)); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsTrueConstant()); + EXPECT_THAT(r.replacement(), IsTrueConstant()); } TEST_F(ConstantFoldingReducerTest, ToBooleanWithNonZeroPlainNumber) { Node* input = Parameter(Type::Range(1, V8_INFINITY, zone()), 0); - Node* node = graph()->NewNode(simplified()->ToBoolean(), input); - Node* use_value = UseValue(node); - Reduction r = Reduce(node); + Reduction r = Reduce(graph()->NewNode(simplified()->ToBoolean(), input)); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(use_value->InputAt(1), IsTrueConstant()); + EXPECT_THAT(r.replacement(), IsTrueConstant()); } } // namespace constant_folding_reducer_unittest