[map] Support in-place field representation changes.

This adds a new flag --modify-field-representation-inplace (enabled by
default), which lets the runtime perform field representation changes
for Smi to Tagged or for HeapObject to Tagged in-place instead of
creating new maps and marking the previous map tree as deprecated.

That means we create (a lot) fewer Maps and DescriptorArrays in the
beginning and also need to self-heal fewer objects later (migrating
off the deprecated maps). In TurboFan we just take the "field owner
dependency" whenever we use the field representation, which is very
similar to what we already do for the field types. That means if we
change the representation of a field that we used in optimized code,
we will simply deoptimize that code and have TurboFan potentially
later optimize it again with the new field representation.

On the Speedometer2/ElmJS-TodoMVC test, this reduces the total execution
time from around 415ms to around 352ms, which corresponds to a **15%**
improvement. The overall Speedometer2 score improves from around 74.1
to around 78.3 (on local runs with content_shell), corresponding to a
**5.6%** improvement here. 🎉

On the CNN desktop browsing story, it seems that we reduce map space
utilization/fragmentation by about 4-5%. But since we allocate a lot
less (fewer Maps and DescriptorArrays) we also significantly change
the GC timing, which heavily influences the results here. So take this
with a grain of salt. 🤷

Note: For Double fields, this doesn't change anything, meaning they
still create new maps and deprecate the previous map trees.

Bug: v8:8749, v8:8865, v8:9114
Change-Id: Ibd70efcb59be982863905663dbfaa89aa5b31e14
Cq-Include-Trybots: luci.chromium.try:linux-rel,win7-rel
Doc: http://bit.ly/v8-in-place-field-representation-changes
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1565891
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60822}

src/compiler/access-info.cc

