[TurboFan] Never serialize FeedbackCells

The compiler is only interested in the contents if it contains a
FeedbackVector. If one is discovered, it is serialized, and we
ensure we'll either return it or nothing if the contents of
the cell changed on the main thread.

FeedbackCells can be reset if the bytecode for the associated
function is flushed. We have guarantees only for functions we
choose to inline that this doesn't happen (by holding a strong
handle to the SharedFunctionInfo).

Bug: v8:7790
Change-Id: I9ecff3f4aef39169d84501feae9e47f2d118054e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2434324
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72260}

src/compiler/bytecode-graph-builder.cc

@@ -1048,7 +1048,7 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
       native_context_(native_context),
       shared_info_(shared_info),
       feedback_cell_(feedback_cell),
-      feedback_vector_(feedback_cell.value().AsFeedbackVector()),
+      feedback_vector_(feedback_cell.value()->AsFeedbackVector()),
       invocation_frequency_(invocation_frequency),
       type_hint_lowering_(
           broker, jsgraph, feedback_vector_,
@@ -4561,7 +4561,8 @@ void BuildGraphFromBytecode(JSHeapBroker* broker, Zone* local_zone,
                             BytecodeGraphBuilderFlags flags,
                             TickCounter* tick_counter) {
   DCHECK(broker->IsSerializedForCompilation(
-      shared_info, feedback_cell.value().AsFeedbackVector()));
+      shared_info, feedback_cell.value()->AsFeedbackVector()));
+  DCHECK(feedback_cell.value()->AsFeedbackVector().serialized());
   BytecodeGraphBuilder builder(
       broker, local_zone, broker->target_native_context(), shared_info,
       feedback_cell, osr_offset, jsgraph, invocation_frequency,

src/compiler/heap-refs.h

@@ -72,6 +72,7 @@ enum class OddballType : uint8_t {
   V(ArrayBoilerplateDescription)                    \
   V(CallHandlerInfo)                                \
   V(Cell)                                           \
+  V(FeedbackCell)                                   \
   V(SharedFunctionInfo)                             \
   V(TemplateObjectDescription)
 
@@ -116,7 +117,6 @@ enum class OddballType : uint8_t {
   V(AllocationSite)                           \
   V(Code)                                     \
   V(DescriptorArray)                          \
-  V(FeedbackCell)                             \
   V(FeedbackVector)                           \
   V(FixedArrayBase)                           \
   V(FunctionTemplateInfo)                     \
@@ -544,7 +544,7 @@ class FeedbackCellRef : public HeapObjectRef {
 
   Handle<FeedbackCell> object() const;
   base::Optional<SharedFunctionInfoRef> shared_function_info() const;
-  HeapObjectRef value() const;
+  base::Optional<FeedbackVectorRef> value() const;
 };
 
 class FeedbackVectorRef : public HeapObjectRef {

src/compiler/js-call-reducer.cc

@@ -4073,8 +4073,13 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
     return ReduceJSCall(node, SharedFunctionInfoRef(broker(), p.shared_info()));
   } else if (target->opcode() == IrOpcode::kCheckClosure) {
     FeedbackCellRef cell(broker(), FeedbackCellOf(target->op()));
-    return ReduceJSCall(node,
-                        cell.value().AsFeedbackVector().shared_function_info());
+    if (cell.shared_function_info().has_value()) {
+      return ReduceJSCall(node, *cell.shared_function_info());
+    } else {
+      TRACE_BROKER_MISSING(broker(), "Unable to reduce JSCall. FeedbackCell "
+                                         << cell << " has no FeedbackVector");
+      return NoChange();
+    }
   }
 
   // If {target} is the result of a JSCreateBoundFunction operation,
@@ -4153,11 +4158,10 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
   } else if (feedback_target.has_value() && feedback_target->IsFeedbackCell()) {
     FeedbackCellRef feedback_cell(
         broker(), feedback_target.value().AsFeedbackCell().object());
-    if (feedback_cell.value().IsFeedbackVector()) {
+    if (feedback_cell.value().has_value()) {
       // Check that {target} is a closure with given {feedback_cell},
       // which uniquely identifies a given function inside a native context.
-      FeedbackVectorRef feedback_vector =
-          feedback_cell.value().AsFeedbackVector();
+      FeedbackVectorRef feedback_vector = *feedback_cell.value();
       if (!feedback_vector.serialized()) {
         TRACE_BROKER_MISSING(
             broker(), "feedback vector, not serialized: " << feedback_vector);

src/compiler/js-heap-broker.h

@@ -158,6 +158,13 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
       Object, ObjectRef::BackgroundSerialization background_serialization =
                   ObjectRef::BackgroundSerialization::kDisallowed);
 
+  // Gets data only if we have it. However, thin wrappers will be created for
+  // smis, read-only objects and never-serialized objects.
+  ObjectData* TryGetOrCreateData(
+      Handle<Object>, bool crash_on_error,
+      ObjectRef::BackgroundSerialization background_serialization =
+          ObjectRef::BackgroundSerialization::kDisallowed);
+
   // Check if {object} is any native context's %ArrayPrototype% or
   // %ObjectPrototype%.
   bool IsArrayOrObjectPrototype(const JSObjectRef& object) const;

src/compiler/js-heap-copy-reducer.cc

@@ -29,8 +29,10 @@ Reduction JSHeapCopyReducer::Reduce(Node* node) {
   switch (node->opcode()) {
     case IrOpcode::kCheckClosure: {
       FeedbackCellRef cell(broker(), FeedbackCellOf(node->op()));
-      FeedbackVectorRef feedback_vector = cell.value().AsFeedbackVector();
-      feedback_vector.Serialize();
+      base::Optional<FeedbackVectorRef> feedback_vector = cell.value();
+      if (feedback_vector.has_value()) {
+        feedback_vector->Serialize();
+      }
       break;
     }
     case IrOpcode::kHeapConstant: {

src/compiler/js-inlining-heuristic.cc

@@ -111,12 +111,9 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
   if (m.IsCheckClosure()) {
     DCHECK(!out.functions[0].has_value());
     FeedbackCellRef feedback_cell(broker(), FeedbackCellOf(m.op()));
-    SharedFunctionInfoRef shared_info =
-        feedback_cell.shared_function_info().value();
+    SharedFunctionInfoRef shared_info = *feedback_cell.shared_function_info();
     out.shared_info = shared_info;
-    if (feedback_cell.value().IsFeedbackVector() &&
-        CanConsiderForInlining(broker(), shared_info,
-                               feedback_cell.value().AsFeedbackVector())) {
+    if (CanConsiderForInlining(broker(), shared_info, *feedback_cell.value())) {
       out.bytecode[0] = shared_info.GetBytecodeArray();
     }
     out.num_functions = 1;
@@ -129,9 +126,8 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
     FeedbackCellRef feedback_cell = n.GetFeedbackCellRefChecked(broker());
     SharedFunctionInfoRef shared_info(broker(), p.shared_info());
     out.shared_info = shared_info;
-    if (feedback_cell.value().IsFeedbackVector() &&
-        CanConsiderForInlining(broker(), shared_info,
-                               feedback_cell.value().AsFeedbackVector())) {
+    if (feedback_cell.value().has_value() &&
+        CanConsiderForInlining(broker(), shared_info, *feedback_cell.value())) {
       out.bytecode[0] = shared_info.GetBytecodeArray();
     }
     out.num_functions = 1;

src/compiler/js-inlining.cc

@@ -431,15 +431,19 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
                                                        shared_info->object());
   }
 
-  TRACE("Inlining " << *shared_info << " into " << outer_shared_info
-                    << ((exception_target != nullptr) ? " (inside try-block)"
-                                                      : ""));
   // Determine the target's feedback vector and its context.
   Node* context;
   FeedbackCellRef feedback_cell = DetermineCallContext(node, &context);
-  CHECK(broker()->IsSerializedForCompilation(
-      *shared_info, feedback_cell.value().AsFeedbackVector()));
+  if (!broker()->IsSerializedForCompilation(*shared_info,
+                                            *feedback_cell.value())) {
+    TRACE("Not inlining " << *shared_info << " into " << outer_shared_info
+                          << " because it wasn't serialized for compilation.");
+    return NoChange();
+  }
 
+  TRACE("Inlining " << *shared_info << " into " << outer_shared_info
+                    << ((exception_target != nullptr) ? " (inside try-block)"
+                                                      : ""));
   // ----------------------------------------------------------------
   // After this point, we've made a decision to inline this function.
   // We shall not bailout from inlining if we got here.

src/compiler/js-typed-lowering.cc

@@ -1713,7 +1713,10 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
     shared = SharedFunctionInfoRef(broker(), ccp.shared_info());
   } else if (target->opcode() == IrOpcode::kCheckClosure) {
     FeedbackCellRef cell(broker(), FeedbackCellOf(target->op()));
-    shared = cell.value().AsFeedbackVector().shared_function_info();
+    base::Optional<FeedbackVectorRef> feedback_vector = cell.value();
+    if (feedback_vector.has_value()) {
+      shared = feedback_vector->shared_function_info();
+    }
   }
 
   if (shared.has_value()) {

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

@@ -2142,10 +2142,8 @@ void SerializerForBackgroundCompilation::ProcessCallOrConstruct(
           callee.AddConstant(target->object(), zone(), broker());
         } else {
           // Call; target is feedback cell or callee.
-          if (target->IsFeedbackCell() &&
-              target->AsFeedbackCell().value().IsFeedbackVector()) {
-            FeedbackVectorRef vector =
-                target->AsFeedbackCell().value().AsFeedbackVector();
+          if (target->IsFeedbackCell() && target->AsFeedbackCell().value()) {
+            FeedbackVectorRef vector = *target->AsFeedbackCell().value();
             vector.Serialize();
             VirtualClosure virtual_closure(
                 vector.shared_function_info().object(), vector.object(),

src/diagnostics/objects-printer.cc

@@ -1054,6 +1054,8 @@ void FeedbackVector::FeedbackVectorPrint(std::ostream& os) {  // NOLINT
   os << "\n - optimization tier: " << optimization_tier();
   os << "\n - invocation count: " << invocation_count();
   os << "\n - profiler ticks: " << profiler_ticks();
+  os << "\n - closure feedback cell array: ";
+  closure_feedback_cell_array().ClosureFeedbackCellArrayPrint(os);
 
   FeedbackMetadataIterator iter(metadata());
   while (iter.HasNext()) {
@@ -1421,6 +1423,9 @@ void JSFunction::JSFunctionPrint(std::ostream& os) {  // NOLINT
     os << "feedback metadata is not available in SFI\n";
   } else if (has_feedback_vector()) {
     feedback_vector().FeedbackVectorPrint(os);
+  } else if (has_closure_feedback_cell_array()) {
+    os << "No feedback vector, but we have a closure feedback cell array\n";
+    closure_feedback_cell_array().ClosureFeedbackCellArrayPrint(os);
   } else {
     os << "not available\n";
   }

src/objects/feedback-cell-inl.h

@@ -21,6 +21,8 @@ namespace internal {
 
 TQ_OBJECT_CONSTRUCTORS_IMPL(FeedbackCell)
 
+RELEASE_ACQUIRE_ACCESSORS(FeedbackCell, value, HeapObject, kValueOffset)
+
 void FeedbackCell::clear_padding() {
   if (FeedbackCell::kAlignedSize == FeedbackCell::kUnalignedSize) return;
   DCHECK_GE(FeedbackCell::kAlignedSize, FeedbackCell::kUnalignedSize);

src/objects/feedback-cell.h

@@ -28,6 +28,11 @@ class FeedbackCell : public TorqueGeneratedFeedbackCell<FeedbackCell, Struct> {
   static const int kUnalignedSize = kSize;
   static const int kAlignedSize = RoundUp<kObjectAlignment>(int{kSize});
 
+  using TorqueGeneratedFeedbackCell<FeedbackCell, Struct>::value;
+  using TorqueGeneratedFeedbackCell<FeedbackCell, Struct>::set_value;
+
+  DECL_RELEASE_ACQUIRE_ACCESSORS(value, HeapObject)
+
   inline void clear_padding();
   inline void reset_feedback_vector(
       base::Optional<std::function<void(HeapObject object, ObjectSlot slot,

src/objects/js-function-inl.h

@@ -29,6 +29,8 @@ OBJECT_CONSTRUCTORS_IMPL(JSFunction, JSFunctionOrBoundFunction)
 CAST_ACCESSOR(JSFunction)
 
 ACCESSORS(JSFunction, raw_feedback_cell, FeedbackCell, kFeedbackCellOffset)
+RELEASE_ACQUIRE_ACCESSORS(JSFunction, raw_feedback_cell, FeedbackCell,
+                          kFeedbackCellOffset)
 
 FeedbackVector JSFunction::feedback_vector() const {
   DCHECK(has_feedback_vector());

src/objects/js-function.cc

@@ -282,9 +282,10 @@ void JSFunction::EnsureClosureFeedbackCellArray(Handle<JSFunction> function) {
   if (function->raw_feedback_cell() == isolate->heap()->many_closures_cell()) {
     Handle<FeedbackCell> feedback_cell =
         isolate->factory()->NewOneClosureCell(feedback_cell_array);
-    function->set_raw_feedback_cell(*feedback_cell);
+    function->set_raw_feedback_cell(*feedback_cell, kReleaseStore);
   } else {
-    function->raw_feedback_cell().set_value(*feedback_cell_array);
+    function->raw_feedback_cell().set_value(*feedback_cell_array,
+                                            kReleaseStore);
   }
 }
 
@@ -310,7 +311,7 @@ void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function,
   // for more details.
   DCHECK(function->raw_feedback_cell() !=
          isolate->heap()->many_closures_cell());
-  function->raw_feedback_cell().set_value(*feedback_vector);
+  function->raw_feedback_cell().set_value(*feedback_vector, kReleaseStore);
   function->raw_feedback_cell().SetInterruptBudget();
 }
 

src/objects/js-function.h

@@ -165,6 +165,11 @@ class JSFunction : public JSFunctionOrBoundFunction {
   // the JSFunction's bytecode being flushed.
   DECL_ACCESSORS(raw_feedback_cell, FeedbackCell)
 
+  // [raw_feedback_cell] (synchronized version) When this is initialized from a
+  // newly allocated object (instead of a root sentinel), it should
+  // be written with release store semantics.
+  DECL_RELEASE_ACQUIRE_ACCESSORS(raw_feedback_cell, FeedbackCell)
+
   // Functions related to feedback vector. feedback_vector() can be used once
   // the function has feedback vectors allocated. feedback vectors may not be
   // available after compile when lazily allocating feedback vectors.