Weaken representation tracking assertion in load elimination

Feedback pollution can create situations in which we statically see stores to the same field with incompatible representations; dynamically this should be impossible for a single TurboFan compilation unit. Instead of failing an assertion we produce Unreachable nodes.

R=tebbi@chromium.org

Bug: chromium:967434 chromium:967506
Change-Id: Id549ec84f28b4fed2d2e5ef05b40b48bc5b30e97
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1632169
Commit-Queue: Georg Schmid <gsps@google.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61894}

src/compiler/load-elimination.cc

@@ -924,11 +924,17 @@ Reduction LoadElimination::ReduceStoreField(Node* node,
           state->LookupField(object, field_index, constness);
 
       if (lookup_result) {
-        CHECK(lookup_result->name.is_null() ||
-              IsCompatible(representation, lookup_result->representation));
-        if (constness == PropertyConstness::kConst) {
-          // At runtime, we should never see two consecutive const stores, but
-          // we might see such (unreachable) code statically.
+        // At runtime, we should never encounter
+        // - any store replacing existing info with a different, incompatible
+        //   representation, nor
+        // - two consecutive const stores.
+        // However, we may see such code statically, so we guard against
+        // executing it by emitting Unreachable.
+        bool incompatible_representation =
+            !lookup_result->name.is_null() &&
+            !IsCompatible(representation, lookup_result->representation);
+        if (incompatible_representation ||
+            constness == PropertyConstness::kConst) {
           Node* control = NodeProperties::GetControlInput(node);
           Node* unreachable =
               graph()->NewNode(common()->Unreachable(), effect, control);

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

+// Copyright 2019 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 f1(h_also_eval) {
+  this.x = h_also_eval;
+}
+
+function f2(h, h_eval) {
+  var o = new f1(h());
+  // During the last call to f3 with g2 as an argument, this store is
+  // bi-morphic, including a version that refers to the old map (before
+  // the replacement of f1's prototype). As a result, during load elimination
+  // we see two stores with incompatible representations: One in the
+  // constructor, and one in the impossible branch of the bi-morphic store
+  // site.
+  o.x = h_eval;
+}
+
+function f3(h) {
+  f2(h, h());
+  %OptimizeFunctionOnNextCall(f2);
+  f2(h, h());
+}
+
+function g1() { return {}; };
+function g2() { return 4.2; };
+
+f3(g1);
+f3(g2);
+
+f3(g1);
+f1.prototype = {};
+f3(g2);