Reland "[compiler] Make JSDataViewRef and JSBoundFunctionRef bg-serialized"

This is a reland of 036e5783.
Key JSBoundFunctionRef methods have been changed to return an optional
type, replacing the bogus always-true serialized() method.

Original change's description:
> [compiler] Make JSDataViewRef and JSBoundFunctionRef bg-serialized
>
> ... but keep/make subclass-specific methods do direct reads.
>
> Bug: v8:7790
> Change-Id: Ia4b9d207ce75cf28f6f0f33027ab05e27db49ce9
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2959621
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#75457}

Bug: v8:11960, v8:7790
Change-Id: I1f29283b2fb6e5fe3644e2f4e33341fce2641775
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3013313
Auto-Submit: Georg Neis <neis@chromium.org>
Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75628}

src/compiler/heap-refs.cc

@@ -742,21 +742,36 @@ class ArrayBoilerplateDescriptionData : public HeapObjectData {
 class JSDataViewData : public JSObjectData {
  public:
   JSDataViewData(JSHeapBroker* broker, ObjectData** storage,
-                 Handle<JSDataView> object);
+                 Handle<JSDataView> object,
+                 ObjectDataKind kind = kSerializedHeapObject)
+      : JSObjectData(broker, storage, object, kind) {
+    if (kind == kSerializedHeapObject) {
+      DCHECK(!broker->is_concurrent_inlining());
+      byte_length_ = object->byte_length();
+    } else {
+      DCHECK_EQ(kind, kBackgroundSerializedHeapObject);
+      DCHECK(broker->is_concurrent_inlining());
+    }
+  }
 
-  size_t byte_length() const { return byte_length_; }
+  size_t byte_length() const {
+    DCHECK_EQ(kind(), kSerializedHeapObject);
+    return byte_length_;
+  }
 
  private:
-  size_t const byte_length_;
+  size_t byte_length_ = 0;  // Only valid if not concurrent inlining.
 };
 
 class JSBoundFunctionData : public JSObjectData {
  public:
   JSBoundFunctionData(JSHeapBroker* broker, ObjectData** storage,
-                      Handle<JSBoundFunction> object);
+                      Handle<JSBoundFunction> object,
+                      ObjectDataKind kind = kSerializedHeapObject)
+      : JSObjectData(broker, storage, object, kind) {}
 
+  // For main-thread serialization only.
   bool Serialize(JSHeapBroker* broker);
-  bool serialized() const { return serialized_; }
 
   ObjectData* bound_target_function() const {
     DCHECK(!broker()->is_concurrent_inlining());
@@ -1700,17 +1715,9 @@ class ScriptContextTableData : public FixedArrayData {
       : FixedArrayData(broker, storage, object, kind) {}
 };
 
-JSDataViewData::JSDataViewData(JSHeapBroker* broker, ObjectData** storage,
-                               Handle<JSDataView> object)
-    : JSObjectData(broker, storage, object),
-      byte_length_(object->byte_length()) {}
-
-JSBoundFunctionData::JSBoundFunctionData(JSHeapBroker* broker,
-                                         ObjectData** storage,
-                                         Handle<JSBoundFunction> object)
-    : JSObjectData(broker, storage, object) {}
-
 bool JSBoundFunctionData::Serialize(JSHeapBroker* broker) {
+  DCHECK(!broker->is_concurrent_inlining());
+
   if (serialized_) return true;
   if (broker->StackHasOverflowed()) return false;
 
@@ -3133,15 +3140,35 @@ uint64_t HeapNumberRef::value_as_bits() const {
   return ObjectRef::data()->AsHeapNumber()->value_as_bits();
 }
 
-// These JSBoundFunction fields are immutable after initialization. Moreover,
-// as long as JSObjects are still serialized on the main thread, all
-// JSBoundFunctionRefs are created at a time when the underlying objects are
-// guaranteed to be fully initialized.
-BIMODAL_ACCESSOR_WITH_FLAG(JSBoundFunction, JSReceiver, bound_target_function)
-BIMODAL_ACCESSOR_WITH_FLAG(JSBoundFunction, Object, bound_this)
-BIMODAL_ACCESSOR_WITH_FLAG(JSBoundFunction, FixedArray, bound_arguments)
+base::Optional<JSReceiverRef> JSBoundFunctionRef::bound_target_function()
+    const {
+  if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
+    // Immutable after initialization.
+    return TryMakeRef(broker(), object()->bound_target_function(),
+                      kAssumeMemoryFence);
+  }
+  return TryMakeRef<JSReceiver>(
+      broker(), data()->AsJSBoundFunction()->bound_target_function());
+}
+base::Optional<ObjectRef> JSBoundFunctionRef::bound_this() const {
+  if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
+    // Immutable after initialization.
+    return TryMakeRef(broker(), object()->bound_this(), kAssumeMemoryFence);
+  }
+  return TryMakeRef<Object>(broker(),
+                            data()->AsJSBoundFunction()->bound_this());
+}
+FixedArrayRef JSBoundFunctionRef::bound_arguments() const {
+  if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
+    // Immutable after initialization.
+    return MakeRefAssumeMemoryFence(broker(), object()->bound_arguments());
+  }
+  return FixedArrayRef(broker(),
+                       data()->AsJSBoundFunction()->bound_arguments());
+}
 
