[ignition] desugar async functions/generators/modules in BytecodeGenerator

src/interpreter/bytecode-generator.cc

Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index c28d4687d8a8cf5b742626739d8cbeb50ee9dc54..916a965855ccf0ad5d7182201b6c06cbcfecd853 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -199,7 +199,7 @@ class BytecodeGenerator::ControlScope::DeferredCommands final {
   ControlScope* execution_control() { return generator_->execution_control(); }
 
  private:
-  BytecodeGenerator* generator_;
+  BytecodeGenerator* generator_ = nullptr;

[rmcilroy, 2017/02/07 17:24:41]

Why was this change made? The constructor should set generator shouldn't it?

   ZoneVector<Entry> deferred_;
   Register token_register_;
   Register result_register_;
@@ -580,6 +580,217 @@ class BytecodeGenerator::GlobalDeclarationsBuilder final : public ZoneObject {
   bool has_constant_pool_entry_;
 };
 
+template <class T>
+class TryBlockBuilder;
+class BytecodeGenerator::SimpleTryCatchBuilder : private TryCatchBuilder {
+ public:
+  typedef ControlScopeForTryCatch TryControlScope;
+
+  explicit SimpleTryCatchBuilder(BytecodeGenerator* generator,
+                                 HandlerTable::CatchPrediction catch_prediction)
+      : TryCatchBuilder(generator->builder(), catch_prediction),
+        generator_(generator) {}
+
+  Register context() const {
+    DCHECK(context_.is_valid());
+    return context_;
+  }
+
+ private:
+  friend class TryBlockBuilder<SimpleTryCatchBuilder>;
+  friend class CatchBlockBuilder;
+
+  void BeginTry(ControlScopeForTryCatch* scope) {
+    DCHECK_NULL(control_scope_);
+    control_scope_ = scope;
+
+    // Preserve the context in a dedicated register, so that it can be restored
+    // when the handler is entered by the stack-unwinding machinery.
+    // TODO(mstarzinger): Be smarter about register allocation.
+    DCHECK(!context_.is_valid());
+    context_ = generator_->register_allocator()->NewRegister();
+    builder()->MoveRegister(Register::current_context(), context_);
+
+    TryCatchBuilder::BeginTry(context_);
+    last_catch_prediction_ = generator_->catch_prediction_;
+    if (catch_prediction() != HandlerTable::UNCAUGHT) {
+      generator_->catch_prediction_ = catch_prediction();
+    }

[rmcilroy, 2017/02/07 17:24:41]

I'm not sure why we need to update the catch prediction on the generator here -
what changed that means we need to keep track of this on the generator? Can't we
just keep track of the current handler's catch prediction?

+  }
+
+  void EndTry() {
+    DCHECK_NOT_NULL(control_scope_);
+    generator_->catch_prediction_ = last_catch_prediction_;
+    last_catch_prediction_ = HandlerTable::UNCAUGHT;
+
+    TryCatchBuilder::EndTry();
+    control_scope_ = nullptr;
+  }
+
+  void BeginCatch() {
+    DCHECK_NULL(control_scope_);
+    DCHECK(context_.is_valid());
+  }
+
+  void EndCatch() { TryCatchBuilder::EndCatch(); }
+
+  BytecodeGenerator* generator_;
+  Register context_;
+  ControlScopeForTryCatch* control_scope_ = nullptr;
+  HandlerTable::CatchPrediction last_catch_prediction_;
+};
+
+class BytecodeGenerator::SimpleTryFinallyBuilder : public TryFinallyBuilder {

[rmcilroy, 2017/02/07 17:24:41]

I'm not sure on the naming, why is this SimpleTryFinallyBuilder (is there a more
complex version?). Also I'd prefer not to have to extend the TryFinallyBuilder
in control-flow-builders given that they are only used by the code here. Would
it be possible to move the logic into the control-flow-builder variant and only
have a single TryCatch/FinallyBuilder? I think this would be easier if the
comment regarding catch prediction above is addressed since then you should only
need to pass a register to the builder and don't need the bytecode generator.

+ public:
+  typedef ControlScopeForTryFinally TryControlScope;
+
+  explicit SimpleTryFinallyBuilder(BytecodeGenerator* generator)
+      : TryFinallyBuilder(generator->builder(), generator->catch_prediction_),
+        generator_(generator),
+        token_(generator->register_allocator()->NewRegister()),
+        result_(generator->register_allocator()->NewRegister()),
+        commands_(generator, token_, result_) {
+    // We keep a record of all paths that enter the finally-block to be able to
+    // dispatch to the correct continuation point after the statements in the
+    // finally-block have been evaluated.
+    //
+    // The try-finally construct can enter the finally-block in three ways:
+    // 1. By exiting the try-block normally, falling through at the end.
+    // 2. By exiting the try-block with a function-local control flow transfer
+    //    (i.e. through break/continue/return statements).
+    // 3. By exiting the try-block with a thrown exception.
+    //
+    // The result register semantics depend on how the block was entered:
+    //  - ReturnStatement: It represents the return value being returned.
+    //  - ThrowStatement: It represents the exception being thrown.
+    //  - BreakStatement/ContinueStatement: Undefined and not used.
+    //  - Falling through into finally-block: Undefined and not used.
+  }
+
+ private:
+  friend class TryBlockBuilder<SimpleTryFinallyBuilder>;
+  friend class FinallyBlockBuilder;
+
+  void BeginTry(ControlScopeForTryFinally* control_scope) {
+    DCHECK_NULL(control_scope_);
+    control_scope_ = control_scope;
+
+    // Preserve the context in a dedicated register, so that it can be restored
+    // when the handler is entered by the stack-unwinding machinery.
+    // TODO(mstarzinger): Be smarter about register allocation.
+    DCHECK(!context_or_message_.is_valid());
+    context_or_message_ = generator_->register_allocator()->NewRegister();
+    generator_->builder()->MoveRegister(Register::current_context(),
+                                        context_or_message_);
+
+    TryFinallyBuilder::BeginTry(context_or_message_);
+  }
+
+  void EndTry() {
+    DCHECK_NOT_NULL(control_scope_);
+    TryFinallyBuilder::EndTry();
+    control_scope_ = nullptr;
+
+    // Record fall-through and exception cases.
+    commands_.RecordFallThroughPath();
+    TryFinallyBuilder::LeaveTry();
+    TryFinallyBuilder::BeginHandler();
+    commands_.RecordHandlerReThrowPath();
+  }
+
+  void BeginFinally() {
+    TryFinallyBuilder::BeginFinally();
+    generator_->builder()
+        ->LoadTheHole()
+        .SetPendingMessage()
+        .StoreAccumulatorInRegister(context_or_message_);
+  }
+
+  void EndFinally() {
+    TryFinallyBuilder::EndFinally();
+
+    // Pending message object is restored on exit.
+    generator_->builder()
+        ->LoadAccumulatorWithRegister(context_or_message_)
+        .SetPendingMessage();
+
+    // Dynamic dispatch after the finally-block.
+    commands_.ApplyDeferredCommands();
+  }
+
+  ControlScopeForTryFinally* control_scope_ = nullptr;
+  BytecodeGenerator* generator_;
+  Register token_;
+  Register result_;
+  ControlScope::DeferredCommands commands_;
+  Register context_or_message_;
+};
+
+template <class T>
+class BytecodeGenerator::TryBlockBuilder {

[rmcilroy, 2017/02/07 17:24:41]

I don't think the scoped builders are necessary, I think it would be simpler and
clearer just to explicitly have BeginTry / LeaveTry / BeginHandler /
BeginFinally / EndFinally etc. (and maybe some asserts in each to ensure that
they have all been called when the builder is destroyed).

+ public:
+  typedef T SimpleControlBuilder;
+  explicit TryBlockBuilder(SimpleControlBuilder& builder);
+  ~TryBlockBuilder();
+
+ private:
+  SimpleControlBuilder* builder_;
+  typename SimpleControlBuilder::TryControlScope scope_;
+};
+
+template <>
+BytecodeGenerator::TryBlockBuilder<BytecodeGenerator::SimpleTryCatchBuilder>::
+    TryBlockBuilder(SimpleTryCatchBuilder& builder)
+    : builder_(&builder), scope_(builder_->generator_, builder_) {
+  builder_->BeginTry(&scope_);
+}
+
+template <>
+BytecodeGenerator::TryBlockBuilder<
+    BytecodeGenerator::SimpleTryCatchBuilder>::~TryBlockBuilder() {
+  builder_->EndTry();
+}
+
+template <>
+BytecodeGenerator::TryBlockBuilder<BytecodeGenerator::SimpleTryFinallyBuilder>::
+    TryBlockBuilder(SimpleTryFinallyBuilder& builder)
+    : builder_(&builder),
+      scope_(builder_->generator_, builder_, &builder_->commands_) {
+  builder_->BeginTry(&scope_);
+}
+
+template <>
+BytecodeGenerator::TryBlockBuilder<
+    BytecodeGenerator::SimpleTryFinallyBuilder>::~TryBlockBuilder() {
+  builder_->EndTry();
+}
+
+class BytecodeGenerator::CatchBlockBuilder {
+ public:
+  explicit CatchBlockBuilder(SimpleTryCatchBuilder& builder)
+      : builder_(&builder) {
+    builder_->BeginCatch();
+  }
+
+  ~CatchBlockBuilder() { builder_->EndCatch(); }
+
+ private:
+  SimpleTryCatchBuilder* builder_;
+};
+
+class BytecodeGenerator::FinallyBlockBuilder {
+ public:
+  explicit FinallyBlockBuilder(SimpleTryFinallyBuilder& builder)
+      : builder_(&builder) {
+    builder_->BeginFinally();
+  }
+
+  ~FinallyBlockBuilder() { builder_->EndFinally(); }
+
+ private:
+  SimpleTryFinallyBuilder* builder_;
+};
+
 BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
     : zone_(info->zone()),
       builder_(new (zone()) BytecodeArrayBuilder(
@@ -601,6 +812,7 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
       generator_resume_points_(info->literal()->yield_count(), info->zone()),
       generator_state_(),
       loop_depth_(0),
+      catch_prediction_(HandlerTable::UNCAUGHT),
       home_object_symbol_(info->isolate()->factory()->home_object_symbol()),
       iterator_symbol_(info->isolate()->factory()->iterator_symbol()),
       prototype_string_(info->isolate()->factory()->prototype_string()),
@@ -696,14 +908,6 @@ void BytecodeGenerator::GenerateBytecode(uintptr_t stack_limit) {
     GenerateBytecodeBody();
   }
 
-  // In generator functions, we may not have visited every yield in the AST
-  // since we skip some obviously dead code. Hence the generated bytecode may
-  // contain jumps to unbound labels (resume points that will never be used).
-  // We bind these now.
-  for (auto& label : generator_resume_points_) {
-    if (!label.is_bound()) builder()->Bind(&label);
-  }
-
   // Emit an implicit return instruction in case control flow can fall off the
   // end of the function without an explicit return being present on all paths.
   if (builder()->RequiresImplicitReturn()) {
@@ -736,11 +940,234 @@ void BytecodeGenerator::GenerateBytecodeBody() {
   // Emit initializing assignments for module namespace imports (if any).
   VisitModuleNamespaceImports();
 
+  FunctionLiteral* literal = info()->literal();
+
   // Perform a stack-check before the body.
-  builder()->StackCheck(info()->literal()->start_position());
+  builder()->StackCheck(literal->start_position());
+
+  // Build assignment to variable <function name> if function is a named
+  // expression and the variable is used.
+  if (literal->is_named_expression()) {
+    Variable* function_var = scope()->function_var();
+    if (function_var != nullptr && !function_var->IsUnallocated()) {
+      builder()->LoadAccumulatorWithRegister(Register::function_closure());
+      BuildVariableAssignment(function_var, Token::INIT,
+                              FeedbackVectorSlot::Invalid(),
+                              HoleCheckMode::kElided);
+    }
+  }
 
+  if (IsAsyncFunction(literal->kind())) {
+    return GenerateBytecodeBodyForAsyncFunction(literal);
+  }
+  if (IsGeneratorFunction(literal->kind())) {
+    return GenerateBytecodeBodyForGenerator(literal);
+  }
+  if (IsModule(literal->kind())) {
+    return GenerateBytecodeBodyForModule(literal);
+  }
+
+  Block* parameter_init_block = literal->parameter_init_block();
   // Visit statements in the function body.
-  VisitStatements(info()->literal()->body());
+  if (parameter_init_block != nullptr) {
+    VisitBlock(parameter_init_block);
+  }
+  VisitStatements(literal->body());
+}
+
+void BytecodeGenerator::BuildAllocateAndStoreJSGeneratorObject(
+    FunctionLiteral* literal, BuildJSGeneratorObject tag) {
+  RegisterAllocationScope register_scope(this);
+  RegisterList args = register_allocator()->NewRegisterList(2);
+  builder()->MoveRegister(Register::function_closure(), args[0]);
+
+  int yield_position = literal->position();
+  builder()->SetExpressionPosition(yield_position);
+
+  if (IsArrowFunction(literal->kind())) {
+    // Lexical `this`
+    builder()->LoadUndefined().StoreAccumulatorInRegister(args[1]);
+  } else {
+    // Receiver parameter
+    builder()->MoveRegister(builder()->Parameter(0), args[1]);
+  }
+  builder()->CallRuntime(Runtime::kCreateJSGeneratorObject, args);
+
+  Variable* var_generator = scope()->generator_object_var();
+  DCHECK_NOT_NULL(var_generator);
+
+  switch (tag) {
+    case BuildJSGeneratorObject::kInitialYield: {
+      Register generator = args[0];
+      builder()->StoreAccumulatorInRegister(generator);
+      BuildVariableAssignment(var_generator, Token::INIT,
+                              FeedbackVectorSlot::Invalid(),
+                              HoleCheckMode::kElided);
+      BuildYield(0, generator, generator);
+      BuildYieldResumePoint(0, Yield::kOnExceptionThrow, generator,
+                            yield_position);
+      break;
+    }
+    case BuildJSGeneratorObject::kNoInitialYield: {
+      BuildVariableAssignment(var_generator, Token::INIT,
+                              FeedbackVectorSlot::Invalid(),
+                              HoleCheckMode::kElided);
+      break;
+    }
+  }
+}
+
+void BytecodeGenerator::BuildAllocateAndStoreJSPromise() {
+  Variable* var_promise = scope()->promise_var();
+  DCHECK_NOT_NULL(var_promise);
+
+  RegisterAllocationScope register_scope(this);
+  RegisterList args = register_allocator()->NewRegisterList(1);
+  builder()->LoadUndefined().StoreAccumulatorInRegister(args[0]).CallJSRuntime(
+      Context::ASYNC_FUNCTION_PROMISE_CREATE_INDEX, args);
+  BuildVariableAssignment(var_promise, Token::INIT,
+                          FeedbackVectorSlot::Invalid(),
+                          HoleCheckMode::kElided);
+}
+
+void BytecodeGenerator::BindUnboundGeneratorResumePoints() {
+  DCHECK(IsResumableFunction(info()->literal()->kind()));
+  for (auto& label : generator_resume_points_) {
+    if (!label.is_bound()) builder()->Bind(&label);
+  }
+}
+
+void BytecodeGenerator::GenerateBytecodeBodyForGenerator(
+    FunctionLiteral* literal) {
+  DCHECK(IsGeneratorFunction(literal->kind()));
+
+  // Desugar parameters before generator is allocated.
+  Block* const parameter_init_block = literal->parameter_init_block();
+  if (parameter_init_block != nullptr) {
+    // Initialize non-simple parameters before initial yield.
+    VisitBlock(parameter_init_block);
+  }
+
+  {
+    RegisterAllocationScope register_scope(this);
+    SimpleTryFinallyBuilder try_control_builder(this);
+    {  // Try ...
+      TryBlockBuilderForFinally try_builder(try_control_builder);
+      BuildAllocateAndStoreJSGeneratorObject(
+          literal, BuildJSGeneratorObject::kInitialYield);
+      VisitStatements(literal->body());
+    }
+
+    {  // Finally ...
+      FinallyBlockBuilder finally_builder(try_control_builder);
+      Variable* var_generator = scope()->generator_object_var();
+      DCHECK_NOT_NULL(var_generator);
+
+      RegisterAllocationScope register_scope(this);
+      BuildVariableLoad(var_generator, FeedbackVectorSlot::Invalid(),
+                        HoleCheckMode::kElided);
+      Register generator = register_allocator()->NewRegister();
+      builder()->StoreAccumulatorInRegister(generator).CallRuntime(
+          Runtime::kInlineGeneratorClose, generator);
+    }
+  }
+
+  // In generator functions, we may not have visited every yield in the AST
+  // since we skip some obviously dead code. Hence the generated bytecode may
+  // contain jumps to unbound labels (resume points that will never be used).
+  // We bind these now.
+  BindUnboundGeneratorResumePoints();
+
+  if (builder()->RequiresImplicitReturn()) {
+    // Unreachable, but required for BytecodeArrayBuilder
+    builder()->LoadUndefined().Return();
+  }
+  DCHECK(!builder()->RequiresImplicitReturn());
+}
+
+void BytecodeGenerator::GenerateBytecodeBodyForModule(
+    FunctionLiteral* literal) {
+  DCHECK(IsModule(literal->kind()));
+
+  // Modules do not have non-simple parameters
+  DCHECK_NULL(literal->parameter_init_block());
+
+  BuildAllocateAndStoreJSGeneratorObject(literal,
+                                         BuildJSGeneratorObject::kInitialYield);
+  VisitStatements(literal->body());
+
+  // In modules, we may not have visited every yield in the AST
+  // since we skip some obviously dead code. Hence the generated bytecode may
+  // contain jumps to unbound labels (resume points that will never be used).
+  // We bind these now.
+  BindUnboundGeneratorResumePoints();
+}
+
+void BytecodeGenerator::GenerateBytecodeBodyForAsyncFunction(
+    FunctionLiteral* literal) {
+  DCHECK(IsAsyncFunction(literal->kind()));
+
+  BuildAllocateAndStoreJSPromise();
+
+  {
+    RegisterAllocationScope register_scope(this);
+    SimpleTryCatchBuilder try_control_builder(this, HandlerTable::ASYNC_AWAIT);
+    {
+      TryBlockBuilderForCatch try_block(try_control_builder);
+
+      Block* const parameter_init_block = literal->parameter_init_block();
+      if (parameter_init_block != nullptr) {
+        // Initialize non-simple parameters if necessary, reject Promise in
+        // case of exception
+        RegisterAllocationScope register_scope(this);
+        VisitBlock(parameter_init_block);
+      }
+
+      BuildAllocateAndStoreJSGeneratorObject(
+          literal, BuildJSGeneratorObject::kNoInitialYield);
+
+      // Finish generating function body
+      VisitStatements(literal->body());
+    }
+
+    {
+      CatchBlockBuilder catch_block(try_control_builder);
+      RegisterList args = register_allocator()->NewRegisterList(4);
+      Register promise = args[1];
+      Register exception = args[2];
+
+      builder()
+          ->StoreAccumulatorInRegister(exception)
+          .LoadTheHole()
+          .SetPendingMessage();
+
+      Variable* var_promise = scope()->promise_var();
+      DCHECK_NOT_NULL(var_promise);
+      BuildVariableLoad(var_promise, FeedbackVectorSlot::Invalid(),
+                        HoleCheckMode::kElided);
+      builder()
+          ->StoreAccumulatorInRegister(promise)
+          .LoadUndefined()
+          .StoreAccumulatorInRegister(args[0])
+          .LoadFalse()
+          .StoreAccumulatorInRegister(args[3])
+          .CallJSRuntime(Context::PROMISE_INTERNAL_REJECT_INDEX, args)
+          .LoadAccumulatorWithRegister(promise)
+          .Return();
+    }
+  }
+
+  // In async functions, we may not have visited every yield in the AST
+  // since we skip some obviously dead code. Hence the generated bytecode may
+  // contain jumps to unbound labels (resume points that will never be used).
+  // We bind these now.
+  BindUnboundGeneratorResumePoints();
+
+  if (builder()->RequiresImplicitReturn()) {
+    // Unreachable, but required for BytecodeArrayBuilder
+    builder()->LoadUndefined().Return();
+  }
+  DCHECK(!builder()->RequiresImplicitReturn());
 }
 
 void BytecodeGenerator::BuildIndexedJump(Register index, size_t start_index,
@@ -1335,24 +1762,16 @@ void BytecodeGenerator::VisitForOfStatement(ForOfStatement* stmt) {
 }
 
 void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
-  TryCatchBuilder try_control_builder(builder(), stmt->catch_prediction());
-
-  // Preserve the context in a dedicated register, so that it can be restored
-  // when the handler is entered by the stack-unwinding machinery.
-  // TODO(mstarzinger): Be smarter about register allocation.
-  Register context = register_allocator()->NewRegister();
-  builder()->MoveRegister(Register::current_context(), context);
-
-  // Evaluate the try-block inside a control scope. This simulates a handler
-  // that is intercepting 'throw' control commands.
-  try_control_builder.BeginTry(context);
+  SimpleTryCatchBuilder try_control_builder(this, stmt->catch_prediction());
   {
-    ControlScopeForTryCatch scope(this, &try_control_builder);
+    TryBlockBuilderForCatch try_block(try_control_builder);
     Visit(stmt->try_block());
   }
-  try_control_builder.EndTry();
 
+  // Evaluate the catch-block.
   // Create a catch scope that binds the exception.
+  CatchBlockBuilder catch_block(try_control_builder);
+  Register context = try_control_builder.context();
   BuildNewLocalCatchContext(stmt->variable(), stmt->scope());
   builder()->StoreAccumulatorInRegister(context);
 
@@ -1363,72 +1782,18 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
 
   // Load the catch context into the accumulator.
   builder()->LoadAccumulatorWithRegister(context);
-
-  // Evaluate the catch-block.
   VisitInScope(stmt->catch_block(), stmt->scope());
-  try_control_builder.EndCatch();
 }
 
 void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) {
-  TryFinallyBuilder try_control_builder(builder(), stmt->catch_prediction());
-
-  // We keep a record of all paths that enter the finally-block to be able to
-  // dispatch to the correct continuation point after the statements in the
-  // finally-block have been evaluated.
-  //
-  // The try-finally construct can enter the finally-block in three ways:
-  // 1. By exiting the try-block normally, falling through at the end.
-  // 2. By exiting the try-block with a function-local control flow transfer
-  //    (i.e. through break/continue/return statements).
-  // 3. By exiting the try-block with a thrown exception.
-  //
-  // The result register semantics depend on how the block was entered:
-  //  - ReturnStatement: It represents the return value being returned.
-  //  - ThrowStatement: It represents the exception being thrown.
-  //  - BreakStatement/ContinueStatement: Undefined and not used.
-  //  - Falling through into finally-block: Undefined and not used.
-  Register token = register_allocator()->NewRegister();
-  Register result = register_allocator()->NewRegister();
-  ControlScope::DeferredCommands commands(this, token, result);
-
-  // Preserve the context in a dedicated register, so that it can be restored
-  // when the handler is entered by the stack-unwinding machinery.
-  // TODO(mstarzinger): Be smarter about register allocation.
-  Register context = register_allocator()->NewRegister();
-  builder()->MoveRegister(Register::current_context(), context);
-
-  // Evaluate the try-block inside a control scope. This simulates a handler
-  // that is intercepting all control commands.
-  try_control_builder.BeginTry(context);
+  SimpleTryFinallyBuilder try_control_builder(this);
   {
-    ControlScopeForTryFinally scope(this, &try_control_builder, &commands);
+    TryBlockBuilderForFinally try_block(try_control_builder);
     Visit(stmt->try_block());
   }
-  try_control_builder.EndTry();
-
-  // Record fall-through and exception cases.
-  commands.RecordFallThroughPath();
-  try_control_builder.LeaveTry();
-  try_control_builder.BeginHandler();
-  commands.RecordHandlerReThrowPath();
-
-  // Pending message object is saved on entry.
-  try_control_builder.BeginFinally();
-  Register message = context;  // Reuse register.
 
-  // Clear message object as we enter the finally block.
-  builder()->LoadTheHole().SetPendingMessage().StoreAccumulatorInRegister(
-      message);
-
-  // Evaluate the finally-block.
+  FinallyBlockBuilder finally_block(try_control_builder);
   Visit(stmt->finally_block());
-  try_control_builder.EndFinally();
-
-  // Pending message object is restored on exit.
-  builder()->LoadAccumulatorWithRegister(message).SetPendingMessage();
-
-  // Dynamic dispatch after the finally-block.
-  commands.ApplyDeferredCommands();
 }
 
 void BytecodeGenerator::VisitDebuggerStatement(DebuggerStatement* stmt) {
@@ -2287,20 +2652,31 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   }
 }
 
-void BytecodeGenerator::VisitYield(Yield* expr) {
-  builder()->SetExpressionPosition(expr);
-  Register value = VisitForRegisterValue(expr->expression());
-
-  Register generator = VisitForRegisterValue(expr->generator_object());
+void BytecodeGenerator::BuildYield(int yield_id, Register generator,
+                                   Register value) {
+  DCHECK_GE(yield_id, 0);
+  DCHECK_LT(yield_id, generator_resume_points_.size());
+  DCHECK(generator.is_valid());
+  DCHECK(value.is_valid());
 
   // Save context, registers, and state. Then return.
   builder()
-      ->LoadLiteral(Smi::FromInt(expr->yield_id()))
+      ->LoadLiteral(Smi::FromInt(yield_id))
       .SuspendGenerator(generator)
       .LoadAccumulatorWithRegister(value)
       .Return();  // Hard return (ignore any finally blocks).
+}
 
-  builder()->Bind(&(generator_resume_points_[expr->yield_id()]));
+void BytecodeGenerator::BuildYieldResumePoint(int yield_id,
+                                              Yield::OnException on_exception,
+                                              Register generator,
+                                              int position) {
+  DCHECK_GE(yield_id, 0);
+  DCHECK_LT(yield_id, generator_resume_points_.size());
+  DCHECK(generator.is_valid());
+
+  DCHECK(!generator_resume_points_[yield_id].is_bound());
+  builder()->Bind(&(generator_resume_points_[yield_id]));
   // Upon resume, we continue here.
 
   {
@@ -2328,6 +2704,7 @@ void BytecodeGenerator::VisitYield(Yield* expr) {
     BytecodeLabel resume_with_return;
     BytecodeLabel resume_with_throw;
 
+    // TODO(caitp): Don't generate `resume_with_return` block for non-generators
     builder()
         ->LoadLiteral(Smi::FromInt(JSGeneratorObject::kNext))
         .CompareOperation(Token::EQ_STRICT, resume_mode)
@@ -2349,9 +2726,9 @@ void BytecodeGenerator::VisitYield(Yield* expr) {
     }
 
     builder()->Bind(&resume_with_throw);
-    builder()->SetExpressionPosition(expr);
+    builder()->SetExpressionPosition(position);
     builder()->LoadAccumulatorWithRegister(input);
-    if (expr->rethrow_on_exception()) {
+    if (on_exception == Yield::kOnExceptionRethrow) {
       builder()->ReThrow();
     } else {
       builder()->Throw();
@@ -2362,6 +2739,17 @@ void BytecodeGenerator::VisitYield(Yield* expr) {
   }
 }
 
+void BytecodeGenerator::VisitYield(Yield* expr) {
+  Register generator = VisitForRegisterValue(expr->generator_object());
+
+  builder()->SetExpressionPosition(expr);
+  Register value = VisitForRegisterValue(expr->expression());
+
+  BuildYield(expr->yield_id(), generator, value);
+  BuildYieldResumePoint(expr->yield_id(), expr->on_exception(), generator,
+                        expr->position());
+}
+
 void BytecodeGenerator::VisitThrow(Throw* expr) {
   VisitForAccumulatorValue(expr->exception());
   builder()->SetExpressionPosition(expr);