[turboprop] Migrate deprecated maps in dynamic map check operator

If incoming map is deprecated, generate code to migrate the map. Since
this involves generating additional code and a call to runtime, we only
do this if one of the receiver maps was a migration target when
optimizing this function. If not, we deoptimize and discard the
optimized code if we see a deprecated map. This is to avoid bailout
loops when we see deprecated maps.

This change does the following:
// We generated code to migrate deprecated maps only if one of the maps
// in feedback vector is a migration target.
if ( there are migration targets in feedback)
{
  if (checkMaps fails) {
     if (incoming map is deprecated) {
        migrate the map
        checkMaps with the new map
     } else {
       bailout
     }
  }
} else {
  if (checkMaps fails)
    bailout;
}

Bug: v8:10582, v8:9684
Change-Id: I8a04c77ed209dd2fb0300a783d844f2335a678c8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2292231Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69179}

src/compiler/js-heap-broker.cc

@@ -4676,11 +4676,12 @@ bool ElementAccessFeedback::HasOnlyStringMaps(JSHeapBroker* broker) const {
 
 MinimorphicLoadPropertyAccessFeedback::MinimorphicLoadPropertyAccessFeedback(
     NameRef const& name, FeedbackSlotKind slot_kind, bool is_monomorphic,
-    Handle<Object> handler)
+    Handle<Object> handler, bool has_migration_target_maps)
     : ProcessedFeedback(kMinimorphicPropertyAccess, slot_kind),
       name_(name),
       is_monomorphic_(is_monomorphic),
-      handler_(handler) {
+      handler_(handler),
+      has_migration_target_maps_(has_migration_target_maps) {
   DCHECK(IsLoadICKind(slot_kind));
 }
 
@@ -4779,6 +4780,13 @@ MaybeObjectHandle TryGetMinimorphicHandler(
   }
   return initial_handler;
 }
