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

This reverts commit ab089c78.

Reason for revert: Breaking GC stress (https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/27523)

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}

TBR=neis@chromium.org,mslekova@chromium.org

Change-Id: I4dc95907657597d12cbe1ce6a8ebb694ef44e915
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7790
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1781687Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63517}

src/codegen/optimized-compilation-info.cc

@@ -21,7 +21,6 @@ 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,12 +33,13 @@ namespace compiler {
 class BytecodeGraphBuilder {
  public:
   BytecodeGraphBuilder(JSHeapBroker* broker, Zone* local_zone,
-                       NativeContextRef const& native_context,
-                       SharedFunctionInfoRef const& shared_info,
-                       FeedbackVectorRef const& feedback_vector,
-                       BailoutId osr_offset, JSGraph* jsgraph,
+                       BytecodeArrayRef bytecode_array,
+                       SharedFunctionInfoRef shared,
+                       FeedbackVectorRef feedback_vector, BailoutId osr_offset,
+                       JSGraph* jsgraph,
                        CallFrequency const& invocation_frequency,
-                       SourcePositionTable* source_positions, int inlining_id,
+                       SourcePositionTable* source_positions,
+                       NativeContextRef native_context, int inlining_id,
                        BytecodeGraphBuilderFlags flags,
                        TickCounter* tick_counter);
 
@@ -309,6 +310,7 @@ 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(); }
@@ -319,45 +321,56 @@ class BytecodeGraphBuilder {
     return jsgraph_->simplified();
   }
   Zone* local_zone() const { return local_zone_; }
-  BytecodeArrayRef bytecode_array() const {
-    return shared_info().GetBytecodeArray();
-  }
-  FeedbackVectorRef const& feedback_vector() const { return feedback_vector_; }
+  const BytecodeArrayRef bytecode_array() const { return bytecode_array_; }
+  FeedbackVectorRef 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() const { return current_exception_handler_; }
+
+  int current_exception_handler() { 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;
   }
-  JSHeapBroker* broker() const { return broker_; }
-  NativeContextRef native_context() const { return native_context_; }
+
   SharedFunctionInfoRef shared_info() const { return shared_info_; }
 
+  NativeContextRef native_context() const { return native_context_; }
+
+  JSHeapBroker* broker() const { return broker_; }
+
 #define DECLARE_VISIT_BYTECODE(name, ...) void Visit##name();
   BYTECODE_LIST(DECLARE_VISIT_BYTECODE)
 #undef DECLARE_VISIT_BYTECODE
@@ -365,11 +378,9 @@ 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_;
@@ -420,6 +431,11 @@ 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;
@@ -921,20 +937,18 @@ Node* BytecodeGraphBuilder::Environment::Checkpoint(
 }
 
 BytecodeGraphBuilder::BytecodeGraphBuilder(
-    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)
+    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)
     : broker_(broker),
       local_zone_(local_zone),
       jsgraph_(jsgraph),
-      native_context_(native_context),
-      shared_info_(shared_info),
-      feedback_vector_(feedback_vector),
       invocation_frequency_(invocation_frequency),
+      bytecode_array_(bytecode_array),
+      feedback_vector_(feedback_vector),
       type_hint_lowering_(
           broker, jsgraph, feedback_vector,
           (flags & BytecodeGraphBuilderFlag::kBailoutOnUninitialized)
@@ -942,12 +956,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)),
@@ -967,17 +981,19 @@ 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()));
   }
 }
@@ -4050,18 +4066,23 @@ void BytecodeGraphBuilder::UpdateSourcePosition(int offset) {
 }
 
 void BuildGraphFromBytecode(JSHeapBroker* broker, Zone* local_zone,
-                            SharedFunctionInfoRef const& shared_info,
-                            FeedbackVectorRef const& feedback_vector,
+                            Handle<BytecodeArray> bytecode_array,
+                            Handle<SharedFunctionInfo> shared,
+                            Handle<FeedbackVector> feedback_vector,
                             BailoutId osr_offset, JSGraph* jsgraph,
                             CallFrequency const& invocation_frequency,
                             SourcePositionTable* source_positions,
                             int inlining_id, BytecodeGraphBuilderFlags flags,
                             TickCounter* tick_counter) {
-  DCHECK(shared_info.IsSerializedForCompilation(feedback_vector));
+  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));
   BytecodeGraphBuilder builder(
-      broker, local_zone, broker->target_native_context(), shared_info,
-      feedback_vector, osr_offset, jsgraph, invocation_frequency,
-      source_positions, inlining_id, flags, tick_counter);
+      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);
   builder.CreateGraph();
 }
 

