[compiler] Use MakeRef in heap-refs.cc

Also add convenience overloads that take handles, and use them in
access-info.cc.

Bug: v8:7790
Change-Id: I47e14b407b6a57c15da06b0396f8ae9ebb3a447d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2871445
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74352}

src/compiler/access-info.cc

@@ -385,7 +385,7 @@ AccessInfoFactory::AccessInfoFactory(JSHeapBroker* broker,
 base::Optional<ElementAccessInfo> AccessInfoFactory::ComputeElementAccessInfo(
     Handle<Map> map, AccessMode access_mode) const {
   // Check if it is safe to inline element access for the {map}.
-  base::Optional<MapRef> map_ref = TryMakeRef(broker(), *map);
+  base::Optional<MapRef> map_ref = TryMakeRef(broker(), map);
   if (!map_ref.has_value()) return {};
   if (!CanInlineElementAccess(*map_ref)) return base::nullopt;
   ElementsKind const elements_kind = map_ref->elements_kind();
@@ -446,7 +446,7 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
   Type field_type = Type::NonInternal();
   MaybeHandle<Map> field_map;
 
-  base::Optional<MapRef> map_ref = TryMakeRef(broker(), *map);
+  base::Optional<MapRef> map_ref = TryMakeRef(broker(), map);
   if (!map_ref.has_value()) return Invalid();
 
   ZoneVector<CompilationDependency const*> unrecorded_dependencies(zone());
@@ -485,7 +485,7 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
       // Remember the field map, and try to infer a useful type.
       Handle<Map> map = broker()->CanonicalPersistentHandle(
           descriptors_field_type->AsClass());
-      base::Optional<MapRef> maybe_ref = TryMakeRef(broker(), *map);
+      base::Optional<MapRef> maybe_ref = TryMakeRef(broker(), map);
       if (!maybe_ref.has_value()) return Invalid();
       field_type = Type::For(*maybe_ref);
       field_map = MaybeHandle<Map>(map);
@@ -861,7 +861,7 @@ PropertyAccessInfo AccessInfoFactory::ComputePropertyAccessInfo(
     }
 
     // Walk up the prototype chain.
-    base::Optional<MapRef> map_ref = TryMakeRef(broker(), *map);
+    base::Optional<MapRef> map_ref = TryMakeRef(broker(), map);
     if (!map_ref.has_value()) return Invalid();
     if (!map_ref->TrySerializePrototype()) return Invalid();
 
@@ -1017,7 +1017,7 @@ base::Optional<ElementAccessInfo> AccessInfoFactory::ConsolidateElementLoad(
 
   DCHECK(!feedback.transition_groups().front().empty());
   Handle<Map> first_map = feedback.transition_groups().front().front();
-  base::Optional<MapRef> first_map_ref = TryMakeRef(broker(), *first_map);
+  base::Optional<MapRef> first_map_ref = TryMakeRef(broker(), first_map);
   if (!first_map_ref.has_value()) return {};
   InstanceType instance_type = first_map_ref->instance_type();
   ElementsKind elements_kind = first_map_ref->elements_kind();
@@ -1025,7 +1025,7 @@ base::Optional<ElementAccessInfo> AccessInfoFactory::ConsolidateElementLoad(
   ZoneVector<Handle<Map>> maps(zone());
   for (auto const& group : feedback.transition_groups()) {
     for (Handle<Map> map_handle : group) {
-      base::Optional<MapRef> map = TryMakeRef(broker(), *map_handle);
+      base::Optional<MapRef> map = TryMakeRef(broker(), map_handle);
       if (!map.has_value()) return {};
       if (map->instance_type() != instance_type ||
           !CanInlineElementAccess(*map)) {
@@ -1112,7 +1112,7 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
   MaybeHandle<Map> field_map;
 
   base::Optional<MapRef> transition_map_ref =
-      TryMakeRef(broker(), *transition_map);
+      TryMakeRef(broker(), transition_map);
   if (!transition_map_ref.has_value()) return Invalid();
 
   ZoneVector<CompilationDependency const*> unrecorded_dependencies(zone());
@@ -1154,7 +1154,7 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
       // Remember the field map, and try to infer a useful type.
       Handle<Map> map = broker()->CanonicalPersistentHandle(
           descriptors_field_type->AsClass());
-      base::Optional<MapRef> map_ref = TryMakeRef(broker(), *map);
+      base::Optional<MapRef> map_ref = TryMakeRef(broker(), map);
       if (!map_ref.has_value()) return Invalid();
       field_type = Type::For(*map_ref);
       field_map = map;

src/compiler/heap-refs.cc

@@ -3205,10 +3205,8 @@ int ArrayBoilerplateDescriptionRef::constants_elements_length() const {
 ObjectRef FixedArrayRef::get(int i) const { return TryGet(i).value(); }
 
 base::Optional<ObjectRef> FixedArrayRef::TryGet(int i) const {
-  ObjectData* data = broker()->TryGetOrCreateData(
-      broker()->CanonicalPersistentHandle(object()->get(i, kRelaxedLoad)));
-  if (data == nullptr) return {};
-  return ObjectRef{broker(), data};
+  return TryMakeRef(broker(), broker()->CanonicalPersistentHandle(
+                                  object()->get(i, kRelaxedLoad)));
 }
 
 Float64 FixedDoubleArrayRef::GetFromImmutableFixedDoubleArray(int i) const {
@@ -3462,9 +3460,9 @@ HolderLookupResult FunctionTemplateInfoRef::LookupHolderOfExpectedType(
     }
     Handle<JSObject> prototype =
         broker()->CanonicalPersistentHandle(raw_prototype);
-    if (ObjectData* data = broker()->TryGetOrCreateData(prototype)) {
-      return HolderLookupResult(CallOptimization::kHolderFound,
-                                JSObjectRef(broker(), data));
+    base::Optional<JSObjectRef> prototype_ref = TryMakeRef(broker(), prototype);
+    if (prototype_ref.has_value()) {
+      return HolderLookupResult(CallOptimization::kHolderFound, prototype_ref);
     }
 
     TRACE_BROKER_MISSING(broker(),
@@ -3554,15 +3552,15 @@ base::Optional<FeedbackVectorRef> FeedbackCellRef::value() const {
     // Note that we use the synchronized accessor.
     Object value = object()->value(kAcquireLoad);
     if (!value.IsFeedbackVector()) return base::nullopt;
-    auto vector_handle = broker()->CanonicalPersistentHandle(value);
-    ObjectData* vector = broker()->TryGetOrCreateData(vector_handle);
-    if (vector) {
-      return FeedbackVectorRef(broker(), vector);
-    }
-    TRACE_BROKER_MISSING(
+    base::Optional<FeedbackVectorRef> vector_ref = TryMakeRef(
         broker(),
-        "Unable to retrieve FeedbackVector from FeedbackCellRef " << *this);
-    return base::nullopt;
+        broker()->CanonicalPersistentHandle(FeedbackVector::cast(value)));
+    if (!vector_ref.has_value()) {
+      TRACE_BROKER_MISSING(
+          broker(),
+          "Unable to retrieve FeedbackVector from FeedbackCellRef " << *this);
+    }
+    return vector_ref;
   }
   ObjectData* vector = ObjectRef::data()->AsFeedbackCell()->value();
   return FeedbackVectorRef(broker(), vector->AsFeedbackVector());
@@ -3595,17 +3593,16 @@ void MapRef::SerializeRootMap() {
 // deleting serialization.
 base::Optional<MapRef> MapRef::FindRootMap() const {
   if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
-    // TODO(solanes): Remove the TryGetOrCreateData part when Map is moved to
+    // TODO(solanes): Change TryMakeRef to MakeRef when Map is moved to
     // kNeverSerialized.
-    ObjectData* root_map =
-        broker()->TryGetOrCreateData(broker()->CanonicalPersistentHandle(
-            object()->FindRootMap(broker()->isolate())));
-    if (root_map) {
-      // TODO(solanes, v8:7790): Consider caching the result of the root map.
-      return MapRef(broker(), root_map);
+    // TODO(solanes, v8:7790): Consider caching the result of the root map.
+    base::Optional<MapRef> root_map =
+        TryMakeRef(broker(), broker()->CanonicalPersistentHandle(
+                                 object()->FindRootMap(broker()->isolate())));
+    if (!root_map.has_value()) {
+      TRACE_BROKER_MISSING(broker(), "root map for object " << *this);
     }
-    TRACE_BROKER_MISSING(broker(), "root map for object " << *this);
-    return base::nullopt;
+    return root_map;
   }
   ObjectData* map_data = data()->AsMap()->FindRootMap();
   if (map_data != nullptr) {
@@ -4154,22 +4151,22 @@ base::Optional<ObjectRef> DescriptorArrayRef::GetStrongValue(
     InternalIndex descriptor_index) const {
   if (data_->should_access_heap()) {
     HeapObject heap_object;
-    if (object()
-            ->GetValue(descriptor_index)
-            .GetHeapObjectIfStrong(&heap_object)) {
-      // Since the descriptors in the descriptor array can be changed in-place
-      // via DescriptorArray::Replace, we might get a value that we haven't seen
-      // before.
-      ObjectData* data = broker()->TryGetOrCreateData(
-          broker()->CanonicalPersistentHandle(heap_object));
-      if (data) return ObjectRef(broker(), data);
-
+    if (!object()
+             ->GetValue(descriptor_index)
+             .GetHeapObjectIfStrong(&heap_object)) {
+      return {};
+    }
+    // Since the descriptors in the descriptor array can be changed in-place
+    // via DescriptorArray::Replace, we might get a value that we haven't seen
+    // before.
+    base::Optional<ObjectRef> ref =
+        TryMakeRef(broker(), broker()->CanonicalPersistentHandle(heap_object));
+    if (!ref.has_value()) {
       TRACE_BROKER_MISSING(broker(), "strong value for descriptor array "
                                          << *this << " at index "
                                          << descriptor_index.as_int());
-      // Fall through to the base::nullopt below.
     }
-    return base::nullopt;
+    return ref;
   }
   ObjectData* value =
       data()->AsDescriptorArray()->GetStrongValue(descriptor_index);
@@ -4391,14 +4388,11 @@ void SharedFunctionInfoRef::SerializeScopeInfoChain() {
 base::Optional<FunctionTemplateInfoRef>
 SharedFunctionInfoRef::function_template_info() const {
   if (data_->should_access_heap()) {
-    if (object()->IsApiFunction()) {
-      ObjectData* data =
-          broker()->TryGetOrCreateData(broker()->CanonicalPersistentHandle(
-              object()->function_data(kAcquireLoad)));
-      if (data == nullptr) return base::nullopt;
-      return FunctionTemplateInfoRef(broker(), data, true);
-    }
-    return base::nullopt;
+    if (!object()->IsApiFunction()) return {};
+    return TryMakeRef(
+        broker(),
+        broker()->CanonicalPersistentHandle(
+            FunctionTemplateInfo::cast(object()->function_data(kAcquireLoad))));
   }
   ObjectData* function_template_info =
       data()->AsSharedFunctionInfo()->function_template_info();

src/compiler/heap-refs.h

@@ -140,6 +140,8 @@ enum class RefSerializationKind {
 HEAP_BROKER_OBJECT_LIST(FORWARD_DECL)
 #undef FORWARD_DECL
 
+class ObjectRef;
+
 template <class T>
 struct ref_traits;
 
@@ -152,6 +154,16 @@ struct ref_traits;
 HEAP_BROKER_OBJECT_LIST(REF_TRAITS)
 #undef REF_TYPE
 
+template <>
+struct ref_traits<Object> {
+  using ref_type = ObjectRef;
+  // Note: While a bit awkward, this artificial ref serialization kind value is
+  // okay: smis are never-serialized, and we never create raw non-smi
+  // ObjectRefs (they would at least be HeapObjectRefs instead).
+  static constexpr RefSerializationKind ref_serialization_kind =
+      RefSerializationKind::kNeverSerialized;
+};
+
 class V8_EXPORT_PRIVATE ObjectRef {
  public:
   enum class BackgroundSerialization {

src/compiler/js-heap-broker.h

@@ -566,7 +566,8 @@ class V8_NODISCARD UnparkedScopeIfNeeded {
 // or
 //
 //  FooRef ref = MakeRef(broker, o);
-template <class T>
+template <class T,
+          typename = std::enable_if_t<std::is_convertible<T*, Object*>::value>>
 base::Optional<typename ref_traits<T>::ref_type> TryMakeRef(
     JSHeapBroker* broker, T object) {
   ObjectData* data = broker->TryGetOrCreateData(object);
@@ -574,11 +575,26 @@ base::Optional<typename ref_traits<T>::ref_type> TryMakeRef(
   return {typename ref_traits<T>::ref_type(broker, data)};
 }
 
-template <class T>
+template <class T,
+          typename = std::enable_if_t<std::is_convertible<T*, Object*>::value>>
+base::Optional<typename ref_traits<T>::ref_type> TryMakeRef(
+    JSHeapBroker* broker, Handle<T> object) {
+  return TryMakeRef(broker, *object);
+}
+
+template <class T,
+          typename = std::enable_if_t<std::is_convertible<T*, Object*>::value>>
 typename ref_traits<T>::ref_type MakeRef(JSHeapBroker* broker, T object) {
   return TryMakeRef(broker, object).value();
 }
 
+template <class T,
+          typename = std::enable_if_t<std::is_convertible<T*, Object*>::value>>
+typename ref_traits<T>::ref_type MakeRef(JSHeapBroker* broker,
+                                         Handle<T> object) {
+  return MakeRef(broker, *object);
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8