Constant field tracking for arrays.

This adds constant field tracking for arrays. To prevent changing the
field in some other elements-kind-branch of transition tree, we only
use the const information in the optimizing compiler if the map is not
an array map or if the map is stable (since stable maps cannot
transition to a different elements-kind-branch without deopt).

Some more details:
https://docs.google.com/document/d/1r2GAvdi_wudDS6iRUfdPw0gxWMfV-IX1PqKgwW47FyE

Bug: chromium:912162, v8:8361
Change-Id: Iea1b2f03ddee16205c2141ac5e813a973dd23cf4
Reviewed-on: https://chromium-review.googlesource.com/c/1454606
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59409}

src/compiler/compilation-dependencies.cc

@@ -164,24 +164,17 @@ class FieldTypeDependency final : public CompilationDependencies::Dependency {
   // TODO(neis): Once the concurrent compiler frontend is always-on, we no
   // longer need to explicitly store the type.
   FieldTypeDependency(const MapRef& owner, int descriptor,
-                      const ObjectRef& type, PropertyConstness constness)
-      : owner_(owner),
-        descriptor_(descriptor),
-        type_(type),
-        constness_(constness) {
+                      const ObjectRef& type)
+      : owner_(owner), descriptor_(descriptor), type_(type) {
     DCHECK(owner_.equals(owner_.FindFieldOwner(descriptor_)));
     DCHECK(type_.equals(owner_.GetFieldType(descriptor_)));
-    DCHECK_EQ(constness_, owner_.GetPropertyDetails(descriptor_).constness());
   }
 
   bool IsValid() const override {
     DisallowHeapAllocation no_heap_allocation;
     Handle<Map> owner = owner_.object();
     Handle<Object> type = type_.object();
-    return *type == owner->instance_descriptors()->GetFieldType(descriptor_) &&
-           constness_ == owner->instance_descriptors()
-                             ->GetDetails(descriptor_)
-                             .constness();
+    return *type == owner->instance_descriptors()->GetFieldType(descriptor_);
   }
 
   void Install(const MaybeObjectHandle& code) override {
@@ -194,7 +187,34 @@ class FieldTypeDependency final : public CompilationDependencies::Dependency {
   MapRef owner_;
   int descriptor_;
   ObjectRef type_;
-  PropertyConstness constness_;
+};
+
+class FieldConstnessDependency final
+    : public CompilationDependencies::Dependency {
+ public:
+  FieldConstnessDependency(const MapRef& owner, int descriptor)
+      : owner_(owner), descriptor_(descriptor) {
+    DCHECK(owner_.equals(owner_.FindFieldOwner(descriptor_)));
+    DCHECK_EQ(PropertyConstness::kConst,
+              owner_.GetPropertyDetails(descriptor_).constness());
+  }
+
+  bool IsValid() const override {
+    DisallowHeapAllocation no_heap_allocation;
+    Handle<Map> owner = owner_.object();
+    return PropertyConstness::kConst ==
+           owner->instance_descriptors()->GetDetails(descriptor_).constness();
+  }
+
+  void Install(const MaybeObjectHandle& code) override {
+    SLOW_DCHECK(IsValid());
+    DependentCode::InstallDependency(owner_.isolate(), code, owner_.object(),
+                                     DependentCode::kFieldOwnerGroup);
+  }
+
+ private:
+  MapRef owner_;
+  int descriptor_;
 };
 
 class GlobalPropertyDependency final
@@ -361,15 +381,37 @@ PretenureFlag CompilationDependencies::DependOnPretenureMode(
   return mode;
 }
 
+PropertyConstness CompilationDependencies::DependOnFieldConstness(
+    const MapRef& map, int descriptor) {
+  MapRef owner = map.FindFieldOwner(descriptor);
+  PropertyConstness constness =
+      owner.GetPropertyDetails(descriptor).constness();
+  if (constness == PropertyConstness::kMutable) return constness;
+
+  // If the map can have fast elements transitions, then the field can be only
+  // considered constant if the map does not transition.
+  if (Map::CanHaveFastTransitionableElementsKind(map.instance_type())) {
+    // If the map can already transition away, let us report the field as
+    // mutable.
+    if (!map.is_stable()) {
+      return PropertyConstness::kMutable;
+    }
+    DependOnStableMap(map);
+  }
+
+  DCHECK_EQ(constness, PropertyConstness::kConst);
+  dependencies_.push_front(new (zone_)
+                               FieldConstnessDependency(owner, descriptor));
+  return PropertyConstness::kConst;
+}
+
 void CompilationDependencies::DependOnFieldType(const MapRef& map,
                                                 int descriptor) {
   MapRef owner = map.FindFieldOwner(descriptor);
   ObjectRef type = owner.GetFieldType(descriptor);
-  PropertyConstness constness =
-      owner.GetPropertyDetails(descriptor).constness();
   DCHECK(type.equals(map.GetFieldType(descriptor)));
-  dependencies_.push_front(
-      new (zone_) FieldTypeDependency(owner, descriptor, type, constness));
+  dependencies_.push_front(new (zone_)
+                               FieldTypeDependency(owner, descriptor, type));
 }
 
 void CompilationDependencies::DependOnGlobalProperty(

src/compiler/compilation-dependencies.h

@@ -55,6 +55,15 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // field is identified by the arguments.
   void DependOnFieldType(const MapRef& map, int descriptor);
 
+  // Return a field's constness and, if kConst, record the assumption that it
+  // remains kConst. The field is identified by the arguments.
+  //
+  // For arrays, arguments objects and value wrappers, only consider the field
+  // kConst if the map is stable (and register stability dependency in that
+  // case).  This is to ensure that fast elements kind transitions cannot be
+  // used to mutate fields without deoptimization of the dependent code.
+  PropertyConstness DependOnFieldConstness(const MapRef& map, int descriptor);
+
   // Record the assumption that neither {cell}'s {CellType} changes, nor the
   // {IsReadOnly()} flag of {cell}'s {PropertyDetails}.
   void DependOnGlobalProperty(const PropertyCellRef& cell);

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

@@ -473,7 +473,10 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
           *(factory()->has_instance_symbol()), *(holder_map.object()));
       CHECK_NE(descriptor_index, DescriptorArray::kNotFound);
       holder_map.SerializeOwnDescriptors();
-      dependencies()->DependOnFieldType(holder_map, descriptor_index);
+      if (dependencies()->DependOnFieldConstness(
+              holder_map, descriptor_index) != PropertyConstness::kConst) {
+        return NoChange();
+      }
     }
 
     if (found_on_proto) {

src/compiler/property-access-builder.cc

@@ -220,7 +220,11 @@ Node* PropertyAccessBuilder::TryBuildLoadConstantDataField(
           MapRef map(broker(),
                      handle(it.GetHolder<HeapObject>()->map(), isolate()));
           map.SerializeOwnDescriptors();  // TODO(neis): Remove later.
-          dependencies()->DependOnFieldType(map, it.GetFieldDescriptorIndex());
+          if (dependencies()->DependOnFieldConstness(
+                  map, it.GetFieldDescriptorIndex()) !=
+              PropertyConstness::kConst) {
+            return nullptr;
+          }
         }
         return value;
       }

src/map-updater.cc

@@ -644,10 +644,10 @@ Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
 
       // If the |new_elements_kind_| is still transitionable then the old map's
       // elements kind is also transitionable and therefore the old descriptors
-      // array must already have non in-place generalizable fields.
-      CHECK_IMPLIES(is_transitionable_fast_elements_kind_,
-                    !Map::IsInplaceGeneralizableField(
-                        next_constness, next_representation, *next_field_type));
+      // array must already have generalized field type.
+      CHECK_IMPLIES(
+          is_transitionable_fast_elements_kind_,
+          Map::IsMostGeneralFieldType(next_representation, *next_field_type));
 
       MaybeObjectHandle wrapped_type(
           Map::WrapFieldType(isolate_, next_field_type));

src/objects-debug.cc

@@ -605,8 +605,7 @@ void JSObject::JSObjectVerify(Isolate* isolate) {
           CHECK(!field_type->NowStable() || field_type->NowContains(value));
         }
         CHECK_IMPLIES(is_transitionable_fast_elements_kind,
-                      !Map::IsInplaceGeneralizableField(details.constness(), r,
-                                                        field_type));
+                      Map::IsMostGeneralFieldType(r, field_type));
       }
     }
 

