First version of the new generators implementation.

src/interpreter/bytecode-generator.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/interpreter/bytecode-generator.h"
 
 #include "src/ast/scopes.h"
 #include "src/compiler.h"
 #include "src/interpreter/bytecode-register-allocator.h"
 #include "src/interpreter/control-flow-builders.h"
@@ skipping 547 matching lines @@
     : isolate_(isolate),
       zone_(zone),
       builder_(nullptr),
       info_(nullptr),
       scope_(nullptr),
       globals_(0, zone),
       execution_control_(nullptr),
       execution_context_(nullptr),
       execution_result_(nullptr),
       register_allocator_(nullptr),
+      resume_points_(0, zone),
       try_catch_nesting_level_(0),
-      try_finally_nesting_level_(0) {
+      try_finally_nesting_level_(0),
+      yield_count_(0) {
   InitializeAstVisitor(isolate);
 }
 
 Handle<BytecodeArray> BytecodeGenerator::MakeBytecode(CompilationInfo* info) {
   set_info(info);
   set_scope(info->scope());
 
   // Initialize bytecode array builder.
   set_builder(new (zone()) BytecodeArrayBuilder(
       isolate(), zone(), info->num_parameters_including_this(),
       scope()->MaxNestedContextChainLength(), scope()->num_stack_slots(),
       info->literal()));
 
   // Initialize the incoming context.
   ContextScope incoming_context(this, scope(), false);
 
   // Initialize control scope.
   ControlScopeForTopLevel control(this);
 
+  if (IsGeneratorFunction(info->literal()->kind())) {
+    BuildGeneratorPrologue();
+  }
+
   // Build function context only if there are context allocated variables.
   if (scope()->NeedsContext()) {
     // Push a new inner context scope for the function.
     VisitNewLocalFunctionContext();
     ContextScope local_function_context(this, scope(), false);
     VisitBuildLocalActivationContext();
     MakeBytecodeBody();
   } else {
     MakeBytecodeBody();
   }
@@ skipping 28 matching lines @@
   // Visit declarations within the function scope.
   VisitDeclarations(scope()->declarations());
 
   // Perform a stack-check before the body.
   builder()->StackCheck();
 
   // Visit statements in the function body.
   VisitStatements(info()->literal()->body());
 }
 
+void BytecodeGenerator::BuildGeneratorPrologue() {
+  resume_points_.clear();

[rmcilroy, 2016/04/15 13:17:46]

Do we need the clear? it should be empty when we reach here anyway.

[neis, 2016/04/18 08:04:17]

MakeBytecode could be called several times on the same BytecodeGenerator
instance, right?

If that's not supposed to happen, then I don't understand why CompilationInfo is
passed to MakeBytecode and not to the BytecodeGenerator constructor.

[rmcilroy, 2016/04/18 10:01:57]

No it's only ever used for a single compile info. You are right though, we
should probably change the constructor to take the CompileInfo, I'll upload a CL
to make that change.

[neis, 2016/04/18 14:07:52]

Ok. After your change I can probably initialize it directly with the correct
size.

+  resume_points_.resize(info()->literal()->yield_count());
+
+  BytecodeLabel regular_call;
+  builder()
+      ->LoadAccumulatorWithRegister(Register::new_target())
+      .JumpIfUndefined(&regular_call);
+
+  // This is a resume call. Restore registers and perform state dispatch.
+  // (The current context has already been restored by the trampoline.)
+  {
+    RegisterAllocationScope register_scope(this);
+    Register state = register_allocator()->NewRegister();
+    builder()
+        ->CallRuntime(Runtime::kResumeIgnitionGenerator, Register::new_target(),
+                      1)
+        .StoreAccumulatorInRegister(state);
+
+    // TODO(neis): Optimize this by using a proper jump table.
+    for (size_t i = 0; i < resume_points_.size(); ++i) {
+      builder()
+          ->LoadLiteral(Smi::FromInt(static_cast<int>(i)))
+          .CompareOperation(Token::Value::EQ_STRICT, state)
+          .JumpIfTrue(&(resume_points_[i]));
+    }
+    builder()->Illegal();  // Should never get here.
+  }
+
+  builder()->Bind(&regular_call);
+  // This is a regular call. Fall through to the ordinary function prologue,
+  // after which we will run into the generator object creation and the initial
+  // yield (both inserted by the parser).
+}
 
 void BytecodeGenerator::VisitBlock(Block* stmt) {
   // Visit declarations and statements.
   if (stmt->scope() != nullptr && stmt->scope()->NeedsContext()) {
     VisitNewLocalBlockContext(stmt->scope());
     ContextScope scope(this, stmt->scope());
     VisitBlockDeclarationsAndStatements(stmt);
   } else {
     VisitBlockDeclarationsAndStatements(stmt);
   }
@@ skipping 1575 matching lines @@
     }
     case KEYED_SUPER_PROPERTY: {
       builder()->StoreAccumulatorInRegister(value);
       BuildKeyedSuperPropertyStore(object, home_object, key, value);
       break;
     }
   }
   execution_result()->SetResultInAccumulator();
 }
 
