[turbofan] Fix overly aggressive dead code elimination.

When we eliminate nodes during truncation analysis that have no value
uses, we must make sure that we do not eliminate speculative number
operations that would have side effects depending on the inputs, i.e.
for example a SpeculativeNumberMultiply(x,y) does ToNumber(x) and
ToNumber(y) first, so if either x or y could throw an exception during
ToNumber conversion, we must not eliminate the multiplication, even if
it has no value uses (some later pass may kill the actual machine
multiplication, but the checks on the inputs have to remain still).
So we check whether both x and y are PlainPrimitive, i.e. neither
Receiver nor Symbol, which could raise exceptions for ToNumber, and
only in that case we propagate the "unusedness" of the node to its
inputs.

This also uncovered a bug with the type of Dead, which must be None,
as this represents an impossible value, so we had to fix that too.

Also the dead code removal will not work correctly for constants (i.e.
pure nodes with no value inputs), as those might be cached and hence
we might resurrect them for an unrelated node lowering during
SimplifiedLowering and only later kill the actual node (replacing its
uses with Dead), which would then also replace the new use with Dead.
So that was fixed as well. This shouldn't change anything for the
result, as unused constants automagically disappear from the graph later
on anyways.

R=yangguo@chromium.org
BUG=chromium:631318

Review-Url: https://codereview.chromium.org/2182003002
Cr-Commit-Position: refs/heads/master@{#38038}

src/compiler/simplified-lowering.cc

@@ -1075,7 +1075,13 @@ class RepresentationSelector {
 
   void VisitSpeculativeAdditiveOp(Node* node, Truncation truncation,
                                   SimplifiedLowering* lowering) {
-    if (truncation.IsUnused()) return VisitUnused(node);
+    // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we can
+    // only eliminate an unused speculative number operation if we know that
+    // the inputs are PlainPrimitive, which excludes everything that's might
+    // have side effects or throws during a ToNumber conversion.
+    if (BothInputsAre(node, Type::PlainPrimitive())) {
+      if (truncation.IsUnused()) return VisitUnused(node);
+    }
     if (BothInputsAre(node, type_cache_.kSigned32OrMinusZero) &&
         NodeProperties::GetType(node)->Is(Type::Signed32())) {
       // int32 + int32 = int32   ==>   signed Int32Add/Sub
@@ -1136,8 +1142,15 @@ class RepresentationSelector {
     // Unconditionally eliminate unused pure nodes (only relevant if there's
     // a pure operation in between two effectful ones, where the last one
     // is unused).
-    if (node->op()->HasProperty(Operator::kPure) && truncation.IsUnused()) {
-      return VisitUnused(node);
+    // Note: We must not do this for constants, as they are cached and we
+    // would thus kill the cached {node} during lowering (i.e. replace all
+    // uses with Dead), but at that point some node lowering might have
+    // already taken the constant {node} from the cache (while it was in
+    // a sane state still) and we would afterwards replace that use with
+    // Dead as well.
+    if (node->op()->ValueInputCount() > 0 &&
+        node->op()->HasProperty(Operator::kPure)) {
+      if (truncation.IsUnused()) return VisitUnused(node);
     }
     switch (node->opcode()) {
       //------------------------------------------------------------------
@@ -1261,7 +1274,13 @@ class RepresentationSelector {
       case IrOpcode::kSpeculativeNumberLessThan:
       case IrOpcode::kSpeculativeNumberLessThanOrEqual:
       case IrOpcode::kSpeculativeNumberEqual: {
-        if (truncation.IsUnused()) return VisitUnused(node);
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
         // Number comparisons reduce to integer comparisons for integer inputs.
         if (TypeOf(node->InputAt(0))->Is(Type::Unsigned32()) &&
             TypeOf(node->InputAt(1))->Is(Type::Unsigned32())) {
@@ -1316,7 +1335,13 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kSpeculativeNumberMultiply: {
-        if (truncation.IsUnused()) return VisitUnused(node);
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
         if (BothInputsAre(node, Type::Integral32()) &&
             (NodeProperties::GetType(node)->Is(Type::Signed32()) ||
              NodeProperties::GetType(node)->Is(Type::Unsigned32()) ||
@@ -1391,7 +1416,13 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kSpeculativeNumberDivide: {
-        if (truncation.IsUnused()) return VisitUnused(node);
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
         if (BothInputsAreUnsigned32(node) && truncation.IsUsedAsWord32()) {
           // => unsigned Uint32Div
           VisitWord32TruncatingBinop(node);
@@ -1497,7 +1528,13 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kSpeculativeNumberModulus: {
-        if (truncation.IsUnused()) return VisitUnused(node);
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
         if (BothInputsAreUnsigned32(node) && truncation.IsUsedAsWord32()) {
           // => unsigned Uint32Mod
           VisitWord32TruncatingBinop(node);
@@ -1611,7 +1648,13 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kSpeculativeNumberShiftLeft: {
-        if (truncation.IsUnused()) return VisitUnused(node);
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
         if (BothInputsAre(node, Type::NumberOrOddball())) {
           Type* rhs_type = GetUpperBound(node->InputAt(1));
           VisitBinop(node, UseInfo::TruncatingWord32(),

src/compiler/typer.cc

@@ -810,9 +810,7 @@ Type* Typer::Visitor::TypeProjection(Node* node) {
   return Type::Any();
 }
 
-
-Type* Typer::Visitor::TypeDead(Node* node) { return Type::Any(); }
-
+Type* Typer::Visitor::TypeDead(Node* node) { return Type::None(); }
 
 // JS comparison operators.
 

test/mjsunit/regress/regress-crbug-631318-1.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x < x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-10.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x << x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-11.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x >> x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-12.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x >>> x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-13.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x & x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-14.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x | x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-15.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x ^ x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-2.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x > x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-3.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x >= x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-4.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x <= x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-5.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x + x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-6.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x / x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-7.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x * x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-8.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x % x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));

test/mjsunit/regress/regress-crbug-631318-9.js

@@ -0,0 +1,14 @@
+// Copyright 2016 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(x) { return x - x; }
+foo(1);
+foo(2);
+
+function bar(x) { foo(x); }
+%OptimizeFunctionOnNextCall(bar);
+
+assertThrows(() => bar(Symbol.toPrimitive));