src/compiler/bytecode-graph-builder.h

@@ -39,8 +39,9 @@ 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,
-                            SharedFunctionInfoRef const& shared_info,
-                            FeedbackVectorRef const& feedback_vector,
+                            Handle<BytecodeArray> bytecode_array,
+                            Handle<SharedFunctionInfo> shared,
+                            Handle<FeedbackVector> feedback_vector,
                             BailoutId osr_offset, JSGraph* jsgraph,
                             CallFrequency const& invocation_frequency,
                             SourcePositionTable* source_positions,

src/compiler/heap-refs.h

@@ -493,12 +493,10 @@ 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,21 +1262,17 @@ 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;
 
- private:
-  double const invocation_count_;
-  double const total_profiler_ticks_;
+  void SerializeSlots(JSHeapBroker* broker);
 
+ private:
   bool serialized_ = false;
   ZoneVector<ObjectData*> feedback_;
   ZoneVector<ObjectData*> closure_feedback_cell_array_;
@@ -1286,8 +1282,6 @@ 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()) {}
 
@@ -1306,11 +1300,11 @@ FeedbackCellData* FeedbackVectorData::GetClosureFeedbackCell(
   return closure_feedback_cell_array_[index]->AsFeedbackCell();
 }
 
-void FeedbackVectorData::Serialize(JSHeapBroker* broker) {
+void FeedbackVectorData::SerializeSlots(JSHeapBroker* broker) {
   if (serialized_) return;
   serialized_ = true;
 
-  TraceScope tracer(broker, this, "FeedbackVectorData::Serialize");
+  TraceScope tracer(broker, this, "FeedbackVectorData::SerializeSlots");
   Handle<FeedbackVector> vector = Handle<FeedbackVector>::cast(object());
   DCHECK(feedback_.empty());
   feedback_.reserve(vector->length());
@@ -3158,9 +3152,6 @@ 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)
@@ -3727,8 +3718,8 @@ Float64 FixedDoubleArrayData::Get(int i) const {
   return contents_[i];
 }
 
-void FeedbackVectorRef::Serialize() {
-  data()->AsFeedbackVector()->Serialize(broker());
+void FeedbackVectorRef::SerializeSlots() {
+  data()->AsFeedbackVector()->SerializeSlots(broker());
 }
 
 bool NameRef::IsUniqueName() const {
@@ -3905,6 +3896,7 @@ 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).Serialize();
+        FeedbackVectorRef(broker(), p.feedback().vector).SerializeSlots();
       }
       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).Serialize();
+        FeedbackVectorRef(broker(), p.feedback().vector).SerializeSlots();
       }
       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).Serialize();
+        FeedbackVectorRef(broker(), p.feedback().vector).SerializeSlots();
       }
       break;
     }

src/compiler/js-inlining.cc

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

src/compiler/pipeline.cc

@@ -992,6 +992,11 @@ 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
@@ -999,11 +1004,6 @@ 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;
     }
-
-    JSFunctionRef closure(data->broker(), data->info()->closure());
-    double invocation_count = closure.feedback_vector().invocation_count();
-    double total_ticks = closure.feedback_vector().total_profiler_ticks();
+    double invocation_count =
+        data->info()->closure()->feedback_vector().invocation_count();
+    double total_ticks =
+        data->info()->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,9 +1212,10 @@ struct GraphBuilderPhase {
     }
     double executed_bytecode_bytes = total_ticks * FLAG_interrupt_budget;
     CallFrequency frequency(invocation_count / (executed_bytecode_bytes / KB));
-
     BuildGraphFromBytecode(
-        data->broker(), temp_zone, closure.shared(), closure.feedback_vector(),
+        data->broker(), temp_zone, data->info()->bytecode_array(),
+        data->info()->shared_info(),
+        handle(data->info()->closure()->feedback_vector(), data->isolate()),
         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.Serialize();
+  feedback_vector_ref.SerializeSlots();
   TraverseBytecode();
   return environment()->return_value_hints();
 }