[turbofan] Properly rename receiver on CheckHeapObject.

We need to rename the receiver on CheckHeapObject, because we
don't canonicalize numbers in SignedSmall range, and thus we
the representation selection can hand out TaggedSigned values
for receiver uses, even though we checked for TaggedPointerness
first.

Note that this is rather hacky and just intended to fix the bug
ASAP. We need to think about how to deal with representations in
earlier compilation stages.

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

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

src/compiler/js-builtin-reducer.cc

@@ -107,6 +107,19 @@ JSBuiltinReducer::JSBuiltinReducer(Editor* editor, JSGraph* jsgraph,
 
 namespace {
 
+// TODO(turbofan): Shall we move this to the NodeProperties? Or some (untyped)
+// alias analyzer?
+bool IsSame(Node* a, Node* b) {
+  if (a == b) {
+    return true;
+  } else if (a->opcode() == IrOpcode::kCheckHeapObject) {
+    return IsSame(a->InputAt(0), b);
+  } else if (b->opcode() == IrOpcode::kCheckHeapObject) {
+    return IsSame(a, b->InputAt(0));
+  }
+  return false;
+}
+
 MaybeHandle<Map> GetMapWitness(Node* node) {
   Node* receiver = NodeProperties::GetValueInput(node, 1);
   Node* effect = NodeProperties::GetEffectInput(node);
@@ -114,7 +127,7 @@ MaybeHandle<Map> GetMapWitness(Node* node) {
   // for the {receiver}, and if so use that map for the lowering below.
   for (Node* dominator = effect;;) {
     if (dominator->opcode() == IrOpcode::kCheckMaps &&
-        dominator->InputAt(0) == receiver) {
+        IsSame(dominator->InputAt(0), receiver)) {
       if (dominator->op()->ValueInputCount() == 2) {
         HeapObjectMatcher m(dominator->InputAt(1));
         if (m.HasValue()) return Handle<Map>::cast(m.Value());
@@ -331,7 +344,7 @@ bool HasInstanceTypeWitness(Node* receiver, Node* effect,
                             InstanceType instance_type) {
   for (Node* dominator = effect;;) {
     if (dominator->opcode() == IrOpcode::kCheckMaps &&
-        dominator->InputAt(0) == receiver) {
+        IsSame(dominator->InputAt(0), receiver)) {
       // Check if all maps have the given {instance_type}.
       for (int i = 1; i < dominator->op()->ValueInputCount(); ++i) {
         Node* const map = NodeProperties::GetValueInput(dominator, i);
@@ -922,7 +935,7 @@ Node* GetStringWitness(Node* node) {
   // the lowering below.
   for (Node* dominator = effect;;) {
     if (dominator->opcode() == IrOpcode::kCheckString &&
-        dominator->InputAt(0) == receiver) {
+        IsSame(dominator->InputAt(0), receiver)) {
       return dominator;
     }
     if (dominator->op()->EffectInputCount() != 1) {

src/compiler/js-call-reducer.cc

@@ -191,6 +191,19 @@ Reduction JSCallReducer::ReduceFunctionPrototypeCall(Node* node) {
 
 namespace {
 
+// TODO(turbofan): Shall we move this to the NodeProperties? Or some (untyped)
+// alias analyzer?
+bool IsSame(Node* a, Node* b) {
+  if (a == b) {
+    return true;
+  } else if (a->opcode() == IrOpcode::kCheckHeapObject) {
+    return IsSame(a->InputAt(0), b);
+  } else if (b->opcode() == IrOpcode::kCheckHeapObject) {
+    return IsSame(a, b->InputAt(0));
+  }
+  return false;
+}
+
 // TODO(turbofan): Share with similar functionality in JSInliningHeuristic
 // and JSNativeContextSpecialization, i.e. move to NodeProperties helper?!
 MaybeHandle<Map> InferReceiverMap(Node* node) {
@@ -200,7 +213,7 @@ MaybeHandle<Map> InferReceiverMap(Node* node) {
   // for the {receiver}, and if so use that map for the lowering below.
   for (Node* dominator = effect;;) {
     if (dominator->opcode() == IrOpcode::kCheckMaps &&
-        dominator->InputAt(0) == receiver) {
+        IsSame(dominator->InputAt(0), receiver)) {
       if (dominator->op()->ValueInputCount() == 2) {
         HeapObjectMatcher m(dominator->InputAt(1));
         if (m.HasValue()) return Handle<Map>::cast(m.Value());

src/compiler/js-inlining.cc

@@ -282,6 +282,19 @@ Node* JSInliner::CreateTailCallerFrameState(Node* node, Node* frame_state) {
 
 namespace {
 
+// TODO(turbofan): Shall we move this to the NodeProperties? Or some (untyped)
+// alias analyzer?
+bool IsSame(Node* a, Node* b) {
+  if (a == b) {
+    return true;
+  } else if (a->opcode() == IrOpcode::kCheckHeapObject) {
+    return IsSame(a->InputAt(0), b);
+  } else if (b->opcode() == IrOpcode::kCheckHeapObject) {
+    return IsSame(a, b->InputAt(0));
+  }
+  return false;
+}
+
 // TODO(bmeurer): Unify this with the witness helper functions in the
 // js-builtin-reducer.cc once we have a better understanding of the
 // map tracking we want to do, and eventually changed the CheckMaps
@@ -296,7 +309,7 @@ namespace {
 bool NeedsConvertReceiver(Node* receiver, Node* effect) {
   for (Node* dominator = effect;;) {
     if (dominator->opcode() == IrOpcode::kCheckMaps &&
-        dominator->InputAt(0) == receiver) {
+        IsSame(dominator->InputAt(0), receiver)) {
       // Check if all maps have the given {instance_type}.
       for (int i = 1; i < dominator->op()->ValueInputCount(); ++i) {
         HeapObjectMatcher m(NodeProperties::GetValueInput(dominator, i));

src/compiler/js-native-context-specialization.cc

@@ -161,7 +161,8 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
                                            receiver, effect, control);
     } else {
       // Monomorphic property access.
-      effect = BuildCheckHeapObject(receiver, effect, control);
+      receiver = effect = graph()->NewNode(simplified()->CheckHeapObject(),
+                                           receiver, effect, control);
       effect = BuildCheckMaps(receiver, effect, control,
                               access_info.receiver_maps());
     }
@@ -199,7 +200,8 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
       receiverissmi_control = graph()->NewNode(common()->IfTrue(), branch);
       receiverissmi_effect = effect;
     } else {
-      effect = BuildCheckHeapObject(receiver, effect, control);
+      receiver = effect = graph()->NewNode(simplified()->CheckHeapObject(),
+                                           receiver, effect, control);
     }
 
     // Load the {receiver} map. The resulting effect is the dominating effect
@@ -507,7 +509,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
     }
 
     // Ensure that {receiver} is a heap object.
-    effect = BuildCheckHeapObject(receiver, effect, control);
+    receiver = effect = graph()->NewNode(simplified()->CheckHeapObject(),
+                                         receiver, effect, control);
 
     // Check for the monomorphic case.
     if (access_infos.size() == 1) {
@@ -1449,33 +1452,6 @@ Node* JSNativeContextSpecialization::BuildCheckMaps(
                           inputs);
 }
 
-Node* JSNativeContextSpecialization::BuildCheckHeapObject(Node* receiver,
-                                                          Node* effect,
-                                                          Node* control) {
-  switch (receiver->opcode()) {
-    case IrOpcode::kHeapConstant:
-    case IrOpcode::kJSCreate:
-    case IrOpcode::kJSCreateArguments:
-    case IrOpcode::kJSCreateArray:
-    case IrOpcode::kJSCreateClosure:
-    case IrOpcode::kJSCreateIterResultObject:
-    case IrOpcode::kJSCreateLiteralArray:
-    case IrOpcode::kJSCreateLiteralObject:
-    case IrOpcode::kJSCreateLiteralRegExp:
-    case IrOpcode::kJSConvertReceiver:
-    case IrOpcode::kJSToName:
-    case IrOpcode::kJSToString:
-    case IrOpcode::kJSToObject:
-    case IrOpcode::kJSTypeOf: {
-      return effect;
-    }
-    default: {
-      return graph()->NewNode(simplified()->CheckHeapObject(), receiver, effect,
-                              control);
-    }
-  }
-}
-
 void JSNativeContextSpecialization::AssumePrototypesStable(
     std::vector<Handle<Map>> const& receiver_maps, Handle<JSObject> holder) {
   // Determine actual holder and perform prototype chain checks.

src/compiler/js-native-context-specialization.h

@@ -118,9 +118,6 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
   Node* BuildCheckMaps(Node* receiver, Node* effect, Node* control,
                        std::vector<Handle<Map>> const& maps);
 
-  // Construct an appropriate heap object check.
-  Node* BuildCheckHeapObject(Node* receiver, Node* effect, Node* control);
-
   // Adds stability dependencies on all prototypes of every class in
   // {receiver_type} up to (and including) the {holder}.
   void AssumePrototypesStable(std::vector<Handle<Map>> const& receiver_maps,

test/mjsunit/regress/regress-crbug-662410.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 g(v) { return v.constructor; }
+
+g({});
+g({});
+
+function f() {
+  var i = 0;
+  do {
+    i = i + 1;
+    g(i);
+  } while (i < 1);
+}
+
+%OptimizeFunctionOnNextCall(f);
+f();