[compiler] Make HeapNumberRef background serialized

HeapNumberRef can be serialized in the background, so long as we record
a dependency to check at the end of compilation to ensure that the
number (interpreted as bits) has not changed.

Bug: v8:7790
Change-Id: I5c1c27466192580ff33bd55c8fa44dac957f2171
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2872827
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74423}

src/compiler/compilation-dependencies.cc

@@ -227,6 +227,22 @@ class ConstantInDictionaryPrototypeChainDependency final
   PropertyKind kind_;
 };
 
+class HeapNumberValueDependency final : public CompilationDependency {
+ public:
+  HeapNumberValueDependency(Handle<HeapNumber> number, uint64_t value)
+      : number_(number), value_(value) {}
+
+  bool IsValid() const override { return number_->value_as_bits() == value_; }
+
+  void Install(const MaybeObjectHandle& code) const override {
+    SLOW_DCHECK(IsValid());
+  }
+
+ private:
+  Handle<HeapNumber> number_;
+  const uint64_t value_;
+};
+
 class TransitionDependency final : public CompilationDependency {
  public:
   explicit TransitionDependency(const MapRef& map) : map_(map) {
@@ -748,6 +764,11 @@ CompilationDependencies::DependOnInitialMapInstanceSizePrediction(
   return SlackTrackingPrediction(initial_map, instance_size);
 }
 
+void CompilationDependencies::DependOnHeapNumberValue(Handle<HeapNumber> number,
+                                                      uint64_t value) {
+  RecordDependency(zone_->New<HeapNumberValueDependency>(number, value));
+}
+
 CompilationDependency const*
 CompilationDependencies::TransitionDependencyOffTheRecord(
     const MapRef& target_map) const {

src/compiler/compilation-dependencies.h

@@ -97,6 +97,10 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // Record the assumption that {site}'s {ElementsKind} doesn't change.
   void DependOnElementsKind(const AllocationSiteRef& site);
 
+  // HeapNumber::value() when read from the background thread may be
+  // invalid, and must be checked at the end of compilation.
+  void DependOnHeapNumberValue(Handle<HeapNumber> number, uint64_t value);
+
   // For each given map, depend on the stability of (the maps of) all prototypes
   // up to (and including) the {last_prototype}.
   template <class MapContainer>

src/compiler/heap-refs.cc

@@ -11,6 +11,7 @@
 #include "src/api/api-inl.h"
 #include "src/ast/modules.h"
 #include "src/codegen/code-factory.h"
+#include "src/compiler/compilation-dependencies.h"
 #include "src/compiler/graph-reducer.h"
 #include "src/compiler/js-heap-broker.h"
 #include "src/execution/protectors-inl.h"
@@ -72,8 +73,10 @@ namespace {
 
 bool IsReadOnlyHeapObject(Object object) {
   DisallowGarbageCollection no_gc;
+  // HeapNumber is excluded because we have a uniform background
+  // serialization treatment for it.
   return (object.IsCode() && Code::cast(object).is_builtin()) ||
-         (object.IsHeapObject() &&
+         (object.IsHeapObject() && !object.IsHeapNumber() &&
           ReadOnlyHeap::Contains(HeapObject::cast(object)));
 }
 
@@ -754,18 +757,29 @@ class RegExpBoilerplateDescriptionData : public HeapObjectData {
   int flags_;
 };
 
+// for HeapNumber, we should always read from the Data object, which records
+// the one time we read the value, possibly on main thread (in the case of
+// bytecode serialization), or possibly on the background thread. The read of
+// this value has no guarantee of being correct. Therefore, instantiation of
+// a HeapNumberData must be associated with the creation of a compilation
+// dependency which will check later on the main thread if the bits of the
+// heap number are exactly correct.
 class HeapNumberData : public HeapObjectData {
  public:
   HeapNumberData(JSHeapBroker* broker, ObjectData** storage,
                  Handle<HeapNumber> object,
                  ObjectDataKind kind = ObjectDataKind::kSerializedHeapObject)
-      : HeapObjectData(broker, storage, object, kind),
-        value_(object->value()) {}
+      : HeapObjectData(broker, storage, object, kind) {
+    const uint64_t value_as_bits = object->value_as_bits_relaxed();
+    STATIC_ASSERT(sizeof(value_) == sizeof(value_as_bits));
+    value_ = *reinterpret_cast<const double*>(&value_as_bits);
+    broker->dependencies()->DependOnHeapNumberValue(object, value_as_bits);
+  }
 
   double value() const { return value_; }
 
  private:
-  double const value_;
+  double value_;
 };
 
 class ContextData : public HeapObjectData {
@@ -3294,7 +3308,15 @@ BIMODAL_ACCESSOR_C(FeedbackVector, double, invocation_count)
 
 BIMODAL_ACCESSOR(HeapObject, Map, map)
 
-BIMODAL_ACCESSOR_C(HeapNumber, double, value)
+double HeapNumberRef::value() const {
+  if (data_->should_access_heap()) {
+    // TODO(mvstanton): it would make things simpler to eliminate the
+    // kUnserializedHeapObject case, even for OSR compiles.
+    CHECK_EQ(data_->kind(), kUnserializedHeapObject);
+    return object()->value();
+  }
+  return ObjectRef::data()->AsHeapNumber()->value();
+}
 
 // These JSBoundFunction fields are immutable after initialization. Moreover,
 // as long as JSObjects are still serialized on the main thread, all

src/compiler/heap-refs.h

@@ -123,7 +123,7 @@ enum class RefSerializationKind {
   V(FeedbackVector, RefSerializationKind::kNeverSerialized)               \
   V(FixedArrayBase, RefSerializationKind::kBackgroundSerialized)          \
   V(FunctionTemplateInfo, RefSerializationKind::kNeverSerialized)         \
-  V(HeapNumber, RefSerializationKind::kPossiblyBackgroundSerialized)      \
+  V(HeapNumber, RefSerializationKind::kBackgroundSerialized)              \
   V(JSReceiver, RefSerializationKind::kBackgroundSerialized)              \
   V(Map, RefSerializationKind::kBackgroundSerialized)                     \
   V(Name, RefSerializationKind::kNeverSerialized)                         \

src/compiler/js-heap-broker.h

@@ -348,6 +348,16 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
 
   RootIndexMap const& root_index_map() { return root_index_map_; }
 
+  CompilationDependencies* dependencies() const {
+    DCHECK_NOT_NULL(dependencies_);
+    return dependencies_;
+  }
+
+  void set_dependencies(CompilationDependencies* dependencies) {
+    DCHECK_NULL(dependencies_);
+    dependencies_ = dependencies;
+  }
+
   class MapUpdaterMutexDepthScope final {
    public:
     explicit MapUpdaterMutexDepthScope(JSHeapBroker* ptr)
@@ -441,6 +451,7 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
 
   Isolate* const isolate_;
   Zone* const zone_ = nullptr;
+  CompilationDependencies* dependencies_ = nullptr;
   base::Optional<NativeContextRef> target_native_context_;
   RefsMap* refs_;
   RootIndexMap root_index_map_;

src/compiler/pipeline.cc

@@ -183,6 +183,7 @@ class PipelineData {
             : nullptr;
     dependencies_ =
         info_->zone()->New<CompilationDependencies>(broker_, info_->zone());
+    broker_->set_dependencies(dependencies_);
   }
 
 #if V8_ENABLE_WEBASSEMBLY

src/objects/heap-number-inl.h

@@ -25,6 +25,13 @@ uint64_t HeapNumber::value_as_bits() const {
   return base::ReadUnalignedValue<uint64_t>(field_address(kValueOffset));
 }
 
+uint64_t HeapNumber::value_as_bits_relaxed() const {
+  return static_cast<uint64_t>(RELAXED_READ_UINT32_FIELD(*this, kValueOffset)) |
+         (static_cast<uint64_t>(
+              RELAXED_READ_UINT32_FIELD(*this, kValueOffset + kUInt32Size))
+          << 32);
+}
+
 void HeapNumber::set_value_as_bits(uint64_t bits) {
   base::WriteUnalignedValue<uint64_t>(field_address(kValueOffset), bits);
 }

src/objects/heap-number.h

@@ -20,6 +20,7 @@ namespace internal {
 class HeapNumber
     : public TorqueGeneratedHeapNumber<HeapNumber, PrimitiveHeapObject> {
  public:
+  inline uint64_t value_as_bits_relaxed() const;
   inline uint64_t value_as_bits() const;
   inline void set_value_as_bits(uint64_t bits);