[compiler] Fix two FeedbackCellRef uses

1) Code in JSCallReducer read a FeedbackCell twice and expected the
   result to be the same.

2) JSInliningHeuristics, in the CheckClosure case, assumed that the
   FeedbackCell contains a FeedbackVector.

Bug: chromium:1248743, v8:7790
Change-Id: I66d6dd5f7a879c2479572e1896dd78aeedd2fa27
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3160200Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76815}

src/compiler/js-call-reducer.cc

@@ -4368,13 +4368,13 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
     return ReduceJSCall(node, p.shared_info(broker()));
   } else if (target->opcode() == IrOpcode::kCheckClosure) {
     FeedbackCellRef cell = MakeRef(broker(), FeedbackCellOf(target->op()));
-    if (cell.shared_function_info().has_value()) {
-      return ReduceJSCall(node, *cell.shared_function_info());
-    } else {
+    base::Optional<SharedFunctionInfoRef> shared = cell.shared_function_info();
+    if (!shared.has_value()) {
       TRACE_BROKER_MISSING(broker(), "Unable to reduce JSCall. FeedbackCell "
                                          << cell << " has no FeedbackVector");
       return NoChange();
     }
+    return ReduceJSCall(node, *shared);
   }
 
   // If {target} is the result of a JSCreateBoundFunction operation,

src/compiler/js-inlining-heuristic.cc

@@ -27,8 +27,16 @@ bool IsSmall(int const size) {
 }
 
 bool CanConsiderForInlining(JSHeapBroker* broker,
-                            SharedFunctionInfoRef const& shared,
                             FeedbackCellRef const& feedback_cell) {
+  base::Optional<FeedbackVectorRef> feedback_vector =
+      feedback_cell.feedback_vector();
+  if (!feedback_vector.has_value()) {
+    TRACE("Cannot consider " << feedback_cell
+                             << " for inlining (no feedback vector)");
+    return false;
+  }
+  SharedFunctionInfoRef shared = feedback_vector->shared_function_info();
+
   if (!shared.HasBytecodeArray()) {
     TRACE("Cannot consider " << shared << " for inlining (no bytecode)");
     return false;
@@ -37,14 +45,21 @@ bool CanConsiderForInlining(JSHeapBroker* broker,
   // flushing it during the remaining compilation.
   shared.GetBytecodeArray();
 
-  base::Optional<FeedbackVectorRef> feedback_vector =
+  // Read feedback vector again in case it got flushed before we were able to
+  // prevent flushing above.
+  base::Optional<FeedbackVectorRef> feedback_vector_again =
       feedback_cell.feedback_vector();
-  if (!feedback_vector.has_value()) {
+  if (!feedback_vector_again.has_value()) {
     TRACE("Cannot consider " << shared << " for inlining (no feedback vector)");
     return false;
   }
-  // Note that due to prevented bytecode flushing, this {feedback_cell} won't
-  // change its value anymore during compilation.
+  if (!feedback_vector_again->equals(*feedback_vector)) {
+    // The new feedback vector likely contains lots of uninitialized slots, so
+    // it doesn't make much sense to inline this function now.
+    TRACE("Not considering " << shared
+                             << " for inlining (feedback vector changed)");
+    return false;
+  }
 
   SharedFunctionInfo::Inlineability inlineability = shared.GetInlineability();
   if (inlineability != SharedFunctionInfo::kIsInlineable) {
@@ -59,9 +74,14 @@ bool CanConsiderForInlining(JSHeapBroker* broker,
 
 bool CanConsiderForInlining(JSHeapBroker* broker,
                             JSFunctionRef const& function) {
-  return CanConsiderForInlining(
-      broker, function.shared(),
-      function.raw_feedback_cell(broker->dependencies()));
+  FeedbackCellRef feedback_cell =
+      function.raw_feedback_cell(broker->dependencies());
+  bool const result = CanConsiderForInlining(broker, feedback_cell);
+  if (result) {
+    CHECK(
+        function.shared().equals(feedback_cell.shared_function_info().value()));
+  }
+  return result;
 }
 
 }  // namespace
@@ -75,8 +95,8 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
 
   HeapObjectMatcher m(callee);
   if (m.HasResolvedValue() && m.Ref(broker()).IsJSFunction()) {
-    out.functions[0] = m.Ref(broker()).AsJSFunction();
-    JSFunctionRef function = out.functions[0].value();
+    JSFunctionRef function = m.Ref(broker()).AsJSFunction();
+    out.functions[0] = function;
     if (CanConsiderForInlining(broker(), function)) {
       out.bytecode[0] = function.shared().GetBytecodeArray();
       out.num_functions = 1;
@@ -108,11 +128,9 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
   if (m.IsCheckClosure()) {
     DCHECK(!out.functions[0].has_value());
     FeedbackCellRef feedback_cell = MakeRef(broker(), FeedbackCellOf(m.op()));
-    SharedFunctionInfoRef shared_info =
-        feedback_cell.shared_function_info().value();
-    out.shared_info = shared_info;
-    if (CanConsiderForInlining(broker(), shared_info, feedback_cell)) {
-      out.bytecode[0] = shared_info.GetBytecodeArray();
+    if (CanConsiderForInlining(broker(), feedback_cell)) {
+      out.shared_info = feedback_cell.shared_function_info().value();
+      out.bytecode[0] = out.shared_info->GetBytecodeArray();
     }
     out.num_functions = 1;
     return out;
@@ -120,12 +138,11 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
   if (m.IsJSCreateClosure()) {
     DCHECK(!out.functions[0].has_value());
     JSCreateClosureNode n(callee);
-    CreateClosureParameters const& p = n.Parameters();
     FeedbackCellRef feedback_cell = n.GetFeedbackCellRefChecked(broker());
-    SharedFunctionInfoRef shared_info = p.shared_info(broker());
-    out.shared_info = shared_info;
-    if (CanConsiderForInlining(broker(), shared_info, feedback_cell)) {
-      out.bytecode[0] = shared_info.GetBytecodeArray();
+    if (CanConsiderForInlining(broker(), feedback_cell)) {
+      out.shared_info = feedback_cell.shared_function_info().value();
+      out.bytecode[0] = out.shared_info->GetBytecodeArray();
+      CHECK(out.shared_info->equals(n.Parameters().shared_info(broker())));
     }
     out.num_functions = 1;
     return out;