Revert "[compiler] Remove one ObjectRef constructor"

This reverts commit 59bb4325.

Reason for revert: 4683d6fe broke TSAN, reverting all its dependencies first (including this)
https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN/36744/overview


Original change's description:
> [compiler] Remove one ObjectRef constructor
>
> Remove the handle-taking ObjectRef constructor in favor of
> (Try)MakeRef as bottleneck.
>
> Bug: v8:7790
> Change-Id: I3cc3a1dcef4bac53a91c573d1a532332b88c6eb4
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2883664
> Commit-Queue: Georg Neis <neis@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#74593}

Bug: v8:7790
Change-Id: Ifdecf93a3a8c09a3da7118a269fc66c2ae0f1a09
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2901988Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
Commit-Queue: Sathya Gunasekaran  <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74618}

src/compiler/bytecode-graph-builder.cc

@@ -1594,7 +1594,7 @@ void BytecodeGraphBuilder::VisitLdaSmi() {
 }
 
 void BytecodeGraphBuilder::VisitLdaConstant() {
-  ObjectRef object = MakeRef(broker(), GetConstantForIndexOperand(0));
+  ObjectRef object(broker(), GetConstantForIndexOperand(0));
   Node* node = jsgraph()->Constant(object);
   environment()->BindAccumulator(node);
 }
