[turbofan] Only do value numbering when types are compatible.

At the moment, two NumberConstant nodes get different type even if their
value is the same because we always allocate a new heap number for
each number constant. This can lead to replacing a node with a node of
disjoint type in value numbering, which can result in incorrect code
down the line because of inconsistent types.

This fix makes sure that we only replace a node with a sub-type
node. Once we introduce a proper type for number constants, we can
move back to the intersection typing in value numbering.

Unfortunately, it is quite hard to write a repro for this because we cache NumberConstant nodes. We only throw away cached values that have too many conflicts (>5), so the test has to contain values that fall into the same bucket. That's where the magic floating point numbers in the test come from (they have the same low 8-bits of their hashes).

BUG=chromium:633497

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

src/compiler/value-numbering-reducer.cc

@@ -132,8 +132,17 @@ Reduction ValueNumberingReducer::Reduce(Node* node) {
         Type* entry_type = NodeProperties::GetType(entry);
         Type* node_type = NodeProperties::GetType(node);
         if (!entry_type->Is(node_type)) {
-          NodeProperties::SetType(
-              entry, Type::Intersect(entry_type, node_type, graph_zone()));
+          // Ideally, we would set an intersection of {entry_type} and
+          // {node_type} here. However, typing of NumberConstants assigns
+          // different types to constants with the same value (it creates
+          // a fresh heap number), which would make the intersection empty.
+          // To be safe, we use the smaller type if the types are comparable.
+          if (node_type->Is(entry_type)) {
+            NodeProperties::SetType(entry, node_type);
+          } else {
+            // Types are not comparable => do not replace.
+            return NoChange();
+          }
         }
       }
       return Replace(entry);

test/mjsunit/compiler/regress-633497.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 f(a) {
+  var x;
+  a = a|0;
+  var dummy;
+  if (a === 1) {
+    x = 277.5;
+  } else if (a === 2) {
+    x = 0;
+  } else {
+    dummy = 527.5;
+    dummy = 958.5;
+    dummy = 1143.5;
+    dummy = 1368.5;
+    dummy = 1558.5;
+    x = 277.5;
+  }
+  return +x;
+}
+
+f();
+f();
+%OptimizeFunctionOnNextCall(f);
+assertEquals(277.5, f());