-BIMODAL_ACCESSOR_C(JSDataView, size_t, byte_length)
+// Immutable after initialization.
+BIMODAL_ACCESSOR_WITH_FLAG_C(JSDataView, size_t, byte_length)
 
 BIMODAL_ACCESSOR_C(JSFunction, bool, has_feedback_vector)
 BIMODAL_ACCESSOR_C(JSFunction, bool, has_initial_map)
@@ -4099,11 +4126,12 @@ void JSFunctionRef::SerializeCodeAndFeedback() {
   data()->AsJSFunction()->SerializeCodeAndFeedback(broker());
 }
 
-bool JSBoundFunctionRef::serialized() const {
-  if (data_->should_access_heap()) return true;
-  if (data_->AsJSBoundFunction()->serialized()) return true;
-  TRACE_BROKER_MISSING(broker(), "data for JSBoundFunction " << this);
-  return false;
+bool JSBoundFunctionRef::Serialize() {
+  if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
+    return true;
+  }
+  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
+  return data()->AsJSBoundFunction()->Serialize(broker());
 }
 
 bool JSFunctionRef::serialized() const {
@@ -4244,12 +4272,6 @@ bool JSTypedArrayRef::serialized() const {
   return false;
 }
 
-bool JSBoundFunctionRef::Serialize() {
-  if (data_->should_access_heap()) return true;
-  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-  return data()->AsJSBoundFunction()->Serialize(broker());
-}
-
 bool PropertyCellRef::Serialize() const {
   if (data_->should_access_heap()) return true;
   CHECK(broker()->mode() == JSHeapBroker::kSerializing ||

src/compiler/heap-refs.h

@@ -81,8 +81,8 @@ enum class RefSerializationKind {
 #define HEAP_BROKER_OBJECT_LIST(V)                                        \
   /* Subtypes of JSObject */                                              \
   V(JSArray, RefSerializationKind::kBackgroundSerialized)                 \
-  V(JSBoundFunction, RefSerializationKind::kSerialized)                   \
-  V(JSDataView, RefSerializationKind::kSerialized)                        \
+  V(JSBoundFunction, RefSerializationKind::kBackgroundSerialized)         \
+  V(JSDataView, RefSerializationKind::kBackgroundSerialized)              \
   V(JSFunction, RefSerializationKind::kSerialized)                        \
   V(JSGlobalObject, RefSerializationKind::kBackgroundSerialized)          \
   V(JSGlobalProxy, RefSerializationKind::kBackgroundSerialized)           \
@@ -386,11 +386,11 @@ class JSBoundFunctionRef : public JSObjectRef {
   Handle<JSBoundFunction> object() const;
 
   bool Serialize();
-  bool serialized() const;
 
-  // The following are available only after calling Serialize().
-  JSReceiverRef bound_target_function() const;
-  ObjectRef bound_this() const;
+  // TODO(neis): Make return types non-optional once JSFunction is no longer
+  // fg-serialized.
+  base::Optional<JSReceiverRef> bound_target_function() const;
+  base::Optional<ObjectRef> bound_this() const;
   FixedArrayRef bound_arguments() const;
 };
 

src/compiler/js-call-reducer.cc

@@ -4308,11 +4308,13 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
       return ReduceJSCall(node, function.shared());
     } else if (target_ref.IsJSBoundFunction()) {
       JSBoundFunctionRef function = target_ref.AsJSBoundFunction();
-      if (!function.serialized()) return NoChange();
-
-      ObjectRef bound_this = function.bound_this();
+      base::Optional<JSReceiverRef> bound_target_function =
+          function.bound_target_function();
+      if (!bound_target_function.has_value()) return NoChange();
+      base::Optional<ObjectRef> bound_this = function.bound_this();
+      if (!bound_this.has_value()) return NoChange();
       ConvertReceiverMode const convert_mode =
-          bound_this.IsNullOrUndefined()
+          bound_this->IsNullOrUndefined()
               ? ConvertReceiverMode::kNullOrUndefined
               : ConvertReceiverMode::kNotNullOrUndefined;
 
@@ -4333,9 +4335,9 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
 
       // Patch {node} to use [[BoundTargetFunction]] and [[BoundThis]].
       NodeProperties::ReplaceValueInput(
-          node, jsgraph()->Constant(function.bound_target_function()),
+          node, jsgraph()->Constant(*bound_target_function),
           JSCallNode::TargetIndex());
-      NodeProperties::ReplaceValueInput(node, jsgraph()->Constant(bound_this),
+      NodeProperties::ReplaceValueInput(node, jsgraph()->Constant(*bound_this),
                                         JSCallNode::ReceiverIndex());
 
       // Insert the [[BoundArguments]] for {node}.
@@ -5012,8 +5014,9 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
       }
     } else if (target_ref.IsJSBoundFunction()) {
       JSBoundFunctionRef function = target_ref.AsJSBoundFunction();
-      if (!function.serialized()) return NoChange();
-      ObjectRef bound_target_function = function.bound_target_function();
+      base::Optional<JSReceiverRef> bound_target_function =
+          function.bound_target_function();
+      if (!bound_target_function.has_value()) return NoChange();
       FixedArrayRef bound_arguments = function.bound_arguments();
       const int bound_arguments_length = bound_arguments.length();
 
@@ -5032,7 +5035,7 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
 
       // Patch {node} to use [[BoundTargetFunction]].
       node->ReplaceInput(n.TargetIndex(),
-                         jsgraph()->Constant(bound_target_function));
+                         jsgraph()->Constant(*bound_target_function));
 
       // Patch {node} to use [[BoundTargetFunction]]
       // as new.target if {new_target} equals {target}.
@@ -5041,7 +5044,7 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
           graph()->NewNode(common()->Select(MachineRepresentation::kTagged),
                            graph()->NewNode(simplified()->ReferenceEqual(),
                                             target, new_target),
-                           jsgraph()->Constant(bound_target_function),
+                           jsgraph()->Constant(*bound_target_function),
                            new_target));
 
       // Insert the [[BoundArguments]] for {node}.

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

@@ -629,15 +629,15 @@ Reduction JSNativeContextSpecialization::ReduceJSOrdinaryHasInstance(
     // OrdinaryHasInstance on bound functions turns into a recursive invocation
     // of the instanceof operator again.
     JSBoundFunctionRef function = m.Ref(broker()).AsJSBoundFunction();
-    if (!function.serialized()) return NoChange();
-
-    JSReceiverRef bound_target_function = function.bound_target_function();
+    base::Optional<JSReceiverRef> bound_target_function =
+        function.bound_target_function();
+    if (bound_target_function.has_value()) return NoChange();
 
     Node* feedback = jsgraph()->UndefinedConstant();
     NodeProperties::ReplaceValueInput(node, object,
                                       JSInstanceOfNode::LeftIndex());
     NodeProperties::ReplaceValueInput(
-        node, jsgraph()->Constant(bound_target_function),
+        node, jsgraph()->Constant(*bound_target_function),
         JSInstanceOfNode::RightIndex());
     node->InsertInput(zone(), JSInstanceOfNode::FeedbackVectorIndex(),
                       feedback);

src/compiler/serializer-for-background-compilation.cc

@@ -2046,7 +2046,7 @@ JSReceiverRef UnrollBoundFunction(JSBoundFunctionRef const& bound_function,
   JSReceiverRef target = bound_function.AsJSReceiver();
   HintsVector reversed_bound_arguments(zone);
   for (; target.IsJSBoundFunction();
-       target = target.AsJSBoundFunction().bound_target_function()) {
+       target = target.AsJSBoundFunction().bound_target_function().value()) {
     for (int i = target.AsJSBoundFunction().bound_arguments().length() - 1;
          i >= 0; --i) {
       Hints const arg = Hints::SingleConstant(
@@ -2054,7 +2054,7 @@ JSReceiverRef UnrollBoundFunction(JSBoundFunctionRef const& bound_function,
       reversed_bound_arguments.push_back(arg);
     }
     Hints const arg = Hints::SingleConstant(
-        target.AsJSBoundFunction().bound_this().object(), zone);
+        target.AsJSBoundFunction().bound_this().value().object(), zone);
     reversed_bound_arguments.push_back(arg);
   }
 
@@ -3405,7 +3405,7 @@ void SerializerForBackgroundCompilation::ProcessConstantForOrdinaryHasInstance(
   if (constructor.IsJSBoundFunction()) {
     constructor.AsJSBoundFunction().Serialize();
     ProcessConstantForInstanceOf(
-        constructor.AsJSBoundFunction().bound_target_function(),
+        constructor.AsJSBoundFunction().bound_target_function().value(),
         walk_prototypes);
   } else if (constructor.IsJSFunction()) {
     constructor.AsJSFunction().Serialize();