[turbofan] Restrict types in load elimination.

In simplified numbering, we make sanity checks based on types (e.g.,
NumberSubtract should take numbers as inputs), but this can be
violated if optimization passes make types less precise.

In this CL, we fix load elimination to make sure that types are
smaller in the store -> load elimination by taking an intersection
of the load's type with the store value's type and inserting a guard
with that type. Note that the load type comes from type feedback, so
it can be disjoint from the stored value type (in that case, this
must be dead code because the map chack for the load should prevent
us from using the stored value).

BUG=chromium:599412
LOG=n

Review URL: https://codereview.chromium.org/1857133003

Cr-Commit-Position: refs/heads/master@{#35259}

src/compiler/load-elimination.cc

@@ -4,8 +4,11 @@
 
 #include "src/compiler/load-elimination.h"
 
+#include "src/compiler/common-operator.h"
+#include "src/compiler/graph.h"
 #include "src/compiler/node-properties.h"
 #include "src/compiler/simplified-operator.h"
+#include "src/types.h"
 
 namespace v8 {
 namespace internal {
@@ -13,7 +16,6 @@ namespace compiler {
 
 LoadElimination::~LoadElimination() {}
 
-
 Reduction LoadElimination::Reduce(Node* node) {
   switch (node->opcode()) {
     case IrOpcode::kLoadField:
@@ -24,7 +26,6 @@ Reduction LoadElimination::Reduce(Node* node) {
   return NoChange();
 }
 
-
 Reduction LoadElimination::ReduceLoadField(Node* node) {
   DCHECK_EQ(IrOpcode::kLoadField, node->opcode());
   FieldAccess const access = FieldAccessOf(node->op());
@@ -45,8 +46,22 @@ Reduction LoadElimination::ReduceLoadField(Node* node) {
         if (access == FieldAccessOf(effect->op())) {
           if (object == NodeProperties::GetValueInput(effect, 0)) {
             Node* const value = NodeProperties::GetValueInput(effect, 1);
-            ReplaceWithValue(node, value);
-            return Replace(value);
+            Type* stored_value_type = NodeProperties::GetType(value);
+            Type* load_type = NodeProperties::GetType(node);
+            // Make sure the replacement's type is a subtype of the node's
+            // type. Otherwise we could confuse optimizations that were
+            // based on the original type.
+            if (stored_value_type->Is(load_type)) {
+              ReplaceWithValue(node, value);
+              return Replace(value);
+            } else {
+              Node* renamed = graph()->NewNode(
+                  common()->Guard(Type::Intersect(stored_value_type, load_type,
+                                                  graph()->zone())),
+                  value, NodeProperties::GetControlInput(node));
+              ReplaceWithValue(node, renamed);
+              return Replace(renamed);
+            }
           }
           // TODO(turbofan): Alias analysis to the rescue?
           return NoChange();

src/compiler/load-elimination.h

@@ -11,15 +11,25 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+class CommonOperatorBuilder;
+class Graph;
+
 class LoadElimination final : public AdvancedReducer {
  public:
-  explicit LoadElimination(Editor* editor) : AdvancedReducer(editor) {}
+  explicit LoadElimination(Editor* editor, Graph* graph,
+                           CommonOperatorBuilder* common)
+      : AdvancedReducer(editor), graph_(graph), common_(common) {}
   ~LoadElimination() final;
 
   Reduction Reduce(Node* node) final;
 
  private:
+  CommonOperatorBuilder* common() const { return common_; }
+  Graph* graph() { return graph_; }
+
   Reduction ReduceLoadField(Node* node);
+  Graph* graph_;
+  CommonOperatorBuilder* common_;
 };
 
 }  // namespace compiler

src/compiler/pipeline.cc

@@ -628,7 +628,8 @@ struct TypedLoweringPhase {
     JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
     DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(),
                                               data->common());
-    LoadElimination load_elimination(&graph_reducer);
+    LoadElimination load_elimination(&graph_reducer, data->graph(),
+                                     data->common());
     JSBuiltinReducer builtin_reducer(&graph_reducer, data->jsgraph());
     MaybeHandle<LiteralsArray> literals_array =
         data->info()->is_native_context_specializing()

test/mjsunit/mjsunit.status

@@ -151,6 +151,7 @@
 
   # TODO(turbofan): The escape analysis needs some investigation.
   'compiler/escape-analysis-deopt-5': [PASS, NO_VARIANTS],
+  'compiler/escape-analysis-9': [PASS, NO_VARIANTS],
 
   ##############################################################################
   # Too slow in debug mode with --stress-opt mode.

test/mjsunit/regress/regress-599412.js

+// 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 h(a) {
+  if (!a) return false;
+  print();
+}
+
+function g(a) { return a.length; }
+g('0');
+g('1');
+
+function f() {
+  h(g([]));
+}
+
+f();
+%OptimizeFunctionOnNextCall(f);
+f();

test/unittests/compiler/load-elimination-unittest.cc

@@ -12,30 +12,33 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-class LoadEliminationTest : public GraphTest {
+class LoadEliminationTest : public TypedGraphTest {
  public:
-  LoadEliminationTest() : GraphTest(3), simplified_(zone()) {}
+  LoadEliminationTest()
+      : TypedGraphTest(3), common_(zone()), simplified_(zone()) {}
   ~LoadEliminationTest() override {}
 
  protected:
   Reduction Reduce(Node* node) {
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(zone(), graph());
-    LoadElimination reducer(&graph_reducer);
+    LoadElimination reducer(&graph_reducer, graph(), common());
     return reducer.Reduce(node);
   }
 
   SimplifiedOperatorBuilder* simplified() { return &simplified_; }
+  CommonOperatorBuilder* common() { return &common_; }
 
  private:
+  CommonOperatorBuilder common_;
   SimplifiedOperatorBuilder simplified_;
 };
 
 
 TEST_F(LoadEliminationTest, LoadFieldWithStoreField) {
-  Node* object1 = Parameter(0);
-  Node* object2 = Parameter(1);
-  Node* value = Parameter(2);
+  Node* object1 = Parameter(Type::Any(), 0);
+  Node* object2 = Parameter(Type::Any(), 1);
+  Node* value = Parameter(Type::Any(), 2);
   Node* effect = graph()->start();
   Node* control = graph()->start();