[map] Make field representation updates work with elements kind transitions.

Generalize the existing work-around in the method
`Map::GeneralizeIfCanHaveTransitionableFastElementsKind()` to also go to
the most general field representation (in addition to going to the most
field type) for objects with transitionable fast elements kinds. That
means that we essentially disable field representation tracking for
arrays, arguments objects and value wrappers (for which the field type
tracking is already disabled).

Drive-by-fix: Remove the `constness` parameter to the above mentioned
helper method. And fix the printing of the descriptor expectations to
properly print the field type.

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

src/field-type.h

@@ -41,7 +41,7 @@ class FieldType : public Object {
   bool NowIs(FieldType other) const;
   bool NowIs(Handle<FieldType> other) const;
 
-  void PrintTo(std::ostream& os) const;
+  V8_EXPORT_PRIVATE void PrintTo(std::ostream& os) const;
 
   FieldType* operator->() { return this; }
   const FieldType* operator->() const { return this; }

src/map-updater.cc

@@ -147,8 +147,8 @@ Handle<Map> MapUpdater::ReconfigureToDataField(int descriptor,
   }
 
   Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
-      isolate_, old_map_->instance_type(), &new_constness_,
-      &new_representation_, &new_field_type_);
+      isolate_, old_map_->instance_type(), &new_representation_,
+      &new_field_type_);
 
   if (TryReconfigureToDataFieldInplace() == kEnd) return result_map_;
   if (FindRootMap() == kEnd) return result_map_;
@@ -586,8 +586,7 @@ Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
           target_field_type, isolate_);
 
       Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
-          isolate_, instance_type, &next_constness, &next_representation,
-          &next_field_type);
+          isolate_, instance_type, &next_representation, &next_field_type);
 
       MaybeObjectHandle wrapped_type(
           Map::WrapFieldType(isolate_, next_field_type));

src/objects/map-inl.h

@@ -122,19 +122,16 @@ bool Map::CanHaveFastTransitionableElementsKind() const {
 
 // static
 void Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
-    Isolate* isolate, InstanceType instance_type, PropertyConstness* constness,
+    Isolate* isolate, InstanceType instance_type,
     Representation* representation, Handle<FieldType>* field_type) {
   if (CanHaveFastTransitionableElementsKind(instance_type)) {
     // We don't support propagation of field generalization through elements
     // kind transitions because they are inserted into the transition tree
     // before field transitions. In order to avoid complexity of handling
     // such a case we ensure that all maps with transitionable elements kinds
-    // have the most general field type.
-    if (representation->IsHeapObject()) {
-      // The field type is either already Any or should become Any if it was
-      // something else.
-      *field_type = FieldType::Any(isolate);
-    }
+    // have the most general field representation and type.
+    *field_type = FieldType::Any(isolate);
+    *representation = Representation::Tagged();
   }
 }
 

src/objects/map.cc

@@ -447,7 +447,7 @@ MaybeHandle<Map> Map::CopyWithField(Isolate* isolate, Handle<Map> map,
     type = FieldType::Any(isolate);
   } else {
     Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
-        isolate, map->instance_type(), &constness, &representation, &type);
+        isolate, map->instance_type(), &representation, &type);
   }
 
   MaybeObjectHandle wrapped_type = WrapFieldType(isolate, type);

src/objects/map.h

@@ -518,17 +518,16 @@ class Map : public HeapObject {
   static inline bool IsMostGeneralFieldType(Representation representation,
                                             FieldType field_type);
 
-  // Generalizes constness, representation and field_type if objects with given
-  // instance type can have fast elements that can be transitioned by stubs or
-  // optimized code to more general elements kind.
+  // Generalizes representation and field_type if objects with given
+  // instance type can have fast elements that can be transitioned by
+  // stubs or optimized code to more general elements kind.
   // This generalization is necessary in order to ensure that elements kind
   // transitions performed by stubs / optimized code don't silently transition
-  // PropertyConstness::kMutable fields back to VariableMode::kConst state or
+  // fields with representation "Tagged" back to "Smi" or "HeapObject" or
   // fields with HeapObject representation and "Any" type back to "Class" type.
   static inline void GeneralizeIfCanHaveTransitionableFastElementsKind(
       Isolate* isolate, InstanceType instance_type,
-      PropertyConstness* constness, Representation* representation,
-      Handle<FieldType>* field_type);
+      Representation* representation, Handle<FieldType>* field_type);
 
   V8_EXPORT_PRIVATE static Handle<Map> ReconfigureProperty(
       Isolate* isolate, Handle<Map> map, int modify_index,

test/cctest/test-inobject-slack-tracking.cc

@@ -85,8 +85,11 @@ static double GetDoubleFieldValue(JSObject obj, FieldIndex field_index) {
     return obj->RawFastDoublePropertyAt(field_index);
   } else {
     Object value = obj->RawFastPropertyAt(field_index);
-    CHECK(value->IsMutableHeapNumber());
-    return MutableHeapNumber::cast(value)->value();
+    if (value->IsMutableHeapNumber()) {
+      return MutableHeapNumber::cast(value)->value();
+    } else {
+      return value->Number();
+    }
   }
 }
 

test/mjsunit/regress/regress-crbug-959645-1.js

+// 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-representations-inplace
+
+function f(array, x) {
+  array.x = x;
+  array[0] = 1.1;
+  return array;
+}
+
+f([1], 1);
+f([2], 1);
+%HeapObjectVerify(f([3], undefined));

test/mjsunit/regress/regress-crbug-959645-2.js

+// 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-representations-inplace
+
+function f(array, x) {
+  array.x = x;
+  array[0] = undefined;
+  return array;
+}
+
+f([1.1], 1);
+f([2.2], 1);
+%HeapObjectVerify(f([3.3], undefined));