[compiler] Make FixedDoubleArrayRefs never-serialized

FixedDoubleArrays are a special case:

1 The reads are 64-bit and unaligned, thus use memcpy underneath.
2 The compiler only reads FDArray values for (constant) boilerplate
  elements.

1) makes proper atomic reads tricky-to-impossible without a lock.
Luckily, 2) means we know that the array values are immutable after
initialization, thus we can simply do a non-atomic read from the
compiler thread.

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

src/compiler/compilation-dependencies.cc

@@ -444,7 +444,7 @@ class ElementsKindDependency final : public CompilationDependency {
   bool IsValid() const override {
     Handle<AllocationSite> site = site_.object();
     ElementsKind kind = site->PointsToLiteral()
-                            ? site->boilerplate().GetElementsKind()
+                            ? site->boilerplate(kAcquireLoad).GetElementsKind()
                             : site->GetElementsKind();
     return kind_ == kind;
   }

src/compiler/heap-refs.cc

@@ -157,13 +157,11 @@ constexpr RefSerializationKind RefSerializationKindOf() {
   return RefSerializationKind::kSerialized;
 }
 
-#define DEFINE_REF_SERIALIZATION_KIND(Name, Kind)                        \
-  template <>                                                            \
-  constexpr RefSerializationKind RefSerializationKindOf<Name>() {        \
-    return Kind;                                                         \
-  }                                                                      \
-  /* The static assert is needed to avoid 'unused function' warnings. */ \
-  STATIC_ASSERT(RefSerializationKindOf<Name>() == Kind);
+#define DEFINE_REF_SERIALIZATION_KIND(Name, Kind)                 \
+  template <>                                                     \
+  constexpr RefSerializationKind RefSerializationKindOf<Name>() { \
+    return Kind;                                                  \
+  }
 HEAP_BROKER_OBJECT_LIST(DEFINE_REF_SERIALIZATION_KIND)
 #undef DEFINE_REF_SERIALIZATION_KIND
 
@@ -172,6 +170,21 @@ constexpr bool IsSerializedRef() {
   return RefSerializationKindOf<T>() == RefSerializationKind::kSerialized;
 }
 
+RefSerializationKind RefSerializationKindOf(ObjectData* const data) {
+  Object o = *data->object();
+  if (o.IsSmi()) {
+    return RefSerializationKind::kNeverSerialized;
+#define DEFINE_REF_SERIALIZATION_KIND(Name, Kind) \
+  }                                               \
+  /* NOLINTNEXTLINE(readability/braces) */        \
+  else if (o.Is##Name()) {                        \
+    return RefSerializationKindOf<Name>();
+    HEAP_BROKER_OBJECT_LIST(DEFINE_REF_SERIALIZATION_KIND)
+#undef DEFINE_REF_SERIALIZATION_KIND
+  }
+  UNREACHABLE();
+}
+
 }  // namespace
 
 class HeapObjectData : public ObjectData {
@@ -1259,7 +1272,7 @@ AllocationSiteData::AllocationSiteData(JSHeapBroker* broker,
       GetAllocationType_(object->GetAllocationType()) {
   if (PointsToLiteral_) {
     IsFastLiteral_ = IsInlinableFastLiteral(
-        handle(object->boilerplate(), broker->isolate()));
+        handle(object->boilerplate(kAcquireLoad), broker->isolate()));
   } else {
     GetElementsKind_ = object->GetElementsKind();
     CanInlineCall_ = object->CanInlineCall();
@@ -1275,7 +1288,7 @@ void AllocationSiteData::SerializeBoilerplate(JSHeapBroker* broker) {
 
   CHECK(IsFastLiteral_);
   DCHECK_NULL(boilerplate_);
-  boilerplate_ = broker->GetOrCreateData(site->boilerplate());
+  boilerplate_ = broker->GetOrCreateData(site->boilerplate(kAcquireLoad));
   if (!boilerplate_->should_access_heap()) {
     boilerplate_->AsJSObject()->SerializeAsBoilerplate(broker);
   }
@@ -1792,41 +1805,14 @@ void FixedArrayData::SerializeContents(JSHeapBroker* broker) {
 
 class FixedDoubleArrayData : public FixedArrayBaseData {
  public:
-  FixedDoubleArrayData(JSHeapBroker* broker, ObjectData** storage,
-                       Handle<FixedDoubleArray> object);
-
-  // Serializes all elements of the fixed array.
-  void SerializeContents(JSHeapBroker* broker);
-
-  Float64 Get(int i) const;
-
- private:
-  bool serialized_contents_ = false;
-  ZoneVector<Float64> contents_;
-};
-
-FixedDoubleArrayData::FixedDoubleArrayData(JSHeapBroker* broker,
-                                           ObjectData** storage,
-                                           Handle<FixedDoubleArray> object)
-    : FixedArrayBaseData(broker, storage, object,
-                         ObjectDataKind::kSerializedHeapObject),
-      contents_(broker->zone()) {}
-
-void FixedDoubleArrayData::SerializeContents(JSHeapBroker* broker) {
-  if (serialized_contents_) return;
-  serialized_contents_ = true;
-
-  TraceScope tracer(broker, this, "FixedDoubleArrayData::SerializeContents");
-  Handle<FixedDoubleArray> self = Handle<FixedDoubleArray>::cast(object());
-  CHECK_EQ(self->length(), length());
-  CHECK(contents_.empty());
-  contents_.reserve(static_cast<size_t>(length()));
-
-  for (int i = 0; i < length(); i++) {
-    contents_.push_back(Float64::FromBits(self->get_representation(i)));
+  FixedDoubleArrayData(
+      JSHeapBroker* broker, ObjectData** storage,
+      Handle<FixedDoubleArray> object,
+      ObjectDataKind kind = ObjectDataKind::kNeverSerializedHeapObject)
+      : FixedArrayBaseData(broker, storage, object, kind) {
+    DCHECK(!broker->is_concurrent_inlining());
   }
-  TRACE(broker, "Copied " << contents_.size() << " elements");
-}
+};
 
 class BytecodeArrayData : public FixedArrayBaseData {
  public:
@@ -2458,8 +2444,6 @@ void JSObjectData::SerializeRecursiveAsBoilerplate(JSHeapBroker* broker,
   } else {
     CHECK(boilerplate->HasDoubleElements());
     CHECK_LE(elements_object->Size(), kMaxRegularHeapObjectSize);
-    DCHECK_EQ(elements_->kind(), ObjectDataKind::kSerializedHeapObject);
-    elements_->AsFixedDoubleArray()->SerializeContents(broker);
   }
 
   // TODO(turbofan): Do we want to support out-of-object properties?
@@ -2846,10 +2830,9 @@ void JSHeapBroker::ClearReconstructibleData() {
     Address key = p->key;
     ObjectData* value = p->value;
     p = refs_->Next(p);
-    const ObjectDataKind kind = value->kind();
-    if (kind == ObjectDataKind::kNeverSerializedHeapObject ||
-        kind == ObjectDataKind::kBackgroundSerializedHeapObject ||
-        kind == ObjectDataKind::kUnserializedReadOnlyHeapObject) {
+    const auto kind = RefSerializationKindOf(value);
+    if (kind == RefSerializationKind::kNeverSerialized ||
+        kind == RefSerializationKind::kBackgroundSerialized) {
       if (value->IsMap() &&
           value->kind() == ObjectDataKind::kBackgroundSerializedHeapObject &&
           value->AsMap()->has_extra_serialized_data()) {
@@ -3101,7 +3084,7 @@ bool AllocationSiteRef::IsFastLiteral() const {
   if (data_->should_access_heap()) {
     CHECK_NE(data_->kind(), ObjectDataKind::kNeverSerializedHeapObject);
     return IsInlinableFastLiteral(
-        handle(object()->boilerplate(), broker()->isolate()));
+        handle(object()->boilerplate(kAcquireLoad), broker()->isolate()));
   }
   return data()->AsAllocationSite()->IsFastLiteral();
 }
@@ -3277,12 +3260,10 @@ ObjectRef FixedArrayRef::get(int i) const {
   return ObjectRef(broker(), data()->AsFixedArray()->Get(i));
 }
 
-Float64 FixedDoubleArrayRef::get(int i) const {
-  if (data_->should_access_heap()) {
-    return Float64::FromBits(object()->get_representation(i));
-  } else {
-    return data()->AsFixedDoubleArray()->Get(i);
-  }
+Float64 FixedDoubleArrayRef::GetFromImmutableFixedDoubleArray(int i) const {
+  STATIC_ASSERT(RefSerializationKindOf<FixedDoubleArray>() ==
+                RefSerializationKind::kNeverSerialized);
+  return Float64::FromBits(object()->get_representation(i));
 }
 
 Handle<ByteArray> BytecodeArrayRef::SourcePositionTable() const {
@@ -4155,8 +4136,8 @@ HeapObjectType HeapObjectRef::GetHeapObjectType() const {
 }
 base::Optional<JSObjectRef> AllocationSiteRef::boilerplate() const {
   if (data_->should_access_heap()) {
-    return JSObjectRef(
-        broker(), broker()->CanonicalPersistentHandle(object()->boilerplate()));
+    return JSObjectRef(broker(), broker()->CanonicalPersistentHandle(
+                                     object()->boilerplate(kAcquireLoad)));
   }
   ObjectData* boilerplate = data()->AsAllocationSite()->boilerplate();
   if (boilerplate) {
@@ -4194,11 +4175,6 @@ ObjectData* FixedArrayData::Get(int i) const {
   return contents_[i];
 }
 
-Float64 FixedDoubleArrayData::Get(int i) const {
-  CHECK_LT(i, static_cast<int>(contents_.size()));
-  return contents_[i];
-}
-
 PropertyDetails DescriptorArrayRef::GetPropertyDetails(
     InternalIndex descriptor_index) const {
   if (data_->should_access_heap()) {

src/compiler/heap-refs.h

@@ -100,7 +100,7 @@ enum class RefSerializationKind {
   /* Subtypes of FixedArrayBase */                                        \
   V(BytecodeArray, RefSerializationKind::kNeverSerialized)                \
   V(FixedArray, RefSerializationKind::kSerialized)                        \
-  V(FixedDoubleArray, RefSerializationKind::kSerialized)                  \
+  V(FixedDoubleArray, RefSerializationKind::kNeverSerialized)             \
   /* Subtypes of Name */                                                  \
   V(String, RefSerializationKind::kNeverSerialized)                       \
   V(Symbol, RefSerializationKind::kNeverSerialized)                       \
@@ -786,7 +786,10 @@ class FixedDoubleArrayRef : public FixedArrayBaseRef {
 
   Handle<FixedDoubleArray> object() const;
 
-  Float64 get(int i) const;
+  // Due to 64-bit unaligned reads, only usable for
+  // immutable-after-initialization FixedDoubleArrays protected by
+  // acquire-release semantics (such as boilerplate elements).
+  Float64 GetFromImmutableFixedDoubleArray(int i) const;
 };
 
 class BytecodeArrayRef : public FixedArrayBaseRef {

src/compiler/js-create-lowering.cc

@@ -1763,7 +1763,7 @@ Node* JSCreateLowering::AllocateFastLiteralElements(Node* effect, Node* control,
   if (elements_map.instance_type() == FIXED_DOUBLE_ARRAY_TYPE) {
     FixedDoubleArrayRef elements = boilerplate_elements.AsFixedDoubleArray();
     for (int i = 0; i < elements_length; ++i) {
-      Float64 value = elements.get(i);
+      Float64 value = elements.GetFromImmutableFixedDoubleArray(i);
       if (value.is_hole_nan()) {
         elements_values[i] = jsgraph()->TheHoleConstant();
       } else {

src/compiler/pipeline.cc

@@ -1553,15 +1553,13 @@ struct SerializationPhase {
       ContextRef(data->broker(),
                  data->specialization_context().FromJust().context);
     }
-    if (data->info()->concurrent_inlining()) {
-      if (FLAG_turbo_concurrent_get_property_access_info) {
-        data->broker()->ClearCachedPropertyAccessInfos();
-        data->dependencies()->ClearForConcurrentGetPropertyAccessInfo();
-      }
-      if (FLAG_stress_concurrent_inlining) {
-        // Force re-serialization from the background thread.
-        data->broker()->ClearReconstructibleData();
-      }
+    if (FLAG_turbo_concurrent_get_property_access_info) {
+      data->broker()->ClearCachedPropertyAccessInfos();
+      data->dependencies()->ClearForConcurrentGetPropertyAccessInfo();
+    }
+    if (FLAG_stress_concurrent_inlining) {
+      // Force re-serialization from the background thread.
+      data->broker()->ClearReconstructibleData();
     }
   }
 };

src/objects/allocation-site-inl.h

@@ -27,6 +27,8 @@ CAST_ACCESSOR(AllocationSite)
 
 ACCESSORS(AllocationSite, transition_info_or_boilerplate, Object,
           kTransitionInfoOrBoilerplateOffset)
+RELEASE_ACQUIRE_ACCESSORS(AllocationSite, transition_info_or_boilerplate,
+                          Object, kTransitionInfoOrBoilerplateOffset)
 ACCESSORS(AllocationSite, nested_site, Object, kNestedSiteOffset)
 INT32_ACCESSORS(AllocationSite, pretenure_data, kPretenureDataOffset)
 INT32_ACCESSORS(AllocationSite, pretenure_create_count,
@@ -41,8 +43,14 @@ JSObject AllocationSite::boilerplate() const {
   return JSObject::cast(transition_info_or_boilerplate());
 }
 
-void AllocationSite::set_boilerplate(JSObject object, WriteBarrierMode mode) {
-  set_transition_info_or_boilerplate(object, mode);
+JSObject AllocationSite::boilerplate(AcquireLoadTag tag) const {
+  DCHECK(PointsToLiteral());
+  return JSObject::cast(transition_info_or_boilerplate(tag));
+}
+
+void AllocationSite::set_boilerplate(JSObject value, ReleaseStoreTag tag,
+                                     WriteBarrierMode mode) {
+  set_transition_info_or_boilerplate(value, tag, mode);
 }
 
 int AllocationSite::transition_info() const {

src/objects/allocation-site.h

@@ -40,7 +40,9 @@ class AllocationSite : public Struct {
   // Contains either a Smi-encoded bitfield or a boilerplate. If it's a Smi the
   // AllocationSite is for a constructed Array.
   DECL_ACCESSORS(transition_info_or_boilerplate, Object)
-  DECL_ACCESSORS(boilerplate, JSObject)
+  DECL_RELEASE_ACQUIRE_ACCESSORS(transition_info_or_boilerplate, Object)
+  DECL_GETTER(boilerplate, JSObject)
+  DECL_RELEASE_ACQUIRE_ACCESSORS(boilerplate, JSObject)
   DECL_INT_ACCESSORS(transition_info)
 
   // nested_site threads a list of sites that represent nested literals

src/runtime/runtime-literals.cc

@@ -291,7 +291,7 @@ class AllocationSiteCreationContext : public AllocationSiteContext {
   }
   void ExitScope(Handle<AllocationSite> scope_site, Handle<JSObject> object) {
     if (object.is_null()) return;
-    scope_site->set_boilerplate(*object);
+    scope_site->set_boilerplate(*object, kReleaseStore);
     if (FLAG_trace_creation_allocation_sites) {
       bool top_level =
           !scope_site.is_null() && top().is_identical_to(scope_site);