Reland "[turbofan] Clean up ConstantFoldingReducer"

This is a reland of 2c834c53,
in which node replacement was too aggressive.

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}

Change-Id: I5d306092dde4119629af4c5e7e424a0e9a14310d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2106193
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66742}

src/compiler/constant-folding-reducer.cc

@@ -11,6 +11,36 @@ 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) {}
@@ -19,44 +49,14 @@ 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->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);
-      }
+      node->op()->HasProperty(Operator::kEliminatable) &&
+      node->opcode() != IrOpcode::kFinishRegion) {
+    Node* constant = TryGetConstant(jsgraph(), node);
+    if (constant != nullptr) {
+      DCHECK_EQ(node->op()->ControlOutputCount(), 0);
+      ReplaceWithValue(node, constant);
+      return Replace(constant);
     }
   }
   return NoChange();

src/compiler/js-graph.cc

@@ -160,6 +160,8 @@ 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,9 +99,10 @@ class V8_EXPORT_PRIVATE JSGraph : public MachineGraph {
   V(FalseConstant)                                \
   V(NullConstant)                                 \
   V(ZeroConstant)                                 \
+  V(MinusZeroConstant)                            \
   V(OneConstant)                                  \
-  V(NaNConstant)                                  \
   V(MinusOneConstant)                             \
+  V(NaNConstant)                                  \
   V(EmptyStateValues)                             \
   V(SingleDeadTypedStateValues)
 

src/compiler/operator.h

@@ -48,9 +48,9 @@ class V8_EXPORT_PRIVATE Operator : public NON_EXPORTED_BASE(ZoneObject) {
     kNoThrow = 1 << 5,      // Can never generate an exception.
     kNoDeopt = 1 << 6,      // Can never generate an eager deoptimization exit.
     kFoldable = kNoRead | kNoWrite,
-    kKontrol = kNoDeopt | kFoldable | kNoThrow,
     kEliminatable = kNoDeopt | kNoWrite | kNoThrow,
-    kPure = kNoDeopt | kNoRead | kNoWrite | kNoThrow | kIdempotent
+    kKontrol = kNoDeopt | kFoldable | kNoThrow,
+    kPure = kKontrol | kIdempotent
   };
 
 // List of all bits, for the visualizer.

test/mjsunit/compiler/regress-1061678.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+Array.prototype[10] = 2;
+
+function foo() {
+  try {
+    [].forEach();
+  } catch (e) {
+  }
+};
+
+%PrepareFunctionForOptimization(foo);
+foo();
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();

test/mjsunit/compiler/regress-1061803.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+function foo() {
+  return arguments[1][0] === arguments[0];
+}
+
+%PrepareFunctionForOptimization(foo);
+assertFalse(foo(0, 0));
+assertFalse(foo(0, 0));
+%OptimizeFunctionOnNextCall(foo);
+assertThrows(foo, TypeError);

test/unittests/compiler/constant-folding-reducer-unittest.cc

@@ -80,6 +80,12 @@ 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_; }
 
@@ -91,20 +97,26 @@ class ConstantFoldingReducerTest : public TypedGraphTest {
 
 TEST_F(ConstantFoldingReducerTest, ParameterWithMinusZero) {
   {
-    Reduction r = Reduce(Parameter(
-        Type::Constant(broker(), factory()->minus_zero_value(), zone())));
+    Node* node = Parameter(
+        Type::Constant(broker(), factory()->minus_zero_value(), zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(-0.0));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(-0.0));
   }
   {
-    Reduction r = Reduce(Parameter(Type::MinusZero()));
+    Node* node = Parameter(Type::MinusZero());
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(-0.0));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(-0.0));
   }
   {
-    Reduction r = Reduce(Parameter(Type::Union(
+    Node* node = Parameter(Type::Union(
         Type::MinusZero(),
-        Type::Constant(broker(), factory()->NewNumber(0), zone()), zone())));
+        Type::Constant(broker(), factory()->NewNumber(0), zone()), zone()));
+    UseValue(node);
+    Reduction r = Reduce(node);
     EXPECT_FALSE(r.Changed());
   }
 }
