[compiler] Refactor HeapObjectRef::BooleanValue paths

.. which used to be implemented by calling BooleanValue eagerly on all
seen heap objects during serialization. 1) it's wasteful to call this
on every object, 2) this was blocking conversion of HeapObjectRefs to
not require main-thread serialization.

This CL replaces the old pattern by a thread-safe TryGetBooleanValue
method, which may fail in some cases (e.g. when trying to read into a
HeapNumber).

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

src/compiler/common-operator-reducer.cc

@@ -28,7 +28,9 @@ Decision DecideCondition(JSHeapBroker* broker, Node* const cond) {
     }
     case IrOpcode::kHeapConstant: {
       HeapObjectMatcher m(unwrapped);
-      return m.Ref(broker).BooleanValue() ? Decision::kTrue : Decision::kFalse;
+      base::Optional<bool> maybe_result = m.Ref(broker).TryGetBooleanValue();
+      if (!maybe_result.has_value()) return Decision::kUnknown;
+      return *maybe_result ? Decision::kTrue : Decision::kFalse;
     }
     default:
       return Decision::kUnknown;

src/compiler/heap-refs.cc

@@ -167,15 +167,13 @@ class HeapObjectData : public ObjectData {
                  Handle<HeapObject> object,
                  ObjectDataKind kind = ObjectDataKind::kSerializedHeapObject);
 
-  bool boolean_value() const { return boolean_value_; }
+  base::Optional<bool> TryGetBooleanValue(JSHeapBroker* broker) const;
   ObjectData* map() const { return map_; }
   InstanceType GetMapInstanceType() const;
 
-  static HeapObjectData* Serialize(JSHeapBroker* broker,
-                                   Handle<HeapObject> object);
-
  private:
-  bool const boolean_value_;
+  base::Optional<bool> TryGetBooleanValueImpl(JSHeapBroker* broker) const;
+
   ObjectData* const map_;
 };
 
@@ -1271,12 +1269,6 @@ void AllocationSiteData::SerializeBoilerplate(JSHeapBroker* broker) {
 HeapObjectData::HeapObjectData(JSHeapBroker* broker, ObjectData** storage,
                                Handle<HeapObject> object, ObjectDataKind kind)
     : ObjectData(broker, storage, object, kind),
-      boolean_value_(object->BooleanValue(broker->isolate())),
-      // We have to use a raw cast below instead of AsMap() because of
-      // recursion. AsMap() would call IsMap(), which accesses the
-      // instance_type_ member. In the case of constructing the MapData for the
-      // meta map (whose map is itself), this member has not yet been
-      // initialized.
       map_(broker->GetOrCreateData(object->synchronized_map())) {
   CHECK_IMPLIES(kind == kSerializedHeapObject,
                 broker->mode() == JSHeapBroker::kSerializing);
@@ -1284,6 +1276,40 @@ HeapObjectData::HeapObjectData(JSHeapBroker* broker, ObjectData** storage,
                 kind == kBackgroundSerializedHeapObject);
 }
 
+base::Optional<bool> HeapObjectData::TryGetBooleanValue(
+    JSHeapBroker* broker) const {
+  // Keep in sync with Object::BooleanValue.
+  auto result = TryGetBooleanValueImpl(broker);
+  DCHECK_IMPLIES(broker->IsMainThread() && result.has_value(),
+                 result.value() == object()->BooleanValue(broker->isolate()));
+  return result;
+}
+
+base::Optional<bool> HeapObjectData::TryGetBooleanValueImpl(
+    JSHeapBroker* broker) const {
+  DisallowGarbageCollection no_gc;
+  Object o = *object();
+  Isolate* isolate = broker->isolate();
+  const InstanceType t = GetMapInstanceType();
+  if (o.IsTrue(isolate)) {
+    return true;
+  } else if (o.IsFalse(isolate)) {
+    return false;
+  } else if (o.IsNullOrUndefined(isolate)) {
+    return false;
+  } else if (MapRef{broker, map()}.is_undetectable()) {
+    return false;  // Undetectable object is false.
+  } else if (InstanceTypeChecker::IsString(t)) {
+    // TODO(jgruber): Implement in possible cases.
+    return {};
+  } else if (InstanceTypeChecker::IsHeapNumber(t)) {
+    return {};
+  } else if (InstanceTypeChecker::IsBigInt(t)) {
+    return {};
+  }
+  return true;
+}
+
 InstanceType HeapObjectData::GetMapInstanceType() const {
   ObjectData* map_data = map();
   if (map_data->should_access_heap()) {
@@ -3778,11 +3804,12 @@ bool ObjectRef::IsTheHole() const {
          AsHeapObject().map().oddball_type() == OddballType::kHole;
 }
 
-bool ObjectRef::BooleanValue() const {
+base::Optional<bool> ObjectRef::TryGetBooleanValue() const {
   if (data_->should_access_heap()) {
     return object()->BooleanValue(broker()->isolate());
   }
-  return IsSmi() ? (AsSmi() != 0) : data()->AsHeapObject()->boolean_value();
+  if (IsSmi()) return AsSmi() != 0;
+  return data()->AsHeapObject()->TryGetBooleanValue(broker());
 }
 
 Maybe<double> ObjectRef::OddballToNumber() const {

src/compiler/heap-refs.h

@@ -198,7 +198,7 @@ class V8_EXPORT_PRIVATE ObjectRef {
   bool IsNullOrUndefined() const;
   bool IsTheHole() const;
 
-  bool BooleanValue() const;
+  base::Optional<bool> TryGetBooleanValue() const;
   Maybe<double> OddballToNumber() const;
 
   Isolate* isolate() const;

src/compiler/simplified-operator-reducer.cc

@@ -60,7 +60,9 @@ Reduction SimplifiedOperatorReducer::Reduce(Node* node) {
     case IrOpcode::kChangeTaggedToBit: {
       HeapObjectMatcher m(node->InputAt(0));
       if (m.HasResolvedValue()) {
-        return ReplaceInt32(m.Ref(broker()).BooleanValue());
+        base::Optional<bool> maybe_result =
+            m.Ref(broker()).TryGetBooleanValue();
+        if (maybe_result.has_value()) return ReplaceInt32(*maybe_result);
       }
       if (m.IsChangeBitToTagged()) return Replace(m.InputAt(0));
       break;

src/objects/map.cc

@@ -1141,7 +1141,7 @@ Handle<Map> Map::AsElementsKind(Isolate* isolate, Handle<Map> map,
 
 int Map::NumberOfEnumerableProperties() const {
   int result = 0;
-  DescriptorArray descs = instance_descriptors(kAcquireLoad);
+  DescriptorArray descs = instance_descriptors(kRelaxedLoad);
   for (InternalIndex i : IterateOwnDescriptors()) {
     if ((descs.GetDetails(i).attributes() & ONLY_ENUMERABLE) == 0 &&
         !descs.GetKey(i).FilterKey(ENUMERABLE_STRINGS)) {
@@ -1153,7 +1153,7 @@ int Map::NumberOfEnumerableProperties() const {
 
 int Map::NextFreePropertyIndex() const {
   int number_of_own_descriptors = NumberOfOwnDescriptors();
-  DescriptorArray descs = instance_descriptors(kAcquireLoad);
+  DescriptorArray descs = instance_descriptors(kRelaxedLoad);
   // Search properties backwards to find the last field.
   for (int i = number_of_own_descriptors - 1; i >= 0; --i) {
     PropertyDetails details = descs.GetDetails(InternalIndex(i));