[ignition] Single-switch generator bytecode

Currently, yields and awaits inside loops compile to bytecode which
switches to the top of the loop header, and switch again once inside the
loop. This is to make loops reducible.

This replaces this switching logic with a single switch bytecode that
directly jumps to the bytecode being resumed. Among other things, this
allows us to no longer maintain the generator state after the switch at
the top of the function, and avoid having to track loop suspend counts.

TurboFan still needs to have reducible loops, so we now insert loop
header switches during bytecode graph building, for suspends that are
discovered to be inside loops during bytecode analysis. We do, however,
do some environment magic across loop headers since we know that we will
continue switching if and only if we reached that loop header via a
generator resume. This allows us to generate fewer phis and tighten
liveness.

Change-Id: Id2720ce1d6955be9a48178322cc209b3a4b8d385
Reviewed-on: https://chromium-review.googlesource.com/866734
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50804}

src/compiler/bytecode-analysis.h

@@ -39,15 +39,49 @@ class V8_EXPORT_PRIVATE BytecodeLoopAssignments {
   BitVector* bit_vector_;
 };
 
+// Jump targets for resuming a suspended generator.
+class V8_EXPORT_PRIVATE ResumeJumpTarget {
+ public:
+  // Create a resume jump target representing an actual resume.
+  static ResumeJumpTarget Leaf(int suspend_id, int target_offset);
+
+  // Create a resume jump target at a loop header, which will have another
+  // resume jump after the loop header is crossed.
+  static ResumeJumpTarget AtLoopHeader(int loop_header_offset,
+                                       const ResumeJumpTarget& next);
+
+  int suspend_id() const { return suspend_id_; }
+  int target_offset() const { return target_offset_; }
+  bool is_leaf() const { return target_offset_ == final_target_offset_; }
+
+ private:
+  // The suspend id of the resume.
+  int suspend_id_;
+  // The target offset of this resume jump.
+  int target_offset_;
+  // The final offset of this resume, which may be across multiple jumps.
+  int final_target_offset_;
+
+  ResumeJumpTarget(int suspend_id, int target_offset, int final_target_offset);
+};
+
 struct V8_EXPORT_PRIVATE LoopInfo {
  public:
   LoopInfo(int parent_offset, int parameter_count, int register_count,
            Zone* zone)
       : parent_offset_(parent_offset),
-        assignments_(parameter_count, register_count, zone) {}
+        assignments_(parameter_count, register_count, zone),
+        resume_jump_targets_(zone) {}
 
   int parent_offset() const { return parent_offset_; }
 
+  const ZoneVector<ResumeJumpTarget>& resume_jump_targets() const {
+    return resume_jump_targets_;
+  }
+  void AddResumeTarget(const ResumeJumpTarget& target) {
+    resume_jump_targets_.push_back(target);
+  }
+
   BytecodeLoopAssignments& assignments() { return assignments_; }
   const BytecodeLoopAssignments& assignments() const { return assignments_; }
 
@@ -55,6 +89,7 @@ struct V8_EXPORT_PRIVATE LoopInfo {
   // The offset to the parent loop, or -1 if there is no parent.
   int parent_offset_;
   BytecodeLoopAssignments assignments_;
+  ZoneVector<ResumeJumpTarget> resume_jump_targets_;
 };
 
 class V8_EXPORT_PRIVATE BytecodeAnalysis BASE_EMBEDDED {
@@ -78,10 +113,16 @@ class V8_EXPORT_PRIVATE BytecodeAnalysis BASE_EMBEDDED {
   // Get the loop info of the loop header at {header_offset}.
   const LoopInfo& GetLoopInfoFor(int header_offset) const;
 
+  // Get the top-level resume jump targets.
+  const ZoneVector<ResumeJumpTarget>& resume_jump_targets() const {
+    return resume_jump_targets_;
+  }
+
   // True if the current analysis has an OSR entry point.
   bool HasOsrEntryPoint() const { return osr_entry_point_ != -1; }
 
   int osr_entry_point() const { return osr_entry_point_; }
+
   // Gets the in-liveness for the bytecode at {offset}.
   const BytecodeLivenessState* GetInLivenessFor(int offset) const;
 
@@ -99,6 +140,12 @@ class V8_EXPORT_PRIVATE BytecodeAnalysis BASE_EMBEDDED {
   void PushLoop(int loop_header, int loop_end);
 
 #if DEBUG
+  bool ResumeJumpTargetsAreValid();
+  bool ResumeJumpTargetLeavesResolveSuspendIds(
+      int parent_offset,
+      const ZoneVector<ResumeJumpTarget>& resume_jump_targets,
+      std::map<int, int>* unresolved_suspend_ids);
+
   bool LivenessIsValid();
 #endif
 
@@ -112,6 +159,7 @@ class V8_EXPORT_PRIVATE BytecodeAnalysis BASE_EMBEDDED {
 
   ZoneStack<LoopStackEntry> loop_stack_;
   ZoneVector<int> loop_end_index_queue_;
+  ZoneVector<ResumeJumpTarget> resume_jump_targets_;
 
   ZoneMap<int, int> end_to_header_;
   ZoneMap<int, LoopInfo> header_to_info_;

src/compiler/bytecode-graph-builder.h

@@ -252,6 +252,9 @@ class BytecodeGraphBuilder {
   void BuildJumpIfJSReceiver();
 
   void BuildSwitchOnSmi(Node* condition);
+  void BuildSwitchOnGeneratorState(
+      const ZoneVector<ResumeJumpTarget>& resume_jump_targets,
+      bool allow_fallthrough_on_executing);
 
   // Simulates control flow by forward-propagating environments.
   void MergeIntoSuccessorEnvironment(int target_offset);
@@ -382,9 +385,18 @@ class BytecodeGraphBuilder {
 
   // Merge environments are snapshots of the environment at points where the
   // control flow merges. This models a forward data flow propagation of all
-  // values from all predecessors of the merge in question.
+  // values from all predecessors of the merge in question. They are indexed by
+  // the bytecode offset
   ZoneMap<int, Environment*> merge_environments_;
 
+  // Generator merge environments are snapshots of the current resume
+  // environment, tracing back through loop headers to the resume switch of a
+  // generator. They allow us to model a single resume jump as several switch
+  // statements across loop headers, keeping those loop headers reducible,
+  // without having to merge the "executing" environments of the generator into
+  // the "resuming" ones. They are indexed by the suspend id of the resume.
+  ZoneMap<int, Environment*> generator_merge_environments_;
+
   // Exception handlers currently entered by the iteration.
   ZoneStack<ExceptionHandler> exception_handlers_;
   int current_exception_handler_;

src/compiler/js-operator.h

@@ -742,7 +742,7 @@ class V8_EXPORT_PRIVATE JSOperatorBuilder final
   // Used to implement Ignition's SuspendGenerator bytecode.
   const Operator* GeneratorStore(int register_count);
 
-  // Used to implement Ignition's RestoreGeneratorState bytecode.
+  // Used to implement Ignition's SwitchOnGeneratorState bytecode.
   const Operator* GeneratorRestoreContinuation();
   // Used to implement Ignition's ResumeGenerator bytecode.
   const Operator* GeneratorRestoreRegister(int index);

src/interpreter/bytecode-array-accessor.cc

@@ -206,12 +206,18 @@ int BytecodeArrayAccessor::GetJumpTargetOffset() const {
 
 JumpTableTargetOffsets BytecodeArrayAccessor::GetJumpTableTargetOffsets()
     const {
-  DCHECK_EQ(current_bytecode(), Bytecode::kSwitchOnSmiNoFeedback);
-
-  uint32_t table_start = GetIndexOperand(0);
-  uint32_t table_size = GetUnsignedImmediateOperand(1);
-  int32_t case_value_base = GetImmediateOperand(2);
-
+  uint32_t table_start, table_size;
+  int32_t case_value_base;
+  if (current_bytecode() == Bytecode::kSwitchOnGeneratorState) {
+    table_start = GetIndexOperand(1);
+    table_size = GetUnsignedImmediateOperand(2);
+    case_value_base = 0;
+  } else {
+    DCHECK_EQ(current_bytecode(), Bytecode::kSwitchOnSmiNoFeedback);
+    table_start = GetIndexOperand(0);
+    table_size = GetUnsignedImmediateOperand(1);
+    case_value_base = GetImmediateOperand(2);
+  }
   return JumpTableTargetOffsets(this, table_start, table_size, case_value_base);
 }
 

src/interpreter/bytecode-array-builder.cc

@@ -1271,16 +1271,19 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::SuspendGenerator(
   return *this;
 }
 
-BytecodeArrayBuilder& BytecodeArrayBuilder::RestoreGeneratorState(
-    Register generator) {
-  OutputRestoreGeneratorState(generator);
+BytecodeArrayBuilder& BytecodeArrayBuilder::SwitchOnGeneratorState(
+    Register generator, BytecodeJumpTable* jump_table) {
+  DCHECK_EQ(jump_table->case_value_base(), 0);
+  BytecodeNode node(CreateSwitchOnGeneratorStateNode(
+      generator, jump_table->constant_pool_index(), jump_table->size()));
+  WriteSwitch(&node, jump_table);
+  LeaveBasicBlock();
   return *this;
 }
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::ResumeGenerator(
-    Register generator, Register generator_state, RegisterList registers) {
-  OutputResumeGenerator(generator, generator_state, registers,
-                        registers.register_count());
+    Register generator, RegisterList registers) {
+  OutputResumeGenerator(generator, registers, registers.register_count());
   return *this;
 }
 

src/interpreter/bytecode-array-builder.h

@@ -430,9 +430,9 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final {
   BytecodeArrayBuilder& SuspendGenerator(Register generator,
                                          RegisterList registers,
                                          int suspend_id);
-  BytecodeArrayBuilder& RestoreGeneratorState(Register generator);
+  BytecodeArrayBuilder& SwitchOnGeneratorState(Register generator,
+                                               BytecodeJumpTable* jump_table);
   BytecodeArrayBuilder& ResumeGenerator(Register generator,
-                                        Register generator_state,
                                         RegisterList registers);
 
   // Exception handling.

src/interpreter/bytecode-array-writer.cc

@@ -159,6 +159,7 @@ void BytecodeArrayWriter::UpdateExitSeenInBlock(Bytecode bytecode) {
     case Bytecode::kJump:
     case Bytecode::kJumpConstant:
     case Bytecode::kSuspendGenerator:
+    case Bytecode::kSwitchOnGeneratorState:
       exit_seen_in_block_ = true;
       break;
     default:

src/interpreter/bytecode-generator.cc

@@ -878,7 +878,6 @@ BytecodeGenerator::BytecodeGenerator(
       execution_result_(nullptr),
       incoming_new_target_or_generator_(),
       generator_jump_table_(nullptr),
-      generator_state_(),
       loop_depth_(0),
       catch_prediction_(HandlerTable::UNCAUGHT) {
   DCHECK_EQ(closure_scope(), closure_scope()->GetClosureScope());
@@ -1091,8 +1090,6 @@ void BytecodeGenerator::GenerateBytecodeBody() {
 
 void BytecodeGenerator::AllocateTopLevelRegisters() {
   if (info()->literal()->CanSuspend()) {
-    // Allocate a register for generator_state_.
-    generator_state_ = register_allocator()->NewRegister();
     // Either directly use generator_object_var or allocate a new register for
     // the incoming generator object.
     Variable* generator_object_var = closure_scope()->generator_object_var();
@@ -1124,39 +1121,11 @@ void BytecodeGenerator::VisitIterationHeader(IterationStatement* stmt,
 void BytecodeGenerator::VisitIterationHeader(int first_suspend_id,
                                              int suspend_count,
                                              LoopBuilder* loop_builder) {
-  // Recall that suspend_count is always zero inside ordinary (i.e.
-  // non-generator) functions.
-  if (suspend_count == 0) {
-    loop_builder->LoopHeader();
-  } else {
-    loop_builder->LoopHeaderInGenerator(&generator_jump_table_,
-                                        first_suspend_id, suspend_count);
-
-    // Perform state dispatch on the generator state, assuming this is a resume.
-    builder()
-        ->LoadAccumulatorWithRegister(generator_state_)
-        .SwitchOnSmiNoFeedback(generator_jump_table_);
-
-    // We fall through when the generator state is not in the jump table. If we
-    // are not resuming, we want to fall through to the loop body.
-    // TODO(leszeks): Only generate this test for debug builds, we can skip it
-    // entirely in release assuming that the generator states is always valid.
-    BytecodeLabel not_resuming;
-    builder()
-        ->LoadLiteral(Smi::FromInt(JSGeneratorObject::kGeneratorExecuting))
-        .CompareOperation(Token::Value::EQ_STRICT, generator_state_)
-        .JumpIfTrue(ToBooleanMode::kAlreadyBoolean, &not_resuming);
-
-    // Otherwise this is an error.
-    builder()->Abort(AbortReason::kInvalidJumpTableIndex);
-
-    builder()->Bind(&not_resuming);
-  }
+  loop_builder->LoopHeader();
 }
 
 void BytecodeGenerator::BuildGeneratorPrologue() {
   DCHECK_GT(info()->literal()->suspend_count(), 0);
-  DCHECK(generator_state_.is_valid());
   DCHECK(generator_object().is_valid());
   generator_jump_table_ =
       builder()->AllocateJumpTable(info()->literal()->suspend_count(), 0);
@@ -1174,19 +1143,12 @@ void BytecodeGenerator::BuildGeneratorPrologue() {
     builder()
         ->CallRuntime(Runtime::kInlineGeneratorGetContext, generator_object())
         .PushContext(generator_context)
-        .RestoreGeneratorState(generator_object())
-        .StoreAccumulatorInRegister(generator_state_)
-        .SwitchOnSmiNoFeedback(generator_jump_table_);
+        .SwitchOnGeneratorState(generator_object(), generator_jump_table_);
+    // The switch is guaranteed to jump (or abort), so there is no fall-through.
   }
-  // We fall through when the generator state is not in the jump table.
-  // TODO(leszeks): Only generate this for debug builds.
-  builder()->Abort(AbortReason::kInvalidJumpTableIndex);
 
   // This is a regular call.
-  builder()
-      ->Bind(&regular_call)
-      .LoadLiteral(Smi::FromInt(JSGeneratorObject::kGeneratorExecuting))
-      .StoreAccumulatorInRegister(generator_state_);
+  builder()->Bind(&regular_call);
   // Now fall through to the ordinary function prologue, after which we will run
   // into the generator object creation and other extra code inserted by the
   // parser.
@@ -2878,10 +2840,9 @@ void BytecodeGenerator::BuildSuspendPoint(int suspend_id,
   // Upon resume, we continue here.
   builder()->Bind(generator_jump_table_, suspend_id);
 
-  // Clobbers all registers, updating the state to indicate that we have
-  // finished resuming and setting the accumulator to the [[input_or_debug_pos]]
-  // slot of the generator object.
-  builder()->ResumeGenerator(generator_object(), generator_state_, registers);
+  // Clobbers all registers and sets the accumulator to the
+  // [[input_or_debug_pos]] slot of the generator object.
+  builder()->ResumeGenerator(generator_object(), registers);
 }
 
 void BytecodeGenerator::VisitYield(Yield* expr) {

src/interpreter/bytecode-generator.h

@@ -345,7 +345,6 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   Register incoming_new_target_or_generator_;
 
   BytecodeJumpTable* generator_jump_table_;
-  Register generator_state_;
   int loop_depth_;
 
   HandlerTable::CatchPrediction catch_prediction_;

src/interpreter/bytecodes.h

@@ -314,11 +314,12 @@ namespace interpreter {
   V(ThrowSuperAlreadyCalledIfNotHole, AccumulatorUse::kRead)                   \
                                                                                \
   /* Generators */                                                             \
-  V(RestoreGeneratorState, AccumulatorUse::kWrite, OperandType::kReg)          \
+  V(SwitchOnGeneratorState, AccumulatorUse::kNone, OperandType::kReg,          \
+    OperandType::kIdx, OperandType::kUImm)                                     \
   V(SuspendGenerator, AccumulatorUse::kRead, OperandType::kReg,                \
     OperandType::kRegList, OperandType::kRegCount, OperandType::kUImm)         \
   V(ResumeGenerator, AccumulatorUse::kWrite, OperandType::kReg,                \
-    OperandType::kRegOut, OperandType::kRegOutList, OperandType::kRegCount)    \
+    OperandType::kRegOutList, OperandType::kRegCount)                          \
                                                                                \
   /* Debugger */                                                               \
   V(Debugger, AccumulatorUse::kNone)                                           \
@@ -626,7 +627,8 @@ class V8_EXPORT_PRIVATE Bytecodes final : public AllStatic {
 
   // Returns true if the bytecode is a switch.
   static constexpr bool IsSwitch(Bytecode bytecode) {
-    return bytecode == Bytecode::kSwitchOnSmiNoFeedback;
+    return bytecode == Bytecode::kSwitchOnSmiNoFeedback ||
+           bytecode == Bytecode::kSwitchOnGeneratorState;
   }
 
   // Returns true if |bytecode| has no effects. These bytecodes only manipulate

src/interpreter/interpreter-generator.cc

@@ -3108,38 +3108,53 @@ IGNITION_HANDLER(SuspendGenerator, InterpreterAssembler) {
   Return(GetAccumulator());
 }
 
-// RestoreGeneratorState <generator>
+// SwitchOnGeneratorState <generator> <table_start> <table_length>
 //
-// Loads the generator's state and stores it in the accumulator,
-// before overwriting it with kGeneratorExecuting.
-IGNITION_HANDLER(RestoreGeneratorState, InterpreterAssembler) {
+// Loads the |generator|'s state and stores it in the accumulator, before
+// overwriting it with kGeneratorExecuting. Then, jumps to the appropriate
+// resume bytecode, by looking up the generator state in a jump table in the
+// constant pool, starting at |table_start|, and of length |table_length|.
+IGNITION_HANDLER(SwitchOnGeneratorState, InterpreterAssembler) {
   Node* generator_reg = BytecodeOperandReg(0);
   Node* generator = LoadRegister(generator_reg);
 
-  Node* old_state =
+  Node* state =
       LoadObjectField(generator, JSGeneratorObject::kContinuationOffset);
-  Node* new_state = Int32Constant(JSGeneratorObject::kGeneratorExecuting);
+  Node* new_state = SmiConstant(JSGeneratorObject::kGeneratorExecuting);
   StoreObjectField(generator, JSGeneratorObject::kContinuationOffset,
-                   SmiTag(new_state));
-  SetAccumulator(old_state);
+                   new_state);
 
-  Dispatch();
+  Node* table_start = BytecodeOperandIdx(1);
+  // TODO(leszeks): table_length is only used for a CSA_ASSERT, we don't
+  // actually need it otherwise.
+  Node* table_length = BytecodeOperandUImmWord(2);
+
+  // The state must be a Smi.
+  CSA_ASSERT(this, TaggedIsSmi(state));
+
+  Node* case_value = SmiUntag(state);
+
+  CSA_ASSERT(this, IntPtrGreaterThanOrEqual(case_value, IntPtrConstant(0)));
+  CSA_ASSERT(this, IntPtrLessThan(case_value, table_length));
+  USE(table_length);
+
+  Node* entry = IntPtrAdd(table_start, case_value);
+  Node* relative_jump = LoadAndUntagConstantPoolEntry(entry);
+  Jump(relative_jump);
 }
 
-// ResumeGenerator <generator> <generator_state> <first output
-// register> <register count>
+// ResumeGenerator <generator> <first output register> <register count>
 //
 // Imports the register file stored in the generator and marks the generator
 // state as executing.
 IGNITION_HANDLER(ResumeGenerator, InterpreterAssembler) {
   Node* generator_reg = BytecodeOperandReg(0);
-  Node* generator_state_reg = BytecodeOperandReg(1);
-  // Bytecode operand 2 is the start register. It should always be 0, so let's
+  // Bytecode operand 1 is the start register. It should always be 0, so let's
   // ignore it.
-  CSA_ASSERT(this, WordEqual(BytecodeOperandReg(2),
+  CSA_ASSERT(this, WordEqual(BytecodeOperandReg(1),
                              IntPtrConstant(Register(0).ToOperand())));
-  // Bytecode operand 3 is the number of registers to store to the generator.
-  Node* register_count = ChangeUint32ToWord(BytecodeOperandCount(3));
+  // Bytecode operand 2 is the number of registers to store to the generator.
+  Node* register_count = ChangeUint32ToWord(BytecodeOperandCount(2));
 
   Node* generator = LoadRegister(generator_reg);
 
@@ -3147,11 +3162,6 @@ IGNITION_HANDLER(ResumeGenerator, InterpreterAssembler) {
       LoadObjectField(generator, JSGeneratorObject::kRegisterFileOffset),
       register_count);
 
-  // Since we're resuming, update the generator state to indicate that the
-  // generator is now executing.
-  StoreRegister(SmiConstant(JSGeneratorObject::kGeneratorExecuting),
-                generator_state_reg);
-
   // Return the generator's input_or_debug_pos in the accumulator.
   SetAccumulator(
       LoadObjectField(generator, JSGeneratorObject::kInputOrDebugPosOffset));

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -391,9 +391,10 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
     builder.JumpIfTrue(ToBooleanMode::kAlreadyBoolean, &after_suspend)
         .SuspendGenerator(reg, reg_list, 0)
         .Bind(&after_suspend)
-        .RestoreGeneratorState(reg)
-        .ResumeGenerator(reg, reg, reg_list);
+        .ResumeGenerator(reg, reg_list);
   }
+  BytecodeJumpTable* gen_jump_table = builder.AllocateJumpTable(1, 0);
+  builder.SwitchOnGeneratorState(reg, gen_jump_table).Bind(gen_jump_table, 0);
 
   // Intrinsics handled by the interpreter.
   builder.CallRuntime(Runtime::kInlineIsArray, reg_list);
@@ -445,6 +446,7 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
       operand_scale = Bytecodes::PrefixBytecodeToOperandScale(final_bytecode);
       prefix_offset = 1;
       code = the_array->get(i + 1);
+      scorecard[code] += 1;
       final_bytecode = Bytecodes::FromByte(code);
     }
     i += prefix_offset + Bytecodes::Size(final_bytecode, operand_scale);
@@ -464,7 +466,7 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
 #define CHECK_BYTECODE_PRESENT(Name, ...)                                \
   /* Check Bytecode is marked in scorecard, unless it's a debug break */ \
   if (!Bytecodes::IsDebugBreak(Bytecode::k##Name)) {                     \
-    CHECK_GE(scorecard[Bytecodes::ToByte(Bytecode::k##Name)], 1);        \
+    EXPECT_GE(scorecard[Bytecodes::ToByte(Bytecode::k##Name)], 1);       \
   }
   BYTECODE_LIST(CHECK_BYTECODE_PRESENT)
 #undef CHECK_BYTECODE_PRESENT