Reland "[turbofan] Prepare for moving part of CreateGraph into the background"

This is a reland of ab089c78, after
making a flaky test more robust.

Original change's description:
> [turbofan] Prepare for moving part of CreateGraph into the background
>
> - Pass Refs, not Handles, to graph builder, and drop bytecode array argument
>   (get it from SFI instead).
> - Add some fields to FeedbackVectorRef that are needed to avoid heap access
>   in BytecodeGraphBuilderPhase.
> - Rename FeedbackVectorRef's SerializeSlots to Serialize, since it's more
>   than just the feedback slots.
> - Rearrange the last steps in PipelineCompilationJob::PrepareJobImpl such
>   that CreateGraph is last.
>
> Bug: v8:7790
> Change-Id: I4b17790d1d74da41ba63ee68e3a33968662fc398
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1781682
> Reviewed-by: Maya Lekova <mslekova@chromium.org>
> Commit-Queue: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#63515}

Bug: v8:7790
Change-Id: Ia6f4c1ebd82dea93c14437514d0e25b730523f75
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1781694Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63545}

src/codegen/optimized-compilation-info.cc

@@ -21,6 +21,7 @@ OptimizedCompilationInfo::OptimizedCompilationInfo(
     Zone* zone, Isolate* isolate, Handle<SharedFunctionInfo> shared,
     Handle<JSFunction> closure)
     : OptimizedCompilationInfo(Code::OPTIMIZED_FUNCTION, zone) {
+  DCHECK_EQ(*shared, closure->shared());
   DCHECK(shared->is_compiled());
   bytecode_array_ = handle(shared->GetBytecodeArray(), isolate);
   shared_info_ = shared;

src/compiler/bytecode-graph-builder.cc

@@ -33,13 +33,12 @@ namespace compiler {
 class BytecodeGraphBuilder {
  public:
   BytecodeGraphBuilder(JSHeapBroker* broker, Zone* local_zone,
-                       BytecodeArrayRef bytecode_array,
-                       SharedFunctionInfoRef shared,
-                       FeedbackVectorRef feedback_vector, BailoutId osr_offset,
-                       JSGraph* jsgraph,
+                       NativeContextRef const& native_context,
+                       SharedFunctionInfoRef const& shared_info,
+                       FeedbackVectorRef const& feedback_vector,
+                       BailoutId osr_offset, JSGraph* jsgraph,
                        CallFrequency const& invocation_frequency,
-                       SourcePositionTable* source_positions,
-                       NativeContextRef native_context, int inlining_id,
+                       SourcePositionTable* source_positions, int inlining_id,
                        BytecodeGraphBuilderFlags flags,
                        TickCounter* tick_counter);
 
@@ -310,7 +309,6 @@ class BytecodeGraphBuilder {
     int context_register_;  // Index of register holding handler context.
   };
 
-  // Field accessors
   Graph* graph() const { return jsgraph_->graph(); }
   CommonOperatorBuilder* common() const { return jsgraph_->common(); }
   Zone* graph_zone() const { return graph()->zone(); }
@@ -321,55 +319,44 @@ class BytecodeGraphBuilder {
     return jsgraph_->simplified();
   }
   Zone* local_zone() const { return local_zone_; }
-  const BytecodeArrayRef bytecode_array() const { return bytecode_array_; }
-  FeedbackVectorRef feedback_vector() const { return feedback_vector_; }
+  BytecodeArrayRef bytecode_array() const {
+    return shared_info().GetBytecodeArray();
+  }
+  FeedbackVectorRef const& feedback_vector() const { return feedback_vector_; }
   const JSTypeHintLowering& type_hint_lowering() const {
     return type_hint_lowering_;
   }
   const FrameStateFunctionInfo* frame_state_function_info() const {
     return frame_state_function_info_;
   }
-
   SourcePositionTableIterator& source_position_iterator() {
     return *source_position_iterator_.get();
   }
-
   interpreter::BytecodeArrayIterator& bytecode_iterator() {
     return bytecode_iterator_;
   }
-
   BytecodeAnalysis const& bytecode_analysis() const {
     return bytecode_analysis_;
   }
-
   int currently_peeled_loop_offset() const {
     return currently_peeled_loop_offset_;
   }
-
   void set_currently_peeled_loop_offset(int offset) {
     currently_peeled_loop_offset_ = offset;
   }
-
   bool skip_next_stack_check() const { return skip_next_stack_check_; }
-
   void unset_skip_next_stack_check() { skip_next_stack_check_ = false; }
-
-  int current_exception_handler() { return current_exception_handler_; }
-
+  int current_exception_handler() const { return current_exception_handler_; }
   void set_current_exception_handler(int index) {
     current_exception_handler_ = index;
   }
-
   bool needs_eager_checkpoint() const { return needs_eager_checkpoint_; }
   void mark_as_needing_eager_checkpoint(bool value) {
     needs_eager_checkpoint_ = value;
   }
-
-  SharedFunctionInfoRef shared_info() const { return shared_info_; }
-
-  NativeContextRef native_context() const { return native_context_; }
-
   JSHeapBroker* broker() const { return broker_; }
+  NativeContextRef native_context() const { return native_context_; }
+  SharedFunctionInfoRef shared_info() const { return shared_info_; }
 
 #define DECLARE_VISIT_BYTECODE(name, ...) void Visit##name();
   BYTECODE_LIST(DECLARE_VISIT_BYTECODE)
@@ -378,9 +365,11 @@ class BytecodeGraphBuilder {
   JSHeapBroker* const broker_;
   Zone* const local_zone_;
   JSGraph* const jsgraph_;
+  // The native context for which we optimize.
+  NativeContextRef const native_context_;
+  SharedFunctionInfoRef const shared_info_;
+  FeedbackVectorRef const feedback_vector_;
   CallFrequency const invocation_frequency_;
-  BytecodeArrayRef const bytecode_array_;
-  FeedbackVectorRef feedback_vector_;
   JSTypeHintLowering const type_hint_lowering_;
   const FrameStateFunctionInfo* const frame_state_function_info_;
   std::unique_ptr<SourcePositionTableIterator> source_position_iterator_;
@@ -431,11 +420,6 @@ class BytecodeGraphBuilder {
 
   SourcePosition const start_position_;
 
-  SharedFunctionInfoRef const shared_info_;
-
-  // The native context for which we optimize.
-  NativeContextRef const native_context_;
-
   TickCounter* const tick_counter_;
 
   static int const kBinaryOperationHintIndex = 1;
@@ -937,18 +921,20 @@ Node* BytecodeGraphBuilder::Environment::Checkpoint(
 }
 
 BytecodeGraphBuilder::BytecodeGraphBuilder(
-    JSHeapBroker* broker, Zone* local_zone, BytecodeArrayRef bytecode_array,
-    SharedFunctionInfoRef shared_info, FeedbackVectorRef feedback_vector,
-    BailoutId osr_offset, JSGraph* jsgraph,
-    CallFrequency const& invocation_frequency,
-    SourcePositionTable* source_positions, NativeContextRef native_context,
-    int inlining_id, BytecodeGraphBuilderFlags flags, TickCounter* tick_counter)
+    JSHeapBroker* broker, Zone* local_zone,
+    NativeContextRef const& native_context,
+    SharedFunctionInfoRef const& shared_info,
+    FeedbackVectorRef const& feedback_vector, BailoutId osr_offset,
+    JSGraph* jsgraph, CallFrequency const& invocation_frequency,
+    SourcePositionTable* source_positions, int inlining_id,
+    BytecodeGraphBuilderFlags flags, TickCounter* tick_counter)
     : broker_(broker),
       local_zone_(local_zone),
       jsgraph_(jsgraph),
-      invocation_frequency_(invocation_frequency),
-      bytecode_array_(bytecode_array),
+      native_context_(native_context),
+      shared_info_(shared_info),
       feedback_vector_(feedback_vector),
+      invocation_frequency_(invocation_frequency),
       type_hint_lowering_(
           broker, jsgraph, feedback_vector,
           (flags & BytecodeGraphBuilderFlag::kBailoutOnUninitialized)
@@ -956,12 +942,12 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
               : JSTypeHintLowering::kNoFlags),
       frame_state_function_info_(common()->CreateFrameStateFunctionInfo(
           FrameStateType::kInterpretedFunction,
-          bytecode_array.parameter_count(), bytecode_array.register_count(),
+          bytecode_array().parameter_count(), bytecode_array().register_count(),
           shared_info.object())),
       bytecode_iterator_(
-          base::make_unique<OffHeapBytecodeArray>(bytecode_array)),
+          base::make_unique<OffHeapBytecodeArray>(bytecode_array())),
       bytecode_analysis_(broker_->GetBytecodeAnalysis(
-          bytecode_array.object(), osr_offset,
+          bytecode_array().object(), osr_offset,
           flags & BytecodeGraphBuilderFlag::kAnalyzeEnvironmentLiveness,
           FLAG_concurrent_inlining ? SerializationPolicy::kAssumeSerialized
                                    : SerializationPolicy::kSerializeIfNeeded)),
@@ -981,19 +967,17 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
       state_values_cache_(jsgraph),
       source_positions_(source_positions),
       start_position_(shared_info.StartPosition(), inlining_id),
-      shared_info_(shared_info),
-      native_context_(native_context),
       tick_counter_(tick_counter) {
   if (FLAG_concurrent_inlining) {
     // With concurrent inlining on, the source position address doesn't change
     // because it's been copied from the heap.
     source_position_iterator_ = base::make_unique<SourcePositionTableIterator>(
-        Vector<const byte>(bytecode_array.source_positions_address(),
-                           bytecode_array.source_positions_size()));
+        Vector<const byte>(bytecode_array().source_positions_address(),
+                           bytecode_array().source_positions_size()));
   } else {
     // Otherwise, we need to access the table through a handle.
     source_position_iterator_ = base::make_unique<SourcePositionTableIterator>(
-        handle(bytecode_array.object()->SourcePositionTableIfCollected(),
+        handle(bytecode_array().object()->SourcePositionTableIfCollected(),
                isolate()));
   }
 }
@@ -4067,23 +4051,18 @@ void BytecodeGraphBuilder::UpdateSourcePosition(int offset) {
 }
 
 void BuildGraphFromBytecode(JSHeapBroker* broker, Zone* local_zone,
-                            Handle<BytecodeArray> bytecode_array,
-                            Handle<SharedFunctionInfo> shared,
-                            Handle<FeedbackVector> feedback_vector,
+                            SharedFunctionInfoRef const& shared_info,
+                            FeedbackVectorRef const& feedback_vector,
                             BailoutId osr_offset, JSGraph* jsgraph,
                             CallFrequency const& invocation_frequency,
                             SourcePositionTable* source_positions,
                             int inlining_id, BytecodeGraphBuilderFlags flags,
                             TickCounter* tick_counter) {
-  BytecodeArrayRef bytecode_array_ref(broker, bytecode_array);
-  DCHECK(bytecode_array_ref.IsSerializedForCompilation());
-  FeedbackVectorRef feedback_vector_ref(broker, feedback_vector);
-  SharedFunctionInfoRef shared_ref(broker, shared);
-  DCHECK(shared_ref.IsSerializedForCompilation(feedback_vector_ref));
+  DCHECK(shared_info.IsSerializedForCompilation(feedback_vector));
   BytecodeGraphBuilder builder(
-      broker, local_zone, bytecode_array_ref, shared_ref, feedback_vector_ref,
-      osr_offset, jsgraph, invocation_frequency, source_positions,
-      broker->target_native_context(), inlining_id, flags, tick_counter);
+      broker, local_zone, broker->target_native_context(), shared_info,
+      feedback_vector, osr_offset, jsgraph, invocation_frequency,
+      source_positions, inlining_id, flags, tick_counter);
   builder.CreateGraph();
 }
 

src/compiler/bytecode-graph-builder.h

@@ -39,9 +39,8 @@ using BytecodeGraphBuilderFlags = base::Flags<BytecodeGraphBuilderFlag>;
 // Note: {invocation_frequency} is taken by reference to work around a GCC bug
 // on AIX (v8:8193).
 void BuildGraphFromBytecode(JSHeapBroker* broker, Zone* local_zone,
-                            Handle<BytecodeArray> bytecode_array,
-                            Handle<SharedFunctionInfo> shared,
-                            Handle<FeedbackVector> feedback_vector,
+                            SharedFunctionInfoRef const& shared_info,
+                            FeedbackVectorRef const& feedback_vector,
                             BailoutId osr_offset, JSGraph* jsgraph,
                             CallFrequency const& invocation_frequency,
                             SourcePositionTable* source_positions,

src/compiler/heap-refs.h

@@ -493,10 +493,12 @@ class FeedbackVectorRef : public HeapObjectRef {
 
   Handle<FeedbackVector> object() const;
 
+  double invocation_count() const;
+  double total_profiler_ticks() const;
+
+  void Serialize();
   ObjectRef get(FeedbackSlot slot) const;
   FeedbackCellRef GetClosureFeedbackCell(int index) const;
-
-  void SerializeSlots();
 };
 
 class CallHandlerInfoRef : public HeapObjectRef {

src/compiler/js-heap-broker.cc

@@ -1262,17 +1262,21 @@ FeedbackCellData::FeedbackCellData(JSHeapBroker* broker, ObjectData** storage,
 
 class FeedbackVectorData : public HeapObjectData {
  public:
-  const ZoneVector<ObjectData*>& feedback() { return feedback_; }
-
   FeedbackVectorData(JSHeapBroker* broker, ObjectData** storage,
                      Handle<FeedbackVector> object);
 
+  double invocation_count() const { return invocation_count_; }
+  double total_profiler_ticks() const { return total_profiler_ticks_; }
+
+  void Serialize(JSHeapBroker* broker);
+  const ZoneVector<ObjectData*>& feedback() { return feedback_; }
   FeedbackCellData* GetClosureFeedbackCell(JSHeapBroker* broker,
                                            int index) const;
 
-  void SerializeSlots(JSHeapBroker* broker);
-
  private:
+  double const invocation_count_;
+  double const total_profiler_ticks_;
+
   bool serialized_ = false;
   ZoneVector<ObjectData*> feedback_;
   ZoneVector<ObjectData*> closure_feedback_cell_array_;
@@ -1282,6 +1286,8 @@ FeedbackVectorData::FeedbackVectorData(JSHeapBroker* broker,
                                        ObjectData** storage,
                                        Handle<FeedbackVector> object)
     : HeapObjectData(broker, storage, object),
+      invocation_count_(object->invocation_count()),
+      total_profiler_ticks_(object->total_profiler_ticks()),
       feedback_(broker->zone()),
       closure_feedback_cell_array_(broker->zone()) {}
 
@@ -1300,11 +1306,11 @@ FeedbackCellData* FeedbackVectorData::GetClosureFeedbackCell(
   return closure_feedback_cell_array_[index]->AsFeedbackCell();
 }
 
-void FeedbackVectorData::SerializeSlots(JSHeapBroker* broker) {
+void FeedbackVectorData::Serialize(JSHeapBroker* broker) {
   if (serialized_) return;
   serialized_ = true;
 
-  TraceScope tracer(broker, this, "FeedbackVectorData::SerializeSlots");
+  TraceScope tracer(broker, this, "FeedbackVectorData::Serialize");
   Handle<FeedbackVector> vector = Handle<FeedbackVector>::cast(object());
   DCHECK(feedback_.empty());
   feedback_.reserve(vector->length());
@@ -3164,6 +3170,9 @@ BIMODAL_ACCESSOR_C(BytecodeArray, interpreter::Register,
 
 BIMODAL_ACCESSOR(Cell, Object, value)
 
+BIMODAL_ACCESSOR_C(FeedbackVector, double, invocation_count)
+BIMODAL_ACCESSOR_C(FeedbackVector, double, total_profiler_ticks)
+
 BIMODAL_ACCESSOR(HeapObject, Map, map)
 
 BIMODAL_ACCESSOR(JSArray, Object, length)
@@ -3734,8 +3743,8 @@ Float64 FixedDoubleArrayData::Get(int i) const {
   return contents_[i];
 }
 
-void FeedbackVectorRef::SerializeSlots() {
-  data()->AsFeedbackVector()->SerializeSlots(broker());
+void FeedbackVectorRef::Serialize() {
+  data()->AsFeedbackVector()->Serialize(broker());
 }
 
 bool NameRef::IsUniqueName() const {
@@ -3912,7 +3921,6 @@ void SharedFunctionInfoRef::SetSerializedForCompilation(
 
 void SharedFunctionInfoRef::SerializeFunctionTemplateInfo() {
   CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-
   data()->AsSharedFunctionInfo()->SerializeFunctionTemplateInfo(broker());
 }
 

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

@@ -89,7 +89,7 @@ Reduction JSHeapCopyReducer::Reduce(Node* node) {
     case IrOpcode::kJSCreateEmptyLiteralArray: {
       if (!FLAG_concurrent_inlining) {
         FeedbackParameter const& p = FeedbackParameterOf(node->op());
-        FeedbackVectorRef(broker(), p.feedback().vector).SerializeSlots();
+        FeedbackVectorRef(broker(), p.feedback().vector).Serialize();
       }
       break;
     }
@@ -106,7 +106,7 @@ Reduction JSHeapCopyReducer::Reduce(Node* node) {
       if (!FLAG_concurrent_inlining) {
         CreateLiteralParameters const& p =
             CreateLiteralParametersOf(node->op());
-        FeedbackVectorRef(broker(), p.feedback().vector).SerializeSlots();
+        FeedbackVectorRef(broker(), p.feedback().vector).Serialize();
       }
       break;
     }
@@ -114,7 +114,7 @@ Reduction JSHeapCopyReducer::Reduce(Node* node) {
       if (!FLAG_concurrent_inlining) {
         CreateLiteralParameters const& p =
             CreateLiteralParametersOf(node->op());
-        FeedbackVectorRef(broker(), p.feedback().vector).SerializeSlots();
+        FeedbackVectorRef(broker(), p.feedback().vector).Serialize();
       }
       break;
     }

src/compiler/js-inlining.cc

@@ -467,10 +467,10 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
     }
     {
       CallFrequency frequency = call.frequency();
-      BuildGraphFromBytecode(
-          broker(), zone(), bytecode_array.object(), shared_info->object(),
-          feedback_vector.object(), BailoutId::None(), jsgraph(), frequency,
-          source_positions_, inlining_id, flags, &info_->tick_counter());
+      BuildGraphFromBytecode(broker(), zone(), *shared_info, feedback_vector,
+                             BailoutId::None(), jsgraph(), frequency,
+                             source_positions_, inlining_id, flags,
+                             &info_->tick_counter());
     }
 
     // Extract the inlinee start/end nodes.

src/compiler/pipeline.cc

@@ -992,11 +992,6 @@ PipelineCompilationJob::Status PipelineCompilationJob::PrepareJobImpl(
   linkage_ = new (compilation_info()->zone()) Linkage(
       Linkage::ComputeIncoming(compilation_info()->zone(), compilation_info()));
 
-  if (!pipeline_.CreateGraph()) {
-    if (isolate->has_pending_exception()) return FAILED;  // Stack overflowed.
-    return AbortOptimization(BailoutReason::kGraphBuildingFailed);
-  }
-
   if (compilation_info()->is_osr()) data_.InitializeOsrHelper();
 
   // Make sure that we have generated the deopt entries code.  This is in order
@@ -1004,6 +999,11 @@ PipelineCompilationJob::Status PipelineCompilationJob::PrepareJobImpl(
   // assembly.
   Deoptimizer::EnsureCodeForDeoptimizationEntries(isolate);
 
+  if (!pipeline_.CreateGraph()) {
+    if (isolate->has_pending_exception()) return FAILED;  // Stack overflowed.
+    return AbortOptimization(BailoutReason::kGraphBuildingFailed);
+  }
+
   return SUCCEEDED;
 }
 
@@ -1201,10 +1201,10 @@ struct GraphBuilderPhase {
     if (data->info()->is_bailout_on_uninitialized()) {
       flags |= BytecodeGraphBuilderFlag::kBailoutOnUninitialized;
     }
-    double invocation_count =
-        data->info()->closure()->feedback_vector().invocation_count();
-    double total_ticks =
-        data->info()->closure()->feedback_vector().total_profiler_ticks();
+
+    JSFunctionRef closure(data->broker(), data->info()->closure());
+    double invocation_count = closure.feedback_vector().invocation_count();
+    double total_ticks = closure.feedback_vector().total_profiler_ticks();
     if (total_ticks == 0) {
       // This can only happen in tests when forcing optimization.
       // Pick a small number so that inlining still happens.
@@ -1212,10 +1212,9 @@ struct GraphBuilderPhase {
     }
     double executed_bytecode_bytes = total_ticks * FLAG_interrupt_budget;
     CallFrequency frequency(invocation_count / (executed_bytecode_bytes / KB));
+
     BuildGraphFromBytecode(
-        data->broker(), temp_zone, data->info()->bytecode_array(),
-        data->info()->shared_info(),
-        handle(data->info()->closure()->feedback_vector(), data->isolate()),
+        data->broker(), temp_zone, closure.shared(), closure.feedback_vector(),
         data->info()->osr_offset(), data->jsgraph(), frequency,
         data->source_positions(), SourcePosition::kNotInlined, flags,
         &data->info()->tick_counter());

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

@@ -921,7 +921,7 @@ Hints SerializerForBackgroundCompilation::Run() {
                                                        shared.object());
   }
 
-  feedback_vector_ref.SerializeSlots();
+  feedback_vector_ref.Serialize();
   TraverseBytecode();
   return environment()->return_value_hints();
 }

test/mjsunit/compiler/bigint-int64-lowered.js

@@ -70,6 +70,7 @@ function TestInt64LoweredOperations() {
 
 function OptimizeAndTest(fn) {
   %PrepareFunctionForOptimization(fn);
+  %PrepareFunctionForOptimization(assertEquals);
   fn();
   fn();
   %OptimizeFunctionOnNextCall(fn);