+void BytecodeGenerator::VisitYield(Yield* expr) {
+  int id = yield_count_++;

[rmcilroy, 2016/04/15 13:17:46]

Rather than doing this id naming implicitly, would it be safer to have the Yield
AST object include an ID (e.g., the one used for state calculation) which we use
as the id here and as the index to the resume_points_ ?

[neis, 2016/04/18 08:04:17]

I don't think that would be safer and I don't see any advantage. Also, my
transformation for dispatching in loops will rely on yields being numbered
consecutively.

[rmcilroy, 2016/04/18 10:01:57]

OK. I'm fine with this for this CL, but maybe it's worth considering maintaining
yield_count_ and resume_points_ in a small class to abstract the yield / resume
point maintanance (particularly if the dispatching in loops CL adds more logic
to this). 

[neis, 2016/04/18 14:07:52]

OK.  Also, I probably need to move them into arraybuilder for the next CL.

 
-void BytecodeGenerator::VisitYield(Yield* expr) { UNIMPLEMENTED(); }
+  builder()->SetExpressionPosition(expr);
+  Register value = VisitForRegisterValue(expr->expression());

[rmcilroy, 2016/04/15 13:17:46]

nit - maybe scope the pre-yield, and post-resume parts with seperate inner
scopes, and have each allocate it's own RegisterAllocationScope, that way you
will free up temporary registers between the yield and resume points.

[neis, 2016/04/18 08:04:17]

Thanks, I didn't think of that.  I added a scope for the post-resume part.  One
of the registers from the pre-resume part is used later on.

 
+  register_allocator()->PrepareForConsecutiveAllocations(2);
+  Register generator = register_allocator()->NextConsecutiveRegister();
+  Register state = register_allocator()->NextConsecutiveRegister();
+
+  // Save context, registers, and state. Then return.
+  VisitForRegisterValue(expr->generator_object(), generator);
+  builder()
+      ->LoadLiteral(Smi::FromInt(id))
+      .StoreAccumulatorInRegister(state)
+      .CallRuntime(Runtime::kSuspendIgnitionGenerator, generator, 2)
+      .LoadAccumulatorWithRegister(value)
+      .Return();  // Hard return (ignore any finally blocks).
+
+  builder()->Bind(&(resume_points_[id]));
+  // Upon resume, we continue here.
+
+  Register input = register_allocator()->NewRegister();
+  builder()
+      ->CallRuntime(Runtime::kGeneratorGetInput, generator, 1)
+      .StoreAccumulatorInRegister(input);
+
+  Register resume_mode = register_allocator()->NewRegister();
+  builder()
+      ->CallRuntime(Runtime::kGeneratorGetResumeMode, generator, 1)

[rmcilroy, 2016/04/15 13:17:46]

Maybe you could have a runtime function which returns both Input and Resume mode
rather than making two successive runtime calls here?

[neis, 2016/04/18 08:04:17]

Let's keep it as is for now.  I think we want to get rid of these calls in the
long run anyway.

[rmcilroy, 2016/04/18 10:01:57]

Acknowledged.

+      .StoreAccumulatorInRegister(resume_mode);
+
+  // Dispatch on resume mode.
+  {
+    BytecodeLabel resume_with_next;
+    BytecodeLabel resume_with_return;
+    BytecodeLabel resume_with_throw;
+
+    builder()
+        ->LoadLiteral(Smi::FromInt(JSGeneratorObject::kReturn))
+        .CompareOperation(Token::EQ_STRICT, resume_mode)
+        .JumpIfTrue(&resume_with_return)
+        .LoadLiteral(Smi::FromInt(JSGeneratorObject::kThrow))
+        .CompareOperation(Token::EQ_STRICT, resume_mode)
+        .JumpIfTrue(&resume_with_throw)
+        .Jump(&resume_with_next);

[rmcilroy, 2016/04/15 13:17:46]

(micro optimization) - presumably JSGeneratorObject::kNext is the most common
resume mode, if we test for that first we would execute less bytecode in the
common case.

[neis, 2016/04/18 08:04:17]

Done.

+
+    builder()->Bind(&resume_with_return);
+    {
+      register_allocator()->PrepareForConsecutiveAllocations(2);
+      Register value = register_allocator()->NextConsecutiveRegister();
+      Register done = register_allocator()->NextConsecutiveRegister();
+      builder()
+          ->MoveRegister(input, value)
+          .LoadTrue()
+          .StoreAccumulatorInRegister(done)
+          .CallRuntime(Runtime::kCreateIterResultObject, value, 2);
+      execution_control()->ReturnAccumulator();
+    }
+
+    builder()->Bind(&resume_with_throw);
+    builder()
+        ->LoadAccumulatorWithRegister(input)
+        .Throw();
+
+    builder()->Bind(&resume_with_next);
+    builder()->LoadAccumulatorWithRegister(input);
+    execution_result()->SetResultInAccumulator();
+  }
+}
 
 void BytecodeGenerator::VisitThrow(Throw* expr) {
   VisitForAccumulatorValue(expr->exception());
   builder()->SetExpressionPosition(expr);
   builder()->Throw();
-  // Throw statments are modeled as expression instead of statments. These are
-  // converted from assignment statements in Rewriter::ReWrite pass. An
+  // Throw statements are modeled as expressions instead of statements. These
+  // are converted from assignment statements in Rewriter::ReWrite pass. An

[rmcilroy, 2016/04/15 13:17:46]

Thanks!

   // assignment statement expects a value in the accumulator. This is a hack to
   // avoid DCHECK fails assert accumulator has been set.
   execution_result()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) {
   LhsKind property_kind = Property::GetAssignType(expr);
   FeedbackVectorSlot slot = expr->PropertyFeedbackSlot();
   builder()->SetExpressionPosition(expr);
@@ skipping 907 matching lines @@
 }
 
 
 int BytecodeGenerator::feedback_index(FeedbackVectorSlot slot) const {
   return info()->shared_info()->feedback_vector()->GetIndex(slot);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8