@@ -112,14 +124,18 @@ TEST_F(ConstantFoldingReducerTest, ParameterWithMinusZero) {
 TEST_F(ConstantFoldingReducerTest, ParameterWithNull) {
   Handle<HeapObject> null = factory()->null_value();
   {
-    Reduction r = Reduce(Parameter(Type::Constant(broker(), null, zone())));
+    Node* node = Parameter(Type::Constant(broker(), null, zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsHeapConstant(null));
+    EXPECT_THAT(use_value->InputAt(1), IsHeapConstant(null));
   }
   {
-    Reduction r = Reduce(Parameter(Type::Null()));
+    Node* node = Parameter(Type::Null());
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsHeapConstant(null));
+    EXPECT_THAT(use_value->InputAt(1), IsHeapConstant(null));
   }
 }
 
@@ -129,49 +145,62 @@ TEST_F(ConstantFoldingReducerTest, ParameterWithNaN) {
                           std::numeric_limits<double>::signaling_NaN()};
   TRACED_FOREACH(double, nan, kNaNs) {
     Handle<Object> constant = factory()->NewNumber(nan);
-    Reduction r = Reduce(Parameter(Type::Constant(broker(), constant, zone())));
+    Node* node = Parameter(Type::Constant(broker(), constant, zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(IsNaN()));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(IsNaN()));
   }
   {
-    Reduction r = Reduce(
-        Parameter(Type::Constant(broker(), factory()->nan_value(), zone())));
+    Node* node =
+        Parameter(Type::Constant(broker(), factory()->nan_value(), zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(IsNaN()));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(IsNaN()));
   }
   {
-    Reduction r = Reduce(Parameter(Type::NaN()));
+    Node* node = Parameter(Type::NaN());
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(IsNaN()));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(IsNaN()));
   }
 }
 
 TEST_F(ConstantFoldingReducerTest, ParameterWithPlainNumber) {
   TRACED_FOREACH(double, value, kFloat64Values) {
     Handle<Object> constant = factory()->NewNumber(value);
-    Reduction r = Reduce(Parameter(Type::Constant(broker(), constant, zone())));
+    Node* node = Parameter(Type::Constant(broker(), constant, zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(value));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(value));
   }
   TRACED_FOREACH(double, value, kIntegerValues) {
-    Reduction r = Reduce(Parameter(Type::Range(value, value, zone())));
+    Node* node = Parameter(Type::Range(value, value, zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsNumberConstant(value));
+    EXPECT_THAT(use_value->InputAt(1), IsNumberConstant(value));
   }
 }
 
 TEST_F(ConstantFoldingReducerTest, ParameterWithUndefined) {
   Handle<HeapObject> undefined = factory()->undefined_value();
   {
-    Reduction r = Reduce(Parameter(Type::Undefined()));
+    Node* node = Parameter(Type::Undefined());
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsHeapConstant(undefined));
+    EXPECT_THAT(use_value->InputAt(1), IsUndefinedConstant());
   }
   {
-    Reduction r =
-        Reduce(Parameter(Type::Constant(broker(), undefined, zone())));
+    Node* node = Parameter(Type::Constant(broker(), undefined, zone()));
+    Node* use_value = UseValue(node);
+    Reduction r = Reduce(node);
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsHeapConstant(undefined));
+    EXPECT_THAT(use_value->InputAt(1), IsUndefinedConstant());
   }
 }
 
@@ -200,9 +229,11 @@ TEST_F(ConstantFoldingReducerTest, ToBooleanWithFalsish) {
               zone()),
           zone()),
       0);
-  Reduction r = Reduce(graph()->NewNode(simplified()->ToBoolean(), input));
+  Node* node = graph()->NewNode(simplified()->ToBoolean(), input);
+  Node* use_value = UseValue(node);
+  Reduction r = Reduce(node);
   ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(), IsFalseConstant());
+  EXPECT_THAT(use_value->InputAt(1), IsFalseConstant());
 }
 
 TEST_F(ConstantFoldingReducerTest, ToBooleanWithTruish) {
@@ -212,16 +243,20 @@ TEST_F(ConstantFoldingReducerTest, ToBooleanWithTruish) {
           Type::Union(Type::DetectableReceiver(), Type::Symbol(), zone()),
           zone()),
       0);
-  Reduction r = Reduce(graph()->NewNode(simplified()->ToBoolean(), input));
+  Node* node = graph()->NewNode(simplified()->ToBoolean(), input);
+  Node* use_value = UseValue(node);
+  Reduction r = Reduce(node);
   ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(), IsTrueConstant());
+  EXPECT_THAT(use_value->InputAt(1), IsTrueConstant());
 }
 
 TEST_F(ConstantFoldingReducerTest, ToBooleanWithNonZeroPlainNumber) {
   Node* input = Parameter(Type::Range(1, V8_INFINITY, zone()), 0);
-  Reduction r = Reduce(graph()->NewNode(simplified()->ToBoolean(), input));
+  Node* node = graph()->NewNode(simplified()->ToBoolean(), input);
+  Node* use_value = UseValue(node);
+  Reduction r = Reduce(node);
   ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(), IsTrueConstant());
+  EXPECT_THAT(use_value->InputAt(1), IsTrueConstant());
 }
 
 }  // namespace constant_folding_reducer_unittest