@@ -1826,7 +1826,7 @@ void BytecodeGraphBuilder::VisitStaCurrentContextSlot() {
 void BytecodeGraphBuilder::BuildLdaLookupSlot(TypeofMode typeof_mode) {
   PrepareEagerCheckpoint();
   Node* name =
-      jsgraph()->Constant(MakeRef(broker(), GetConstantForIndexOperand(0)));
+      jsgraph()->Constant(ObjectRef(broker(), GetConstantForIndexOperand(0)));
   const Operator* op =
       javascript()->CallRuntime(typeof_mode == TypeofMode::kNotInside
                                     ? Runtime::kLoadLookupSlot
@@ -1978,8 +1978,8 @@ void BytecodeGraphBuilder::BuildLdaLookupContextSlot(TypeofMode typeof_mode) {
     // Slow path, do a runtime load lookup.
     set_environment(slow_environment);
     {
-      Node* name =
-          jsgraph()->Constant(MakeRef(broker(), GetConstantForIndexOperand(0)));
+      Node* name = jsgraph()->Constant(
+          ObjectRef(broker(), GetConstantForIndexOperand(0)));
 
       const Operator* op =
           javascript()->CallRuntime(typeof_mode == TypeofMode::kNotInside
@@ -2061,7 +2061,7 @@ void BytecodeGraphBuilder::VisitStaLookupSlot() {
   PrepareEagerCheckpoint();
   Node* value = environment()->LookupAccumulator();
   Node* name =
-      jsgraph()->Constant(MakeRef(broker(), GetConstantForIndexOperand(0)));
+      jsgraph()->Constant(ObjectRef(broker(), GetConstantForIndexOperand(0)));
   int bytecode_flags = bytecode_iterator().GetFlagOperand(1);
   LanguageMode language_mode = static_cast<LanguageMode>(
       interpreter::StoreLookupSlotFlags::LanguageModeBit::decode(
@@ -2981,7 +2981,7 @@ void BytecodeGraphBuilder::VisitThrowReferenceErrorIfHole() {
   Node* check_for_hole = NewNode(simplified()->ReferenceEqual(), accumulator,
                                  jsgraph()->TheHoleConstant());
   Node* name =
-      jsgraph()->Constant(MakeRef(broker(), GetConstantForIndexOperand(0)));
+      jsgraph()->Constant(ObjectRef(broker(), GetConstantForIndexOperand(0)));
   BuildHoleCheckAndThrow(check_for_hole,
                          Runtime::kThrowAccessedUninitializedVariable, name);
 }

src/compiler/heap-refs.cc

@@ -497,7 +497,7 @@ ObjectRef GetOwnFastDataPropertyFromHeap(JSHeapBroker* broker,
                                          FieldIndex field_index) {
   Handle<Object> constant =
       JSObject::FastPropertyAt(receiver, representation, field_index);
-  return MakeRef(broker, constant);
+  return ObjectRef(broker, constant);
 }
 
 ObjectRef GetOwnDictionaryPropertyFromHeap(JSHeapBroker* broker,
@@ -505,7 +505,7 @@ ObjectRef GetOwnDictionaryPropertyFromHeap(JSHeapBroker* broker,
                                            InternalIndex dict_index) {
   Handle<Object> constant =
       JSObject::DictionaryPropertyAt(receiver, dict_index);
-  return MakeRef(broker, constant);
+  return ObjectRef(broker, constant);
 }
 
 }  // namespace
@@ -3783,8 +3783,8 @@ Maybe<double> ObjectRef::OddballToNumber() const {
 
   switch (type) {
     case OddballType::kBoolean: {
-      ObjectRef true_ref = MakeRef<Object>(
-          broker(), broker()->isolate()->factory()->true_value());
+      ObjectRef true_ref(broker(),
+                         broker()->isolate()->factory()->true_value());
       return this->equals(true_ref) ? Just(1.0) : Just(0.0);
       break;
     }
@@ -3967,6 +3967,13 @@ base::Optional<ObjectRef> SourceTextModuleRef::import_meta() const {
                    data()->AsSourceTextModule()->GetImportMeta(broker()));
 }
 
+ObjectRef::ObjectRef(JSHeapBroker* broker, Handle<Object> object,
+                     bool check_type)
+    : broker_(broker) {
+  CHECK_NE(broker->mode(), JSHeapBroker::kRetired);
+  data_ = broker->GetOrCreateData(object);
+}
+
 namespace {
 
 OddballType GetOddballType(Isolate* isolate, Map map) {

src/compiler/heap-refs.h

@@ -160,11 +160,12 @@ struct ref_traits<Object> {
 
 class V8_EXPORT_PRIVATE ObjectRef {
  public:
+  ObjectRef(JSHeapBroker* broker, Handle<Object> object,
+            bool check_type = true);
   ObjectRef(JSHeapBroker* broker, ObjectData* data, bool check_type = true)
       : data_(data), broker_(broker) {
     CHECK_NOT_NULL(data_);
   }
-
   Handle<Object> object() const;
 
   bool equals(const ObjectRef& other) const;

src/compiler/js-heap-broker.cc

@@ -1103,7 +1103,7 @@ MinimorphicLoadPropertyAccessInfo JSHeapBroker::GetPropertyAccessInfo(
   if (policy == SerializationPolicy::kAssumeSerialized) {
     TRACE_BROKER_MISSING(this, "MinimorphicLoadPropertyAccessInfo for slot "
                                    << source.index() << "  "
-                                   << MakeRef<Object>(this, source.vector));
+                                   << ObjectRef(this, source.vector));
     return MinimorphicLoadPropertyAccessInfo::Invalid();
   }
 
@@ -1113,7 +1113,7 @@ MinimorphicLoadPropertyAccessInfo JSHeapBroker::GetPropertyAccessInfo(
   if (is_concurrent_inlining_) {
     TRACE(this, "Storing MinimorphicLoadPropertyAccessInfo for "
                     << source.index() << "  "
-                    << MakeRef<Object>(this, source.vector));
+                    << ObjectRef(this, source.vector));
     minimorphic_property_access_infos_.insert({source, access_info});
   }
   return access_info;

src/compiler/js-heap-broker.h

@@ -594,7 +594,6 @@ base::Optional<typename ref_traits<T>::ref_type> TryMakeRef(
     JSHeapBroker* broker, Handle<T> object, GetOrCreateDataFlags flags = {}) {
   ObjectData* data = broker->TryGetOrCreateData(object, flags);
   if (data == nullptr) {
-    DCHECK_EQ(flags & kCrashOnError, 0);
     TRACE_BROKER_MISSING(broker, "ObjectData for " << Brief(*object));
     return {};
   }
@@ -604,14 +603,14 @@ base::Optional<typename ref_traits<T>::ref_type> TryMakeRef(
 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, kCrashOnError).value();
+  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 TryMakeRef(broker, object, kCrashOnError).value();
+  return TryMakeRef(broker, object).value();
 }
 
 template <class T,

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

@@ -1469,7 +1469,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadNamed(Node* node) {
   if (m.HasResolvedValue()) {
     ObjectRef object = m.Ref(broker());
     if (object.IsJSFunction() &&
-        name.equals(MakeRef(broker(), factory()->prototype_string()))) {
+        name.equals(ObjectRef(broker(), factory()->prototype_string()))) {
       // Optimize "prototype" property of functions.
       JSFunctionRef function = object.AsJSFunction();
       if (!function.serialized()) return NoChange();
@@ -1484,7 +1484,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadNamed(Node* node) {
       ReplaceWithValue(node, value);
       return Replace(value);
     } else if (object.IsString() &&
-               name.equals(MakeRef(broker(), factory()->length_string()))) {
+               name.equals(ObjectRef(broker(), factory()->length_string()))) {
       // Constant-fold "length" property on constant strings.
       if (!object.AsString().length().has_value()) return NoChange();
       Node* value = jsgraph()->Constant(object.AsString().length().value());
@@ -2210,7 +2210,7 @@ Node* JSNativeContextSpecialization::InlinePropertyGetterCall(
     Node* receiver, ConvertReceiverMode receiver_mode, Node* context,
     Node* frame_state, Node** effect, Node** control,
     ZoneVector<Node*>* if_exceptions, PropertyAccessInfo const& access_info) {
-  ObjectRef constant = MakeRef(broker(), access_info.constant());
+  ObjectRef constant(broker(), access_info.constant());
 
   if (access_info.IsDictionaryProtoAccessorConstant()) {
     // For fast mode holders we recorded dependencies in BuildPropertyLoad.
@@ -2234,7 +2234,7 @@ Node* JSNativeContextSpecialization::InlinePropertyGetterCall(
   } else {
     Node* holder = access_info.holder().is_null()
                        ? receiver
-                       : jsgraph()->Constant(MakeRef(
+                       : jsgraph()->Constant(ObjectRef(
                              broker(), access_info.holder().ToHandleChecked()));
     value = InlineApiCall(receiver, holder, frame_state, nullptr, effect,
                           control, constant.AsFunctionTemplateInfo());
@@ -2255,7 +2255,7 @@ void JSNativeContextSpecialization::InlinePropertySetterCall(
     Node* receiver, Node* value, Node* context, Node* frame_state,
     Node** effect, Node** control, ZoneVector<Node*>* if_exceptions,
     PropertyAccessInfo const& access_info) {
-  ObjectRef constant = MakeRef(broker(), access_info.constant());
+  ObjectRef constant(broker(), access_info.constant());
   Node* target = jsgraph()->Constant(constant);
   // Introduce the call to the setter function.
   if (constant.IsJSFunction()) {
@@ -2269,7 +2269,7 @@ void JSNativeContextSpecialization::InlinePropertySetterCall(
   } else {
     Node* holder = access_info.holder().is_null()
                        ? receiver
-                       : jsgraph()->Constant(MakeRef(
+                       : jsgraph()->Constant(ObjectRef(
                              broker(), access_info.holder().ToHandleChecked()));
     InlineApiCall(receiver, holder, frame_state, value, effect, control,
                   constant.AsFunctionTemplateInfo());
@@ -2366,8 +2366,8 @@ JSNativeContextSpecialization::BuildPropertyLoad(
         InlinePropertyGetterCall(receiver, receiver_mode, context, frame_state,
                                  &effect, &control, if_exceptions, access_info);
   } else if (access_info.IsModuleExport()) {
-    Node* cell =
-        jsgraph()->Constant(MakeRef(broker(), access_info.constant()).AsCell());
+    Node* cell = jsgraph()->Constant(
+        ObjectRef(broker(), access_info.constant()).AsCell());
     value = effect =
         graph()->NewNode(simplified()->LoadField(AccessBuilder::ForCellValue()),
                          cell, effect, control);

src/compiler/property-access-builder.cc

@@ -127,7 +127,7 @@ Node* PropertyAccessBuilder::ResolveHolder(
     PropertyAccessInfo const& access_info, Node* lookup_start_object) {
   Handle<JSObject> holder;
   if (access_info.holder().ToHandle(&holder)) {
-    return jsgraph()->Constant(MakeRef(broker(), holder));
+    return jsgraph()->Constant(ObjectRef(broker(), holder));
   }
   return lookup_start_object;
 }

src/compiler/typed-optimization.cc

@@ -680,23 +680,29 @@ Reduction TypedOptimization::ReduceTypeOf(Node* node) {
   Type const type = NodeProperties::GetType(input);
   Factory* const f = factory();
   if (type.Is(Type::Boolean())) {
-    return Replace(jsgraph()->Constant(MakeRef(broker(), f->boolean_string())));
+    return Replace(
+        jsgraph()->Constant(ObjectRef(broker(), f->boolean_string())));
   } else if (type.Is(Type::Number())) {
-    return Replace(jsgraph()->Constant(MakeRef(broker(), f->number_string())));
+    return Replace(
+        jsgraph()->Constant(ObjectRef(broker(), f->number_string())));
   } else if (type.Is(Type::String())) {
-    return Replace(jsgraph()->Constant(MakeRef(broker(), f->string_string())));
+    return Replace(
+        jsgraph()->Constant(ObjectRef(broker(), f->string_string())));
   } else if (type.Is(Type::BigInt())) {
-    return Replace(jsgraph()->Constant(MakeRef(broker(), f->bigint_string())));
+    return Replace(
+        jsgraph()->Constant(ObjectRef(broker(), f->bigint_string())));
   } else if (type.Is(Type::Symbol())) {
-    return Replace(jsgraph()->Constant(MakeRef(broker(), f->symbol_string())));
+    return Replace(
+        jsgraph()->Constant(ObjectRef(broker(), f->symbol_string())));
   } else if (type.Is(Type::OtherUndetectableOrUndefined())) {
     return Replace(
-        jsgraph()->Constant(MakeRef(broker(), f->undefined_string())));
+        jsgraph()->Constant(ObjectRef(broker(), f->undefined_string())));
   } else if (type.Is(Type::NonCallableOrNull())) {
-    return Replace(jsgraph()->Constant(MakeRef(broker(), f->object_string())));
+    return Replace(
+        jsgraph()->Constant(ObjectRef(broker(), f->object_string())));
   } else if (type.Is(Type::Function())) {
     return Replace(
-        jsgraph()->Constant(MakeRef(broker(), f->function_string())));
+        jsgraph()->Constant(ObjectRef(broker(), f->function_string())));
   }
   return NoChange();
 }

src/compiler/types.cc

@@ -6,7 +6,6 @@
 
 #include <iomanip>
 
-#include "src/compiler/js-heap-broker.h"
 #include "src/handles/handles-inl.h"
 #include "src/objects/instance-type.h"
 #include "src/objects/objects-inl.h"
@@ -838,7 +837,7 @@ Type Type::Constant(double value, Zone* zone) {
 }
 
 Type Type::Constant(JSHeapBroker* broker, Handle<i::Object> value, Zone* zone) {
-  ObjectRef ref = MakeRef(broker, value);
+  ObjectRef ref(broker, value);
   if (ref.IsSmi()) {
     return Constant(static_cast<double>(ref.AsSmi()), zone);
   }

test/cctest/compiler/test-js-constant-cache.cc

@@ -191,8 +191,8 @@ TEST(HeapNumbers) {
     Handle<Object> num = T.factory()->NewNumber(value);
     Handle<HeapNumber> heap = T.factory()->NewHeapNumber(value);
     Node* node1 = T.Constant(value);
-    Node* node2 = T.Constant(MakeRef(T.broker(), num));
-    Node* node3 = T.Constant(MakeRef(T.broker(), heap));
+    Node* node2 = T.Constant(ObjectRef(T.broker(), num));
+    Node* node3 = T.Constant(ObjectRef(T.broker(), heap));
     CHECK_EQ(node1, node2);
     CHECK_EQ(node1, node3);
   }
@@ -202,20 +202,18 @@ TEST(HeapNumbers) {
 TEST(OddballHandle) {
   JSConstantCacheTester T;
 
-  CHECK_EQ(
-      T.UndefinedConstant(),
-      T.Constant(MakeRef<Object>(T.broker(), T.factory()->undefined_value())));
-  CHECK_EQ(
-      T.TheHoleConstant(),
-      T.Constant(MakeRef<Object>(T.broker(), T.factory()->the_hole_value())));
+  CHECK_EQ(T.UndefinedConstant(),
+           T.Constant(ObjectRef(T.broker(), T.factory()->undefined_value())));
+  CHECK_EQ(T.TheHoleConstant(),
+           T.Constant(ObjectRef(T.broker(), T.factory()->the_hole_value())));
   CHECK_EQ(T.TrueConstant(),
-           T.Constant(MakeRef<Object>(T.broker(), T.factory()->true_value())));
+           T.Constant(ObjectRef(T.broker(), T.factory()->true_value())));
   CHECK_EQ(T.FalseConstant(),
-           T.Constant(MakeRef<Object>(T.broker(), T.factory()->false_value())));
+           T.Constant(ObjectRef(T.broker(), T.factory()->false_value())));
   CHECK_EQ(T.NullConstant(),
-           T.Constant(MakeRef<Object>(T.broker(), T.factory()->null_value())));
+           T.Constant(ObjectRef(T.broker(), T.factory()->null_value())));
   CHECK_EQ(T.NaNConstant(),
-           T.Constant(MakeRef<Object>(T.broker(), T.factory()->nan_value())));
+           T.Constant(ObjectRef(T.broker(), T.factory()->nan_value())));
 }
 
 

test/cctest/compiler/test-js-context-specialization.cc

@@ -137,9 +137,9 @@ TEST(ReduceJSLoadContext0) {
   const int slot = 5;
   native->set(slot, *expected);
 
-  Node* const_context = t.jsgraph()->Constant(MakeRef(t.broker(), native));
+  Node* const_context = t.jsgraph()->Constant(ObjectRef(t.broker(), native));
   Node* deep_const_context =
-      t.jsgraph()->Constant(MakeRef(t.broker(), subcontext2));
+      t.jsgraph()->Constant(ObjectRef(t.broker(), subcontext2));
   Node* param_context = t.graph()->NewNode(t.common()->Parameter(0), start);
 
   {
@@ -284,7 +284,8 @@ TEST(ReduceJSLoadContext2) {
   context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
   context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
 
-  Node* context0 = t.jsgraph()->Constant(MakeRef(t.broker(), context_object1));
+  Node* context0 =
+      t.jsgraph()->Constant(ObjectRef(t.broker(), context_object1));
   Node* context1 =
       t.graph()->NewNode(create_function_context, context0, start, start);
   Node* context2 =
@@ -441,9 +442,9 @@ TEST(ReduceJSStoreContext0) {
   const int slot = 5;
   native->set(slot, *expected);
 
-  Node* const_context = t.jsgraph()->Constant(MakeRef(t.broker(), native));
+  Node* const_context = t.jsgraph()->Constant(ObjectRef(t.broker(), native));
   Node* deep_const_context =
-      t.jsgraph()->Constant(MakeRef(t.broker(), subcontext2));
+      t.jsgraph()->Constant(ObjectRef(t.broker(), subcontext2));
   Node* param_context = t.graph()->NewNode(t.common()->Parameter(0), start);
 
   {
@@ -553,7 +554,8 @@ TEST(ReduceJSStoreContext2) {
   context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
   context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
 
-  Node* context0 = t.jsgraph()->Constant(MakeRef(t.broker(), context_object1));
+  Node* context0 =
+      t.jsgraph()->Constant(ObjectRef(t.broker(), context_object1));
   Node* context1 =
       t.graph()->NewNode(create_function_context, context0, start, start);
   Node* context2 =