src/objects/map-inl.h

@@ -102,19 +102,9 @@ InterceptorInfo Map::GetIndexedInterceptor() {
   return InterceptorInfo::cast(info->GetIndexedPropertyHandler());
 }
 
-bool Map::IsInplaceGeneralizableField(PropertyConstness constness,
-                                      Representation representation,
-                                      FieldType field_type) {
-  if (FLAG_track_constant_fields && FLAG_modify_map_inplace &&
-      (constness == PropertyConstness::kConst)) {
-    // VariableMode::kConst -> PropertyConstness::kMutable field generalization
-    // may happen in-place.
-    return true;
-  }
-  if (representation.IsHeapObject() && !field_type->IsAny()) {
-    return true;
-  }
-  return false;
+bool Map::IsMostGeneralFieldType(Representation representation,
+                                 FieldType field_type) {
+  return !representation.IsHeapObject() || field_type->IsAny();
 }
 
 bool Map::CanHaveFastTransitionableElementsKind(InstanceType instance_type) {
@@ -135,13 +125,7 @@ void Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
     // 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
-    // do not have fields that can be generalized in-place (without creation
-    // of a new map).
-    if (FLAG_track_constant_fields && FLAG_modify_map_inplace) {
-      // The constness is either already PropertyConstness::kMutable or should
-      // become PropertyConstness::kMutable if it was VariableMode::kConst.
-      *constness = PropertyConstness::kMutable;
-    }
+    // 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.

src/objects/map.cc

@@ -2443,9 +2443,8 @@ bool Map::EquivalentToForElementsKindTransition(const Map other) const {
   for (int i = 0; i < nof; i++) {
     PropertyDetails details = descriptors->GetDetails(i);
     if (details.location() == kField) {
-      DCHECK(!IsInplaceGeneralizableField(details.constness(),
-                                          details.representation(),
-                                          descriptors->GetFieldType(i)));
+      DCHECK(IsMostGeneralFieldType(details.representation(),
+                                    descriptors->GetFieldType(i)));
     }
   }
 #endif

src/objects/map.h

@@ -504,11 +504,10 @@ class Map : public HeapObject {
                               int modify_index, PropertyConstness new_constness,
                               Representation new_representation,
                               Handle<FieldType> new_field_type);
-  // Returns true if |descriptor|'th property is a field that may be generalized
-  // by just updating current map.
-  static inline bool IsInplaceGeneralizableField(PropertyConstness constness,
-                                                 Representation representation,
-                                                 FieldType field_type);
+  // Returns true if the |field_type| is the most general one for
+  // given |representation|.
+  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

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

@@ -124,18 +124,10 @@ class Expectations {
     kinds_[index] = kind;
     locations_[index] = location;
     if (kind == kData && location == kField &&
-        IsTransitionableFastElementsKind(elements_kind_) &&
-        Map::IsInplaceGeneralizableField(constness, representation,
-                                         FieldType::cast(*value))) {
-      // Maps with transitionable elements kinds must have non in-place
-      // generalizable fields.
-      if (FLAG_track_constant_fields && FLAG_modify_map_inplace &&
-          constness == PropertyConstness::kConst) {
-        constness = PropertyConstness::kMutable;
-      }
-      if (representation.IsHeapObject() && !FieldType::cast(*value)->IsAny()) {
-        value = FieldType::Any(isolate_);
-      }
+        IsTransitionableFastElementsKind(elements_kind_)) {
+      // Maps with transitionable elements kinds must have the most general
+      // field type.
+      value = FieldType::Any(isolate_);
     }
     constnesses_[index] = constness;
     attributes_[index] = attributes;
@@ -1140,6 +1132,7 @@ static void TestReconfigureDataFieldAttribute_GeneralizeFieldTrivial(
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
   dependencies.DependOnFieldType(map_ref, kSplitProp);
+  dependencies.DependOnFieldConstness(map_ref, kSplitProp);
 
   // Reconfigure attributes of property |kSplitProp| of |map2| to NONE, which
   // should generalize representations in |map1|.
@@ -1921,7 +1914,9 @@ static void TestReconfigureElementsKind_GeneralizeFieldTrivial(
   CompilationDependencies dependencies(isolate, &zone);
   MapRef map_ref(&broker, map);
   map_ref.SerializeOwnDescriptors();
+
   dependencies.DependOnFieldType(map_ref, kDiffProp);
+  dependencies.DependOnFieldConstness(map_ref, kDiffProp);
 
   // Reconfigure elements kinds of |map2|, which should generalize
   // representations in |map|.
@@ -1942,7 +1937,8 @@ static void TestReconfigureElementsKind_GeneralizeFieldTrivial(
                             expected.representation, expected.type);
   CHECK(!map->is_deprecated());
   CHECK_EQ(*map, *new_map);
-  CHECK(dependencies.AreValid());
+  CHECK_EQ(IsGeneralizableTo(to.constness, from.constness),
+           dependencies.AreValid());
 
   CHECK(!new_map->is_deprecated());
   CHECK(expectations.Check(*new_map));

test/mjsunit/regress/regress-912162.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
+
+var a = new Array();
+a.prototype = a;
+
+function f() {
+ a.length = 0x2000001;
+ a.push();
+}
+
+({}).__proto__ = a;
+
+f()
+f()
+
+a.length = 1;
+a.fill(-255);
+
+%HeapObjectVerify(a);