[compiler] Mark HeapNumberRef as never serialized

This CL simplifies the approach to HeapNumbers in concurrent
compilation. We'll only create a HeapNumberRef for immutable
HeapNumbers -- this means that we don't need to validate the read
of the value with a compilation dependency check. Mutable
HeapNumbers are handled differently (the value is read for
constant folding, and protected with a constant field dependency).

This CL includes 2 reverts:
Revert "[compiler] Make HeapNumberRef background serialized"
Revert "[compiler] Fix endianness issue when reading HeapNumber"

Bug: v8:7790
Change-Id: I24e65583b787c214b917e96e789d711c2a7c9694
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2891576
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74567}

src/compiler/compilation-dependencies.cc

@@ -227,22 +227,6 @@ 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) {
@@ -764,11 +748,6 @@ 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,10 +97,6 @@ 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,7 +11,6 @@
 #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"
@@ -73,10 +72,8 @@ 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.IsHeapNumber() &&
+         (object.IsHeapObject() &&
           ReadOnlyHeap::Contains(HeapObject::cast(object)));
 }
 
@@ -757,29 +754,18 @@ 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) {
-    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);
-  }
+      : HeapObjectData(broker, storage, object, kind),
+        value_(object->value()) {}
 
   double value() const { return value_; }
 
  private:
-  double value_;
+  double const value_;
 };
 
 class ContextData : public HeapObjectData {
@@ -3299,15 +3285,7 @@ BIMODAL_ACCESSOR_C(FeedbackVector, double, invocation_count)
 
 BIMODAL_ACCESSOR(HeapObject, Map, map)
 
-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();
-}
+BIMODAL_ACCESSOR_C(HeapNumber, double, 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::kBackgroundSerialized)              \
+  V(HeapNumber, RefSerializationKind::kNeverSerialized)                   \
   V(JSReceiver, RefSerializationKind::kBackgroundSerialized)              \
   V(Map, RefSerializationKind::kBackgroundSerialized)                     \
   V(Name, RefSerializationKind::kNeverSerialized)                         \
@@ -429,6 +429,11 @@ class RegExpBoilerplateDescriptionRef : public HeapObjectRef {
   int flags() const;
 };
 
+// HeapNumberRef is only created for immutable HeapNumbers. Mutable
+// HeapNumbers (those owned by in-object or backing store fields with
+// representation type Double are not exposed to the compiler through
+// HeapNumberRef. Instead, we read their value, and protect that read
+// with a field-constness Dependency.
 class HeapNumberRef : public HeapObjectRef {
  public:
   DEFINE_REF_CONSTRUCTOR(HeapNumber, HeapObjectRef)

src/compiler/js-heap-broker.h

@@ -348,16 +348,6 @@ 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)
@@ -451,7 +441,6 @@ 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,7 +183,6 @@ 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,20 +25,6 @@ uint64_t HeapNumber::value_as_bits() const {
   return base::ReadUnalignedValue<uint64_t>(field_address(kValueOffset));
 }
 
-uint64_t HeapNumber::value_as_bits_relaxed() const {
-#if defined(V8_TARGET_BIG_ENDIAN)
-  return (static_cast<uint64_t>(RELAXED_READ_UINT32_FIELD(*this, kValueOffset))
-          << 32) |
-         static_cast<uint64_t>(
-             RELAXED_READ_UINT32_FIELD(*this, kValueOffset + kUInt32Size));
-#else
-  return static_cast<uint64_t>(RELAXED_READ_UINT32_FIELD(*this, kValueOffset)) |
-         (static_cast<uint64_t>(
-              RELAXED_READ_UINT32_FIELD(*this, kValueOffset + kUInt32Size))
-          << 32);
-#endif
-}
-
 void HeapNumber::set_value_as_bits(uint64_t bits) {
   base::WriteUnalignedValue<uint64_t>(field_address(kValueOffset), bits);
 }

src/objects/heap-number.h

@@ -20,7 +20,6 @@ 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);