+
+bool HasMigrationTargets(const MapHandles& maps) {
+  for (Handle<Map> map : maps) {
+    if (map->is_migration_target()) return true;
+  }
+  return false;
+}
 }  // namespace
 
 bool JSHeapBroker::CanUseFeedback(const FeedbackNexus& nexus) const {
@@ -4801,19 +4809,20 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForPropertyAccess(
 
   std::vector<MapAndHandler> maps_and_handlers;
   nexus.ExtractMapsAndFeedback(&maps_and_handlers);
+  MapHandles maps;
+  for (auto const& entry : maps_and_handlers) {
+    maps.push_back(entry.first);
+  }
 
   base::Optional<NameRef> name =
       static_name.has_value() ? static_name : GetNameFeedback(nexus);
   MaybeObjectHandle handler = TryGetMinimorphicHandler(maps_and_handlers, kind);
   if (!handler.is_null()) {
     return *zone()->New<MinimorphicLoadPropertyAccessFeedback>(
-        *name, kind, nexus.ic_state() == MONOMORPHIC, handler.object());
+        *name, kind, nexus.ic_state() == MONOMORPHIC, handler.object(),
+        HasMigrationTargets(maps));
   }
 
-  MapHandles maps;
-  for (auto const& entry : maps_and_handlers) {
-    maps.push_back(entry.first);
-  }
   FilterRelevantReceiverMaps(isolate(), &maps);
 
   // If no maps were found for a non-megamorphic access, then our maps died

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

@@ -1067,9 +1067,13 @@ Reduction JSNativeContextSpecialization::ReduceMinimorphicPropertyAccess(
   if (access_info.IsInvalid()) return NoChange();
 
   PropertyAccessBuilder access_builder(jsgraph(), broker(), nullptr);
+  CheckMapsFlags flags = CheckMapsFlag::kNone;
+  if (feedback.has_migration_target_maps()) {
+    flags |= CheckMapsFlag::kTryMigrateInstance;
+  }
   effect = graph()->NewNode(
       simplified()->DynamicCheckMaps(
-          feedback.handler(), source,
+          flags, feedback.handler(), source,
           feedback.is_monomorphic()
               ? DynamicCheckMapsParameters::ICState::kMonomorphic
               : DynamicCheckMapsParameters::ICState::kPolymorphic),

src/compiler/processed-feedback.h

@@ -178,16 +178,19 @@ class MinimorphicLoadPropertyAccessFeedback : public ProcessedFeedback {
   MinimorphicLoadPropertyAccessFeedback(NameRef const& name,
                                         FeedbackSlotKind slot_kind,
                                         bool is_monomorphic,
-                                        Handle<Object> handler);
+                                        Handle<Object> handler,
+                                        bool has_migration_target_maps);
 
   NameRef const& name() const { return name_; }
   bool is_monomorphic() const { return is_monomorphic_; }
   Handle<Object> handler() const { return handler_; }
+  bool has_migration_target_maps() const { return has_migration_target_maps_; }
 
  private:
   NameRef const name_;
-  bool is_monomorphic_;
-  Handle<Object> handler_;
+  bool const is_monomorphic_;
+  Handle<Object> const handler_;
+  bool const has_migration_target_maps_;
 };
 
 class CallFeedback : public ProcessedFeedback {

src/compiler/simplified-operator.cc

@@ -1474,9 +1474,11 @@ const Operator* SimplifiedOperatorBuilder::CheckMaps(
 }
 
 const Operator* SimplifiedOperatorBuilder::DynamicCheckMaps(
-    Handle<Object> handler, const FeedbackSource& feedback,
+    CheckMapsFlags flags, Handle<Object> handler,
+    const FeedbackSource& feedback,
     DynamicCheckMapsParameters::ICState ic_state) {
-  DynamicCheckMapsParameters const parameters(handler, feedback, ic_state);
+  DynamicCheckMapsParameters const parameters(flags, handler, feedback,
+                                              ic_state);
   return zone()->New<Operator1<DynamicCheckMapsParameters>>(  // --
       IrOpcode::kDynamicCheckMaps,                            // opcode
       Operator::kNoThrow | Operator::kNoWrite,                // flags

src/compiler/simplified-operator.h

@@ -431,15 +431,17 @@ class DynamicCheckMapsParameters final {
  public:
   enum ICState { kMonomorphic, kPolymorphic };
 
-  DynamicCheckMapsParameters(Handle<Object> handler,
+  DynamicCheckMapsParameters(CheckMapsFlags flags, Handle<Object> handler,
                              const FeedbackSource& feedback, ICState state)
-      : handler_(handler), feedback_(feedback), state_(state) {}
+      : flags_(flags), handler_(handler), feedback_(feedback), state_(state) {}
 
+  CheckMapsFlags flags() const { return flags_; }
   Handle<Object> handler() const { return handler_; }
   FeedbackSource const& feedback() const { return feedback_; }
   ICState const& state() const { return state_; }
 
  private:
+  CheckMapsFlags const flags_;
   Handle<Object> const handler_;
   FeedbackSource const feedback_;
   ICState const state_;
@@ -874,7 +876,8 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* CheckMaps(CheckMapsFlags, ZoneHandleSet<Map>,
                             const FeedbackSource& = FeedbackSource());
   const Operator* DynamicCheckMaps(
-      Handle<Object> handler, const FeedbackSource& feedback,
+      CheckMapsFlags flags, Handle<Object> handler,
+      const FeedbackSource& feedback,
       DynamicCheckMapsParameters::ICState ic_state);
   const Operator* CheckNotTaggedHole();
   const Operator* CheckNumber(const FeedbackSource& feedback);

src/deoptimizer/deoptimize-reason.h

@@ -63,6 +63,7 @@ namespace internal {
   V(WrongInstanceType, "wrong instance type")                                  \
   V(WrongMap, "wrong map")                                                     \
   V(MissingMap, "missing map")                                                 \
+  V(DeprecatedMap, "deprecated map")                                           \
   V(WrongHandler, "wrong handler")                                             \
   V(WrongName, "wrong name")                                                   \
   V(WrongValue, "wrong value")                                                 \

test/mjsunit/compiler/test-dynamic-map-check-deprecated-maps-polymorphic.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 f(o) {
+  return o.b;
+}
+
+var o = {a:10, b:20};
+var o1 = {a:10, b:20};
+var o2 = {a:10, b:20};
+var o3 = {a:10, b:20, c:30};
+%PrepareFunctionForOptimization(f);
+// Transition IC state to polymorphic.
+f(o);
+f(o3);
+%OptimizeFunctionOnNextCall(f);
+f(o);
+assertOptimized(f);
+f(o);
+
+// Deprecates O's map.
+o1.b = 10.23;
+// Deoptimizes but retains code.
+f(o1);
+assertOptimized(f);
+
+// Continues to use optimized code since deprecated map is still in the
+// feedback. ICs don't drop deprecated maps in the polymoprhic case.
+f(o);
+f(o);
+assertOptimized(f);

test/mjsunit/compiler/test-dynamic-map-check-deprecated-maps.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 f(o) {
+  return o.b;
+}
+
+var o = {a:10, b:20};
+var o1 = {a:10, b:20};
+var o2 = {a:10, b:20};
+%PrepareFunctionForOptimization(f);
+f(o);
+%OptimizeFunctionOnNextCall(f);
+f(o);
+assertOptimized(f);
+%PrepareFunctionForOptimization(f);
+f(o);
+
+// Deprecates O's map.
+o1.b = 10.23;
+// Deoptimizes but retains code.
+f(o1);
+assertOptimized(f);
+
+// Deoptimizes and discards code.
+f(o);
+f(o);
+assertUnoptimized(f);
+
+// When we reoptimize we should include code for migrating deprecated maps.
+%OptimizeFunctionOnNextCall(f);
+f(o);
+assertOptimized(f);
+
+f(o2);
+f(o2);
+assertOptimized(f);