Revert "[turbofan] Clean up ConstantFoldingReducer"

This reverts commit 2c834c53.

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 <tebbi@chromium.org>
> Commit-Queue: Georg Neis <neis@chromium.org>
> 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/+/2104892Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66717}

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();

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))

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)
 

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<HeapObject> 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<double>::signaling_NaN()};
   TRACED_FOREACH(double, nan, kNaNs) {
     Handle<Object> 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<Object> 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<HeapObject> 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