[turbofan] Fix dead value insertion in simplified lowering.

If type checks in simplified lowering produced dead value (i.e., of
type Type::None()), we have only propagated deadness along value
edges. With this CL, we also insert an Unreachable node after every
effectful node that produces dead value.

This is more consistent with dead code elimination, which also inserts
unreachable nodes after effectful nodes with value output None.

Bug: chromium:884052
Change-Id: Idcb168461f05f1811b2c9c16ab8ff179b259fbd3
Reviewed-on: https://chromium-review.googlesource.com/1228125
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55987}

src/compiler/simplified-lowering.cc

@@ -1488,6 +1488,30 @@ class RepresentationSelector {
     return;
   }
 
+  void InsertUnreachableIfNecessary(Node* node) {
+    DCHECK(lower());
+    // If the node is effectful and it produces an impossible value, then we
+    // insert Unreachable node after it.
+    if (node->op()->ValueOutputCount() > 0 &&
+        node->op()->EffectOutputCount() > 0 &&
+        node->opcode() != IrOpcode::kUnreachable && TypeOf(node).IsNone()) {
+      Node* control = node->op()->ControlOutputCount() > 0
+                          ? node
+                          : NodeProperties::GetControlInput(node, 0);
+
+      Node* unreachable =
+          graph()->NewNode(common()->Unreachable(), node, control);
+
+      // Insert unreachable node and replace all the effect uses of the {node}
+      // with the new unreachable node.
+      for (Edge edge : node->use_edges()) {
+        if (NodeProperties::IsEffectEdge(edge) && edge.from() != unreachable) {
+          edge.UpdateTo(unreachable);
+        }
+      }
+    }
+  }
+
   // Dispatching routine for visiting the node {node} with the usage {use}.
   // Depending on the operator, propagate new usage info to the inputs.
   void VisitNode(Node* node, Truncation truncation,
@@ -1502,9 +1526,12 @@ class RepresentationSelector {
     // 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);
+        node->op()->HasProperty(Operator::kPure) && truncation.IsUnused()) {
+      return VisitUnused(node);
     }
+
+    if (lower()) InsertUnreachableIfNecessary(node);
+
     switch (node->opcode()) {
       //------------------------------------------------------------------
       // Common operators.

test/mjsunit/compiler/regress-884052.js

@@ -0,0 +1,16 @@
+// Copyright 2018 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() {
+  var a = new Array(2);
+  for (var i = 1; i > -1; i = i - 2) {
+    if (i < a.length) a = new Array(i);
+  }
+}
+
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();