[turbofan] Don't mix element accesses with incompatible representations.

Due to speculative optimizations, the compiler can run into situations
where it's asked perform impossible operations, like loading a tagged
element as a float64 instead. All of this is guaranteed to be in dead
code (unless there's a bug), but leads to confusion and violates
assumptions in the compiler (that make perfect sense for code that is
not dead). So teach LoadElimination not to mix up element accesses with
incompatible representations.

BUG=chromium:719479
R=jarin@chromium.org

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

src/compiler/load-elimination.cc

@@ -195,13 +195,23 @@ void LoadElimination::AbstractChecks::Print() const {
   }
 }
 
-Node* LoadElimination::AbstractElements::Lookup(Node* object,
-                                                Node* index) const {
+namespace {
+
+bool IsCompatible(MachineRepresentation r1, MachineRepresentation r2) {
+  if (r1 == r2) return true;
+  return IsAnyTagged(r1) && IsAnyTagged(r2);
+}
+
+}  // namespace
+
+Node* LoadElimination::AbstractElements::Lookup(
+    Node* object, Node* index, MachineRepresentation representation) const {
   for (Element const element : elements_) {
     if (element.object == nullptr) continue;
     DCHECK_NOT_NULL(element.index);
     DCHECK_NOT_NULL(element.value);
-    if (MustAlias(object, element.object) && MustAlias(index, element.index)) {
+    if (MustAlias(object, element.object) && MustAlias(index, element.index) &&
+        IsCompatible(representation, element.representation)) {
       return element.value;
     }
   }
@@ -470,22 +480,26 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillMaps(
   return this;
 }
 
-Node* LoadElimination::AbstractState::LookupElement(Node* object,
-                                                    Node* index) const {
+Node* LoadElimination::AbstractState::LookupElement(
+    Node* object, Node* index, MachineRepresentation representation) const {
   if (this->elements_) {
-    return this->elements_->Lookup(object, index);
+    return this->elements_->Lookup(object, index, representation);
   }
   return nullptr;
 }
 
 LoadElimination::AbstractState const*
 LoadElimination::AbstractState::AddElement(Node* object, Node* index,
-                                           Node* value, Zone* zone) const {
+                                           Node* value,
+                                           MachineRepresentation representation,
+                                           Zone* zone) const {
   AbstractState* that = new (zone) AbstractState(*this);
   if (that->elements_) {
-    that->elements_ = that->elements_->Extend(object, index, value, zone);
+    that->elements_ =
+        that->elements_->Extend(object, index, value, representation, zone);
   } else {
-    that->elements_ = new (zone) AbstractElements(object, index, value, zone);
+    that->elements_ =
+        new (zone) AbstractElements(object, index, value, representation, zone);
   }
   return that;
 }
@@ -823,7 +837,8 @@ Reduction LoadElimination::ReduceLoadElement(Node* node) {
     case MachineRepresentation::kTaggedSigned:
     case MachineRepresentation::kTaggedPointer:
     case MachineRepresentation::kTagged:
-      if (Node* replacement = state->LookupElement(object, index)) {
+      if (Node* replacement = state->LookupElement(
+              object, index, access.machine_type.representation())) {
         // Make sure we don't resurrect dead {replacement} nodes.
         if (!replacement->IsDead()) {
           // We might need to guard the {replacement} if the type of the
@@ -838,7 +853,8 @@ Reduction LoadElimination::ReduceLoadElement(Node* node) {
           return Replace(replacement);
         }
       }
-      state = state->AddElement(object, index, node, zone());
+      state = state->AddElement(object, index, node,
+                                access.machine_type.representation(), zone());
       return UpdateState(node, state);
   }
   return NoChange();
@@ -852,7 +868,8 @@ Reduction LoadElimination::ReduceStoreElement(Node* node) {
   Node* const effect = NodeProperties::GetEffectInput(node);
   AbstractState const* state = node_states_.Get(effect);
   if (state == nullptr) return NoChange();
-  Node* const old_value = state->LookupElement(object, index);
+  Node* const old_value =
+      state->LookupElement(object, index, access.machine_type.representation());
   if (old_value == new_value) {
     // This store is fully redundant.
     return Replace(effect);
@@ -880,7 +897,8 @@ Reduction LoadElimination::ReduceStoreElement(Node* node) {
     case MachineRepresentation::kTaggedSigned:
     case MachineRepresentation::kTaggedPointer:
     case MachineRepresentation::kTagged:
-      state = state->AddElement(object, index, new_value, zone());
+      state = state->AddElement(object, index, new_value,
+                                access.machine_type.representation(), zone());
       break;
   }
   return UpdateState(node, state);

src/compiler/load-elimination.h

@@ -8,6 +8,7 @@
 #include "src/base/compiler-specific.h"
 #include "src/compiler/graph-reducer.h"
 #include "src/globals.h"
+#include "src/machine-type.h"
 #include "src/zone/zone-handle-set.h"
 
 namespace v8 {
@@ -78,19 +79,23 @@ class V8_EXPORT_PRIVATE LoadElimination final
         elements_[i] = Element();
       }
     }
-    AbstractElements(Node* object, Node* index, Node* value, Zone* zone)
+    AbstractElements(Node* object, Node* index, Node* value,
+                     MachineRepresentation representation, Zone* zone)
         : AbstractElements(zone) {
-      elements_[next_index_++] = Element(object, index, value);
+      elements_[next_index_++] = Element(object, index, value, representation);
     }
 
     AbstractElements const* Extend(Node* object, Node* index, Node* value,
+                                   MachineRepresentation representation,
                                    Zone* zone) const {
       AbstractElements* that = new (zone) AbstractElements(*this);
-      that->elements_[that->next_index_] = Element(object, index, value);
+      that->elements_[that->next_index_] =
+          Element(object, index, value, representation);
       that->next_index_ = (that->next_index_ + 1) % arraysize(elements_);
       return that;
     }
-    Node* Lookup(Node* object, Node* index) const;
+    Node* Lookup(Node* object, Node* index,
+                 MachineRepresentation representation) const;
     AbstractElements const* Kill(Node* object, Node* index, Zone* zone) const;
     bool Equals(AbstractElements const* that) const;
     AbstractElements const* Merge(AbstractElements const* that,
@@ -101,12 +106,17 @@ class V8_EXPORT_PRIVATE LoadElimination final
    private:
     struct Element {
       Element() {}
-      Element(Node* object, Node* index, Node* value)
-          : object(object), index(index), value(value) {}
+      Element(Node* object, Node* index, Node* value,
+              MachineRepresentation representation)
+          : object(object),
+            index(index),
+            value(value),
+            representation(representation) {}
 
       Node* object = nullptr;
       Node* index = nullptr;
       Node* value = nullptr;
+      MachineRepresentation representation = MachineRepresentation::kNone;
     };
 
     Element elements_[kMaxTrackedElements];
@@ -224,10 +234,12 @@ class V8_EXPORT_PRIVATE LoadElimination final
     Node* LookupField(Node* object, size_t index) const;
 
     AbstractState const* AddElement(Node* object, Node* index, Node* value,
+                                    MachineRepresentation representation,
                                     Zone* zone) const;
     AbstractState const* KillElement(Node* object, Node* index,
                                      Zone* zone) const;
-    Node* LookupElement(Node* object, Node* index) const;
+    Node* LookupElement(Node* object, Node* index,
+                        MachineRepresentation representation) const;
 
     AbstractState const* AddCheck(Node* node, Zone* zone) const;
     Node* LookupCheck(Node* node) const;

test/mjsunit/regress/regress-crbug-719479.js

+// Copyright 2017 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 baz(a, b) {
+  for (var i = 0; i < a.length; i++) {
+    if (a[i], b[i]) return false;
+  }
+}
+function bar(expected, found) {
+  if (!baz(found, expected)) {
+  }
+};
+bar([{}, 6, NaN], [1.8, , NaN]);
+function foo() {
+  var a = [1,2,3,4];
+  bar(a.length, a.length);
+}
+foo();
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();