[compiler] Pass actual type/rep into dependencies

.. instead of recalculating them at the risk of getting different
answers.

In a concurrent setting, repeated type/rep calculations are not
guaranteed to return the same answer. Instead, calculate them once and
pass them into dependency creation methods.

Note with this CL we now get the type/rep off the holder map and not
the field owner map. The results should be identical and behavior
should not change (verified by CHECKs).

Bug: v8:7790
Change-Id: I2b4c3bb8907082c69448ca743d3c8740cd8f71f3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3055306Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75951}

src/compiler/access-info.cc

@@ -448,22 +448,22 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
       return Invalid();
     }
   }
+
+  Handle<FieldType> descriptors_field_type =
+      broker()->CanonicalPersistentHandle(
+          descriptors->GetFieldType(descriptor));
+
   if (details_representation.IsSmi()) {
     field_type = Type::SignedSmall();
     unrecorded_dependencies.push_back(
-        dependencies()->FieldRepresentationDependencyOffTheRecord(map,
-                                                                  descriptor));
+        dependencies()->FieldRepresentationDependencyOffTheRecord(
+            map, descriptor, details_representation));
   } else if (details_representation.IsDouble()) {
     field_type = type_cache_->kFloat64;
     unrecorded_dependencies.push_back(
-        dependencies()->FieldRepresentationDependencyOffTheRecord(map,
-                                                                  descriptor));
+        dependencies()->FieldRepresentationDependencyOffTheRecord(
+            map, descriptor, details_representation));
   } else if (details_representation.IsHeapObject()) {
-    // Extract the field type from the property details (make sure its
-    // representation is TaggedPointer to reflect the heap object case).
-    Handle<FieldType> descriptors_field_type =
-        broker()->CanonicalPersistentHandle(
-            descriptors->GetFieldType(descriptor));
     if (descriptors_field_type->IsNone()) {
       // Store is not safe if the field type was cleared.
       if (access_mode == AccessMode::kStore) {
@@ -474,8 +474,8 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
       // about the contents now.
     }
     unrecorded_dependencies.push_back(
-        dependencies()->FieldRepresentationDependencyOffTheRecord(map,
-                                                                  descriptor));
+        dependencies()->FieldRepresentationDependencyOffTheRecord(
+            map, descriptor, details_representation));
     if (descriptors_field_type->IsClass()) {
       // Remember the field map, and try to infer a useful type.
       base::Optional<MapRef> maybe_field_map =
@@ -490,7 +490,8 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
   // TODO(turbofan): We may want to do this only depending on the use
   // of the access info.
   unrecorded_dependencies.push_back(
-      dependencies()->FieldTypeDependencyOffTheRecord(map, descriptor));
+      dependencies()->FieldTypeDependencyOffTheRecord(
+          map, descriptor, MakeRef<Object>(broker(), descriptors_field_type)));
 
   PropertyConstness constness;
   if (details.IsReadOnly() && !details.IsConfigurable()) {
@@ -1142,7 +1143,7 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
     }
     unrecorded_dependencies.push_back(
         dependencies()->FieldRepresentationDependencyOffTheRecord(
-            transition_map, number));
+            transition_map, number, details_representation));
   } else if (details_representation.IsDouble()) {
     field_type = type_cache_->kFloat64;
     if (!broker()->is_concurrent_inlining()) {
@@ -1153,7 +1154,7 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
     }
     unrecorded_dependencies.push_back(
         dependencies()->FieldRepresentationDependencyOffTheRecord(
-            transition_map, number));
+            transition_map, number, details_representation));
   } else if (details_representation.IsHeapObject()) {
     // Extract the field type from the property details (make sure its
     // representation is TaggedPointer to reflect the heap object case).
@@ -1171,11 +1172,12 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
     }
     unrecorded_dependencies.push_back(
         dependencies()->FieldRepresentationDependencyOffTheRecord(
-            transition_map, number));
+            transition_map, number, details_representation));
     if (descriptors_field_type->IsClass()) {
       unrecorded_dependencies.push_back(
-          dependencies()->FieldTypeDependencyOffTheRecord(transition_map,
-                                                          number));
+          dependencies()->FieldTypeDependencyOffTheRecord(
+              transition_map, number,
+              MakeRef<Object>(broker(), descriptors_field_type)));
       // Remember the field map, and try to infer a useful type.
       base::Optional<MapRef> maybe_field_map =
           TryMakeRef(broker(), descriptors_field_type->AsClass());

src/compiler/compilation-dependencies.cc

@@ -939,25 +939,25 @@ CompilationDependencies::TransitionDependencyOffTheRecord(
 
 CompilationDependency const*
 CompilationDependencies::FieldRepresentationDependencyOffTheRecord(
-    const MapRef& map, InternalIndex descriptor) const {
+    const MapRef& map, InternalIndex descriptor,
+    Representation representation) const {
   DCHECK(!map.IsNeverSerializedHeapObject());
   MapRef owner = map.FindFieldOwner(descriptor);
   DCHECK(!owner.IsNeverSerializedHeapObject());
-  PropertyDetails details = owner.GetPropertyDetails(descriptor);
-  CHECK(details.representation().Equals(
-      map.GetPropertyDetails(descriptor).representation()));
+  CHECK(owner.GetPropertyDetails(descriptor)
+            .representation()
+            .Equals(representation));
   return zone_->New<FieldRepresentationDependency>(owner, descriptor,
-                                                   details.representation());
+                                                   representation);
 }
 
 CompilationDependency const*
 CompilationDependencies::FieldTypeDependencyOffTheRecord(
-    const MapRef& map, InternalIndex descriptor) const {
+    const MapRef& map, InternalIndex descriptor, const ObjectRef& type) const {
   DCHECK(!map.IsNeverSerializedHeapObject());
   MapRef owner = map.FindFieldOwner(descriptor);
   DCHECK(!owner.IsNeverSerializedHeapObject());
-  ObjectRef type = owner.GetFieldType(descriptor);
-  CHECK(type.equals(map.GetFieldType(descriptor)));
+  CHECK(owner.GetFieldType(descriptor).equals(type));
   return zone_->New<FieldTypeDependency>(owner, descriptor, type);
 }
 

src/compiler/compilation-dependencies.h

@@ -149,12 +149,14 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // Gather the assumption that the field representation of a field does not
   // change. The field is identified by the arguments.
   CompilationDependency const* FieldRepresentationDependencyOffTheRecord(
-      const MapRef& map, InternalIndex descriptor) const;
+      const MapRef& map, InternalIndex descriptor,
+      Representation representation) const;
 
   // Gather the assumption that the field type of a field does not change. The
   // field is identified by the arguments.
   CompilationDependency const* FieldTypeDependencyOffTheRecord(
-      const MapRef& map, InternalIndex descriptor) const;
+      const MapRef& map, InternalIndex descriptor,
+      const ObjectRef& /* Contains a FieldType underneath. */ type) const;
 
  private:
   Zone* const zone_;