[Turboprop] Fix deprecated map migration in dynamic map check builtin.

The TryMigrateInstance should be passed the instance object to migrate,
not the map of the object. Also make the runtime function explicitly
check for JSObjects.

BUG=v8:9684

Change-Id: I03605d9f3103b618243c12ad0b63035484ef4134
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2487270
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70731}

src/builtins/base.tq

@@ -50,8 +50,7 @@ type Zero extends PositiveSmi;
 type Uninitialized extends Tagged;
 
 extern macro MakeWeak(HeapObject): WeakHeapObject;
-extern macro GetHeapObjectAssumeWeak(WeakHeapObject):
-    HeapObject labels ClearedWeakPointer;
+extern macro GetHeapObjectAssumeWeak(MaybeObject): HeapObject labels IfCleared;
 extern macro GetHeapObjectIfStrong(MaybeObject): HeapObject labels IfNotStrong;
 extern macro IsWeakOrCleared(MaybeObject): bool;
 extern macro IsWeakReferenceToObject(MaybeObject, Object): bool;

src/builtins/ic-dynamic-map-checks.tq

@@ -84,9 +84,9 @@ macro PerformPolymorphicCheck(
 }
 
 macro PerformMonomorphicCheck(
-    feedbackVector: FeedbackVector, slotIndex: intptr, maybeMap: MaybeObject,
+    feedbackVector: FeedbackVector, slotIndex: intptr, expectedMap: HeapObject,
     actualMap: Map, actualHandler: Smi|DataHandler): int32 {
-  if (IsWeakReferenceToObject(maybeMap, actualMap)) {
+  if (TaggedEqual(expectedMap, actualMap)) {
     const handlerIndex = slotIndex + 1;
     assert(handlerIndex < feedbackVector.length_intptr);
     const maybeHandler =
@@ -135,17 +135,20 @@ builtin DynamicMapChecks(implicit context: Context)(
     return PerformPolymorphicCheck(
         maybePolymorphicArray, actualMap, actualHandler);
   } label MigrateAndDoMonomorphicCheck {
+    const expectedMap = GetHeapObjectAssumeWeak(feedback) otherwise Deopt;
     if (IsDeprecatedMap(actualMap)) {
       // TODO(gsathya): Should this migration happen before the
       // polymorphic check?
-      const result = TryMigrateInstance(actualMap);
+      const result = TryMigrateInstance(actualValue);
       if (TaggedIsSmi(result)) {
         return kDeopt;
       }
       actualMap = actualValue.map;
     }
     return PerformMonomorphicCheck(
-        feedbackVector, slotIndex, feedback, actualMap, actualHandler);
+        feedbackVector, slotIndex, expectedMap, actualMap, actualHandler);
+  } label Deopt {
+    return kDeopt;
   }
 }
 

src/runtime/runtime-object.cc

@@ -859,9 +859,7 @@ RUNTIME_FUNCTION(Runtime_CompleteInobjectSlackTrackingForMap) {
 RUNTIME_FUNCTION(Runtime_TryMigrateInstance) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(Object, object, 0);
-  if (!object->IsJSObject()) return Smi::zero();
-  Handle<JSObject> js_object = Handle<JSObject>::cast(object);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, js_object, 0);
   // It could have been a DCHECK but we call this function directly from tests.
   if (!js_object->map().is_deprecated()) return Smi::zero();
   // This call must not cause lazy deopts, because it's called from deferred
@@ -869,7 +867,7 @@ RUNTIME_FUNCTION(Runtime_TryMigrateInstance) {
   // ID. So we just try migration and signal failure if necessary,
   // which will also trigger a deopt.
   if (!JSObject::TryMigrateInstance(isolate, js_object)) return Smi::zero();
-  return *object;
+  return *js_object;
 }
 
 static bool IsValidAccessor(Isolate* isolate, Handle<Object> obj) {

test/mjsunit/compiler/test-dynamic-map-check-deprecated-maps2.js

+// Copyright 2020 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 --turboprop --dynamic-map-checks --opt
+// Flags: --no-always-opt
+
+function b(a) { return a; }
+
+function f(o, should_bailout) {
+  b(o.a);
+  let did_bailout = (%GetOptimizationStatus(f) &
+                    V8OptimizationStatus.kTopmostFrameIsTurboFanned) == 0;
+  assertEquals(should_bailout, did_bailout);
+}
+
+var o = {a:10, b:20, c:30};
+var o1 = {a:10, b:20, c:30};
+var o2 = {a:10, b:20, c:30};
+%PrepareFunctionForOptimization(f);
+f(o, true);
+%OptimizeFunctionOnNextCall(f);
+f(o, false);
+assertOptimized(f);
+
+// Transition o to a new map and deprecate the old one (which is embedded in the
+// optimized code for the dynamic map check).
+o.b = 10.23;
+f(o, true);
+f(o1, false);
+f(o2, false);
+assertOptimized(f);
+
+// Deprecate o's new map again and update the feedback vector but don't migrate
+// o.
+o1.c = 20.23;
+f(o1, true);
+assertOptimized(f);
+
+// We should migrates o's map without bailing out.
+f(o, false);
+f(o1, false);
+f(o2, false);
+assertOptimized(f);