@@ -319,9 +319,12 @@ bool AccessInfoFactory::ComputeDataFieldAccessInfo(
   Type field_type = Type::NonInternal();
   MachineRepresentation field_representation = MachineRepresentation::kTagged;
   MaybeHandle<Map> field_map;
+  MapRef map_ref(broker(), map);
   if (details_representation.IsSmi()) {
     field_type = Type::SignedSmall();
     field_representation = MachineRepresentation::kTaggedSigned;
+    map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
+    dependencies()->DependOnFieldRepresentation(map_ref, number);
   } else if (details_representation.IsDouble()) {
     field_type = type_cache_->kFloat64;
     field_representation = MachineRepresentation::kFloat64;
@@ -337,9 +340,10 @@ bool AccessInfoFactory::ComputeDataFieldAccessInfo(
 
       // The field type was cleared by the GC, so we don't know anything
       // about the contents now.
-    } else if (descriptors_field_type->IsClass()) {
-      MapRef map_ref(broker(), map);
-      map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
+    }
+    map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
+    dependencies()->DependOnFieldRepresentation(map_ref, number);
+    if (descriptors_field_type->IsClass()) {
       dependencies()->DependOnFieldType(map_ref, number);
       // Remember the field map, and try to infer a useful type.
       Handle<Map> map(descriptors_field_type->AsClass(), isolate());
@@ -699,9 +703,12 @@ bool AccessInfoFactory::LookupTransition(
   Type field_type = Type::NonInternal();
   MaybeHandle<Map> field_map;
   MachineRepresentation field_representation = MachineRepresentation::kTagged;
+  MapRef transition_map_ref(broker(), transition_map);
   if (details_representation.IsSmi()) {
     field_type = Type::SignedSmall();
     field_representation = MachineRepresentation::kTaggedSigned;
+    transition_map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
+    dependencies()->DependOnFieldRepresentation(transition_map_ref, number);
   } else if (details_representation.IsDouble()) {
     field_type = type_cache_->kFloat64;
     field_representation = MachineRepresentation::kFloat64;
@@ -715,10 +722,10 @@ bool AccessInfoFactory::LookupTransition(
     if (descriptors_field_type->IsNone()) {
       // Store is not safe if the field type was cleared.
       return false;
-    } else if (descriptors_field_type->IsClass()) {
-      MapRef transition_map_ref(broker(), transition_map);
-      transition_map_ref
-          .SerializeOwnDescriptors();  // TODO(neis): Remove later.
+    }
+    transition_map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
+    dependencies()->DependOnFieldRepresentation(transition_map_ref, number);
+    if (descriptors_field_type->IsClass()) {
       dependencies()->DependOnFieldType(transition_map_ref, number);
       // Remember the field map, and try to infer a useful type.
       Handle<Map> map(descriptors_field_type->AsClass(), isolate());

src/compiler/compilation-dependencies.cc

@@ -163,6 +163,41 @@ class PretenureModeDependency final
   AllocationType allocation_;
 };
 
+class FieldRepresentationDependency final
+    : public CompilationDependencies::Dependency {
+ public:
+  // TODO(neis): Once the concurrent compiler frontend is always-on, we no
+  // longer need to explicitly store the representation.
+  FieldRepresentationDependency(const MapRef& owner, int descriptor,
+                                Representation representation)
+      : owner_(owner),
+        descriptor_(descriptor),
+        representation_(representation) {
+    DCHECK(owner_.equals(owner_.FindFieldOwner(descriptor_)));
+    DCHECK(representation_.Equals(
+        owner_.GetPropertyDetails(descriptor_).representation()));
+  }
+
+  bool IsValid() const override {
+    DisallowHeapAllocation no_heap_allocation;
+    Handle<Map> owner = owner_.object();
+    return representation_.Equals(owner->instance_descriptors()
+                                      ->GetDetails(descriptor_)
+                                      .representation());
+  }
+
+  void Install(const MaybeObjectHandle& code) override {
+    SLOW_DCHECK(IsValid());
+    DependentCode::InstallDependency(owner_.isolate(), code, owner_.object(),
+                                     DependentCode::kFieldOwnerGroup);
+  }
+
+ private:
+  MapRef owner_;
+  int descriptor_;
+  Representation representation_;
+};
+
 class FieldTypeDependency final : public CompilationDependencies::Dependency {
  public:
   // TODO(neis): Once the concurrent compiler frontend is always-on, we no
@@ -410,6 +445,16 @@ PropertyConstness CompilationDependencies::DependOnFieldConstness(
   return PropertyConstness::kConst;
 }
 
+void CompilationDependencies::DependOnFieldRepresentation(const MapRef& map,
+                                                          int descriptor) {
+  MapRef owner = map.FindFieldOwner(descriptor);
+  PropertyDetails details = owner.GetPropertyDetails(descriptor);
+  DCHECK(details.representation().Equals(
+      map.GetPropertyDetails(descriptor).representation()));
+  dependencies_.push_front(new (zone_) FieldRepresentationDependency(
+      owner, descriptor, details.representation()));
+}
+
 void CompilationDependencies::DependOnFieldType(const MapRef& map,
                                                 int descriptor) {
   MapRef owner = map.FindFieldOwner(descriptor);

src/compiler/compilation-dependencies.h

@@ -51,6 +51,10 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // not change.
   AllocationType DependOnPretenureMode(const AllocationSiteRef& site);
 
+  // Record the assumption that the field representation of a field does not
+  // change. The field is identified by the arguments.
+  void DependOnFieldRepresentation(const MapRef& map, int descriptor);
+
   // Record the assumption that the field type of a field does not change. The
   // field is identified by the arguments.
   void DependOnFieldType(const MapRef& map, int descriptor);

src/flag-definitions.h

@@ -1044,6 +1044,8 @@ DEFINE_BOOL_READONLY(track_constant_fields, true,
                      "enable constant field tracking")
 DEFINE_BOOL_READONLY(fast_map_update, false,
                      "enable fast map update by caching the migration target")
+DEFINE_BOOL(modify_field_representation_inplace, true,
+            "enable in-place field representation updates")
 DEFINE_INT(max_polymorphic_map_count, 4,
            "maximum number of maps to track in POLYMORPHIC state")
 

src/map-updater.cc

@@ -211,6 +211,9 @@ MapUpdater::State MapUpdater::CopyGeneralizeAllFields(const char* reason) {
 }
 
 MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() {
+  // Updating deprecated maps in-place doesn't make sense.
+  if (old_map_->is_deprecated()) return state_;
+
   // If it's just a representation generalization case (i.e. property kind and
   // attributes stays unchanged) it's fine to transition from None to anything
   // but double without any modification to the object, because the default
@@ -223,7 +226,7 @@ MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() {
   PropertyDetails old_details =
       old_descriptors_->GetDetails(modified_descriptor_);
   Representation old_representation = old_details.representation();
-  if (!old_representation.IsNone()) {
+  if (!old_representation.CanBeInPlaceChangedTo(new_representation_)) {
     return state_;  // Not done yet.
   }
 

src/objects.cc

@@ -3791,6 +3791,17 @@ Handle<DescriptorArray> DescriptorArray::CopyForFastObjectClone(
     MaybeObject type = src->GetValue(i);
     if (details.location() == PropertyLocation::kField) {
       type = MaybeObject::FromObject(FieldType::Any());
+      // TODO(bmeurer,ishell): Igor suggested to use some kind of dynamic
+      // checks in the fast-path for CloneObjectIC instead to avoid the
+      // need to generalize the descriptors here. That will also enable
+      // us to skip the defensive copying of the target map whenever a
+      // CloneObjectIC misses.
+      if (FLAG_modify_field_representation_inplace &&
+          (new_details.representation().IsSmi() ||
+           new_details.representation().IsHeapObject())) {
+        new_details =
+            new_details.CopyWithRepresentation(Representation::Tagged());
+      }
     }
     descriptors->Set(i, key, type, new_details);
   }

src/objects/map.cc

@@ -748,12 +748,14 @@ void Map::UpdateFieldType(Isolate* isolate, int descriptor, Handle<Name> name,
     DescriptorArray descriptors = current->instance_descriptors();
     PropertyDetails details = descriptors->GetDetails(descriptor);
 
-    // It is allowed to change representation here only from None to something.
+    // It is allowed to change representation here only from None
+    // to something or from Smi or HeapObject to Tagged.
     DCHECK(details.representation().Equals(new_representation) ||
-           details.representation().IsNone());
+           details.representation().CanBeInPlaceChangedTo(new_representation));
 
     // Skip if already updated the shared descriptor.
     if (new_constness != details.constness() ||
+        !new_representation.Equals(details.representation()) ||
         descriptors->GetFieldType(descriptor) != *new_wrapped_type.object()) {
       DCHECK_IMPLIES(!FLAG_track_constant_fields,
                      new_constness == PropertyConstness::kMutable);

src/property-details.h

@@ -116,6 +116,12 @@ class Representation {
     return Equals(other);
   }
 
+  bool CanBeInPlaceChangedTo(const Representation& other) const {
+    if (IsNone()) return true;
+    if (!FLAG_modify_field_representation_inplace) return false;
+    return (IsSmi() || IsHeapObject()) && other.IsTagged();
+  }
+
   bool is_more_general_than(const Representation& other) const {
     if (IsHeapObject()) return other.IsNone();
     return kind_ > other.kind_;

test/cctest/test-field-type-tracking.cc

@@ -646,7 +646,7 @@ void TestGeneralizeField(int detach_property_at_index, int property_index,
                                       from.representation, from.type);
     } else {
       map = expectations.AddDataField(map, NONE, kDefaultFieldConstness,
-                                      Representation::Smi(), any_type);
+                                      Representation::Double(), any_type);
       if (i == detach_property_at_index) {
         detach_point_map = map;
       }
@@ -801,7 +801,9 @@ TEST(GeneralizeSmiFieldToTagged) {
   TestGeneralizeField(
       {PropertyConstness::kMutable, Representation::Smi(), any_type},
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      !FLAG_modify_field_representation_inplace,
+      FLAG_modify_field_representation_inplace);
 }
 
 TEST(GeneralizeDoubleFieldToTagged) {
@@ -831,7 +833,9 @@ TEST(GeneralizeHeapObjectFieldToTagged) {
   TestGeneralizeField(
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
       {PropertyConstness::kMutable, Representation::Smi(), any_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      !FLAG_modify_field_representation_inplace,
+      FLAG_modify_field_representation_inplace);
 }
 
 TEST(GeneralizeHeapObjectFieldToHeapObject) {
@@ -2252,10 +2256,9 @@ TEST(ReconfigurePropertySplitMapTransitionsOverflow) {
 // IS_PROTO_TRANS_ISSUE_FIXED and IS_NON_EQUIVALENT_TRANSITION_SUPPORTED are
 // fixed.
 template <typename TestConfig>
-static void TestGeneralizeFieldWithSpecialTransition(TestConfig& config,
-                                                     const CRFTData& from,
-                                                     const CRFTData& to,
-                                                     const CRFTData& expected) {
+static void TestGeneralizeFieldWithSpecialTransition(
+    TestConfig& config, const CRFTData& from, const CRFTData& to,
+    const CRFTData& expected, bool expected_deprecation) {
   Isolate* isolate = CcTest::i_isolate();
 
   Expectations expectations(isolate);
@@ -2301,39 +2304,46 @@ static void TestGeneralizeFieldWithSpecialTransition(TestConfig& config,
     expectations.SetDataField(i, expected.constness, expected.representation,
                               expected.type);
 
-    CHECK(map->is_deprecated());
-    CHECK_NE(*map, *new_map);
-    CHECK(i == 0 || maps[i - 1]->is_deprecated());
-    CHECK(expectations.Check(*new_map));
+    if (expected_deprecation) {
+      CHECK(map->is_deprecated());
+      CHECK_NE(*map, *new_map);
+      CHECK(i == 0 || maps[i - 1]->is_deprecated());
+      CHECK(expectations.Check(*new_map));
 
-    Handle<Map> new_map2 = Map::Update(isolate, map2);
-    CHECK(!new_map2->is_deprecated());
-    CHECK(!new_map2->is_dictionary_map());
+      Handle<Map> new_map2 = Map::Update(isolate, map2);
+      CHECK(!new_map2->is_deprecated());
+      CHECK(!new_map2->is_dictionary_map());
 
-    Handle<Map> tmp_map;
-    if (Map::TryUpdate(isolate, map2).ToHandle(&tmp_map)) {
-      // If Map::TryUpdate() manages to succeed the result must match the result
-      // of Map::Update().
-      CHECK_EQ(*new_map2, *tmp_map);
-    } else {
-      // Equivalent transitions should always find the updated map.
-      CHECK(config.is_non_equivalent_transition());
-    }
-
-    if (config.is_non_equivalent_transition()) {
-      // In case of non-equivalent transition currently we generalize all
-      // representations.
-      for (int i = 0; i < kPropCount; i++) {
-        expectations2.GeneralizeField(i);
+      Handle<Map> tmp_map;
+      if (Map::TryUpdate(isolate, map2).ToHandle(&tmp_map)) {
+        // If Map::TryUpdate() manages to succeed the result must match the
+        // result of Map::Update().
+        CHECK_EQ(*new_map2, *tmp_map);
+      } else {
+        // Equivalent transitions should always find the updated map.
+        CHECK(config.is_non_equivalent_transition());
       }
-      CHECK(new_map2->GetBackPointer()->IsUndefined(isolate));
-      CHECK(expectations2.Check(*new_map2));
-    } else {
-      expectations2.SetDataField(i, expected.constness, expected.representation,
-                                 expected.type);
 
-      CHECK(!new_map2->GetBackPointer()->IsUndefined(isolate));
-      CHECK(expectations2.Check(*new_map2));
+      if (config.is_non_equivalent_transition()) {
+        // In case of non-equivalent transition currently we generalize all
+        // representations.
+        for (int i = 0; i < kPropCount; i++) {
+          expectations2.GeneralizeField(i);
+        }
+        CHECK(new_map2->GetBackPointer()->IsUndefined(isolate));
+        CHECK(expectations2.Check(*new_map2));
+      } else {
+        expectations2.SetDataField(i, expected.constness,
+                                   expected.representation, expected.type);
+
+        CHECK(!new_map2->GetBackPointer()->IsUndefined(isolate));
+        CHECK(expectations2.Check(*new_map2));
+      }
+    } else {
+      CHECK(!map->is_deprecated());
+      // TODO(ishell): Update test expectations properly.
+      // CHECK_EQ(*map2, *new_map);
+      // CHECK(expectations2.Check(*new_map));
     }
   }
 
@@ -2351,7 +2361,6 @@ static void TestGeneralizeFieldWithSpecialTransition(TestConfig& config,
   }
 }
 
-
 TEST(ElementsKindTransitionFromMapOwningDescriptor) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
@@ -2387,7 +2396,14 @@ TEST(ElementsKindTransitionFromMapOwningDescriptor) {
         configs[i],
         {PropertyConstness::kMutable, Representation::Smi(), any_type},
         {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-        {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+        {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+        !FLAG_modify_field_representation_inplace);
+    TestGeneralizeFieldWithSpecialTransition(
+        configs[i],
+        {PropertyConstness::kMutable, Representation::Double(), any_type},
+        {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
+        {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+        true);
   }
 }
 
@@ -2439,7 +2455,14 @@ TEST(ElementsKindTransitionFromMapNotOwningDescriptor) {
         configs[i],
         {PropertyConstness::kMutable, Representation::Smi(), any_type},
         {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-        {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+        {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+        !FLAG_modify_field_representation_inplace);
+    TestGeneralizeFieldWithSpecialTransition(
+        configs[i],
+        {PropertyConstness::kMutable, Representation::Double(), any_type},
+        {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
+        {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+        true);
   }
 }
 
@@ -2475,7 +2498,12 @@ TEST(PrototypeTransitionFromMapOwningDescriptor) {
   TestGeneralizeFieldWithSpecialTransition(
       config, {PropertyConstness::kMutable, Representation::Smi(), any_type},
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      !FLAG_modify_field_representation_inplace);
+  TestGeneralizeFieldWithSpecialTransition(
+      config, {PropertyConstness::kMutable, Representation::Double(), any_type},
+      {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type}, true);
 }
 
 
@@ -2522,7 +2550,12 @@ TEST(PrototypeTransitionFromMapNotOwningDescriptor) {
   TestGeneralizeFieldWithSpecialTransition(
       config, {PropertyConstness::kMutable, Representation::Smi(), any_type},
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      !FLAG_modify_field_representation_inplace);
+  TestGeneralizeFieldWithSpecialTransition(
+      config, {PropertyConstness::kMutable, Representation::Double(), any_type},
+      {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type}, true);
 }
 
 
@@ -2811,12 +2844,17 @@ TEST(TransitionDataConstantToDataField) {
 
   Handle<Object> value2 = isolate->factory()->NewHeapNumber(0);
   TransitionToDataFieldOperator transition_op2(
-      PropertyConstness::kMutable, Representation::Double(), any_type, value2);
+      PropertyConstness::kMutable, Representation::Tagged(), any_type, value2);
 
-  FieldGeneralizationChecker checker(kPropCount - 1,
-                                     PropertyConstness::kMutable,
-                                     Representation::Tagged(), any_type);
-  TestTransitionTo(transition_op1, transition_op2, checker);
+  if (FLAG_track_constant_fields && FLAG_modify_field_representation_inplace) {
+    SameMapChecker checker;
+    TestTransitionTo(transition_op1, transition_op2, checker);
+  } else {
+    FieldGeneralizationChecker checker(kPropCount - 1,
+                                       PropertyConstness::kMutable,
+                                       Representation::Tagged(), any_type);
+    TestTransitionTo(transition_op1, transition_op2, checker);
+  }
 }
 
 

test/cctest/test-heap-profiler.cc

@@ -2948,7 +2948,8 @@ TEST(WeakContainers) {
   CHECK_NE(0, count);
   for (int i = 0; i < count; ++i) {
     const v8::HeapGraphEdge* prop = dependent_code->GetChild(i);
-    CHECK_EQ(v8::HeapGraphEdge::kInternal, prop->GetType());
+    CHECK(prop->GetType() == v8::HeapGraphEdge::kInternal ||
+          prop->GetType() == v8::HeapGraphEdge::kWeak);
   }
 }
 

test/mjsunit/compiler/field-representation-tracking.js

@@ -0,0 +1,74 @@
+// 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 --modify-field-representation-inplace
+// Flags: --no-always-opt --opt
+
+// Test that code embedding accesses to a Smi field gets properly
+// deoptimized if s->t field representation changes are done in-place.
+(function() {
+  function O(x) { this.x = x; }
+
+  function foo(o) { return o.x; }
+  %PrepareFunctionForOptimization(foo);
+  foo(new O(1));
+  foo(new O(2));
+  %OptimizeFunctionOnNextCall(foo);
+  foo(new O(3));
+  assertOptimized(foo);
+
+  new O(null);
+  assertUnoptimized(foo);
+})();
+
+// Test that code embedding assignments to a Smi field gets properly
+// deoptimized if s->t field representation changes are done in-place.
+(function() {
+  function O(x) { this.x = x; }
+
+  function foo(o) { o.x = 0; }
+  %PrepareFunctionForOptimization(foo);
+  foo(new O(1));
+  foo(new O(2));
+  %OptimizeFunctionOnNextCall(foo);
+  foo(new O(3));
+  assertOptimized(foo);
+
+  new O(null);
+  assertUnoptimized(foo);
+})();
+
+// Test that code embedding accesses to a HeapObject field gets properly
+// deoptimized if h->t field representation changes are done in-place.
+(function() {
+  function O(x) { this.x = x; }
+
+  function foo(o) { return o.x; }
+  %PrepareFunctionForOptimization(foo);
+  foo(new O(null));
+  foo(new O("Hello"));
+  %OptimizeFunctionOnNextCall(foo);
+  foo(new O({}));
+  assertOptimized(foo);
+
+  new O(1);
+  assertUnoptimized(foo);
+})();
+
+// Test that code embedding assignments to a Smi field gets properly
+// deoptimized if s->t field representation changes are done in-place.
+(function() {
+  function O(x) { this.x = x; }
+
+  function foo(o) { o.x = true; }
+  %PrepareFunctionForOptimization(foo);
+  foo(new O(null));
+  foo(new O("Hello"));
+  %OptimizeFunctionOnNextCall(foo);
+  foo(new O({}));
+  assertOptimized(foo);
+
+  new O(1);
+  assertUnoptimized(foo);
+})();

test/mjsunit/field-representation-tracking-clone-object.js

@@ -0,0 +1,21 @@
+// 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 foo(o) { return {...o}; }
+
+// Warmup the CloneObjectIC with `o.x` being
+// a HeapObject field.
+var o = {data:null};
+%PrepareFunctionForOptimization(foo);
+foo(o);
+foo(o);
+
+// Now update the field representation of o.x
+// in-place from HeapObject to Tagged and make
+// sure that this is handled properly in the
+// fast-path for CloneObjectIC.
+o.data = 1;
+assertEquals(1, %GetProperty(foo(o), "data"));

test/mjsunit/field-representation-tracking.js

@@ -0,0 +1,31 @@
+// 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 --modify-field-representation-inplace
+
+// Test that s->t field representation changes are done in-place.
+(function() {
+  function O(x) { this.x = x; }
+
+  const a = new O(42);
+  const b = new O(-8);
+  assertTrue(%HaveSameMap(a, b));
+  a.x = null;
+  assertTrue(%HaveSameMap(a, b));
+  b.x = null;
+  assertTrue(%HaveSameMap(a, b));
+})();
+
+// Test that h->t field representation changes are done in-place.
+(function() {
+  function O(x) { this.x = x; }
+
+  const a = new O(null);
+  const b = new O("Hello");
+  assertTrue(%HaveSameMap(a, b));
+  a.x = 1;
+  assertTrue(%HaveSameMap(a, b));
+  b.x = 2;
+  assertTrue(%HaveSameMap(a, b));
+})();

test/mjsunit/field-type-tracking.js

@@ -148,11 +148,11 @@
   var f2 = new Narf(2);
   var f3 = new Narf(3);
   function baz(f, y) { f.y = y; }
-  baz(f1, {y: 9});
-  baz(f2, {y: 9});
-  baz(f2, {y: 9});
+  baz(f1, {b: 9});
+  baz(f2, {b: 9});
+  baz(f2, {b: 9});
   %OptimizeFunctionOnNextCall(baz);
-  baz(f2, {y: 9});
+  baz(f2, {b: 9});
   baz(f3, {a: -1});
   assertUnoptimized(baz);
 })();

test/mjsunit/track-fields.js

@@ -26,6 +26,7 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 // Flags: --track-fields --track-double-fields --allow-natives-syntax
+// Flags: --modify-field-representation-inplace
 
 // Test transitions caused by changes to field representations.
 
@@ -95,7 +96,7 @@ o6.b = 1.5;
 assertFalse(%HaveSameMap(o6, o7));
 // Smi, double, object.
 o7.c = {};
-assertFalse(%HaveSameMap(o6, o7));
+assertTrue(%HaveSameMap(o6, o7));
 // Smi, double, object.
 o6.c = {};
 assertTrue(%HaveSameMap(o6, o7));