[generators] Assign yield ids in ast-numbering rather than in bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code containing yields.

BUG=v8:4907
LOG=n

Review-Url: https://codereview.chromium.org/1927943003
Cr-Commit-Position: refs/heads/master@{#35940}

src/ast/ast-numbering.cc

@@ -72,17 +72,6 @@ class AstNumberingVisitor final : public AstVisitor {
 
   BailoutReason dont_optimize_reason() const { return dont_optimize_reason_; }
 
-  int GetAndResetYieldCount() {
-    int old_yield_count = yield_count_;
-    yield_count_ = 0;
-    return old_yield_count;
-  }
-
-  void StoreAndUpdateYieldCount(IterationStatement* node, int old_yield_count) {
-    node->set_yield_count(yield_count_);
-    yield_count_ += old_yield_count;
-  }
-
   Isolate* isolate_;
   Zone* zone_;
   int next_id_;
@@ -228,6 +217,7 @@ void AstNumberingVisitor::VisitReturnStatement(ReturnStatement* node) {
 
 
 void AstNumberingVisitor::VisitYield(Yield* node) {
+  node->set_yield_id(yield_count_);
   yield_count_++;
   IncrementNodeCount();
   DisableOptimization(kYield);
@@ -296,10 +286,10 @@ void AstNumberingVisitor::VisitDoWhileStatement(DoWhileStatement* node) {
   IncrementNodeCount();
   DisableSelfOptimization();
   node->set_base_id(ReserveIdRange(DoWhileStatement::num_ids()));
-  int old_yield_count = GetAndResetYieldCount();
+  node->set_first_yield_id(yield_count_);
   Visit(node->body());
   Visit(node->cond());
-  StoreAndUpdateYieldCount(node, old_yield_count);
+  node->set_yield_count(yield_count_ - node->first_yield_id());
 }
 
 
@@ -307,10 +297,10 @@ void AstNumberingVisitor::VisitWhileStatement(WhileStatement* node) {
   IncrementNodeCount();
   DisableSelfOptimization();
   node->set_base_id(ReserveIdRange(WhileStatement::num_ids()));
-  int old_yield_count = GetAndResetYieldCount();
+  node->set_first_yield_id(yield_count_);
   Visit(node->cond());
   Visit(node->body());
-  StoreAndUpdateYieldCount(node, old_yield_count);
+  node->set_yield_count(yield_count_ - node->first_yield_id());
 }
 
 
@@ -394,10 +384,10 @@ void AstNumberingVisitor::VisitForInStatement(ForInStatement* node) {
   DisableSelfOptimization();
   node->set_base_id(ReserveIdRange(ForInStatement::num_ids()));
   Visit(node->enumerable());  // Not part of loop.
-  int old_yield_count = GetAndResetYieldCount();
+  node->set_first_yield_id(yield_count_);
   Visit(node->each());
   Visit(node->body());
-  StoreAndUpdateYieldCount(node, old_yield_count);
+  node->set_yield_count(yield_count_ - node->first_yield_id());
   ReserveFeedbackSlots(node);
 }
 
@@ -407,12 +397,12 @@ void AstNumberingVisitor::VisitForOfStatement(ForOfStatement* node) {
   DisableCrankshaft(kForOfStatement);
   node->set_base_id(ReserveIdRange(ForOfStatement::num_ids()));
   Visit(node->assign_iterator());  // Not part of loop.
-  int old_yield_count = GetAndResetYieldCount();
+  node->set_first_yield_id(yield_count_);
   Visit(node->next_result());
   Visit(node->result_done());
   Visit(node->assign_each());
   Visit(node->body());
-  StoreAndUpdateYieldCount(node, old_yield_count);
+  node->set_yield_count(yield_count_ - node->first_yield_id());
   ReserveFeedbackSlots(node);
 }
 
@@ -461,11 +451,11 @@ void AstNumberingVisitor::VisitForStatement(ForStatement* node) {
   DisableSelfOptimization();
   node->set_base_id(ReserveIdRange(ForStatement::num_ids()));
   if (node->init() != NULL) Visit(node->init());  // Not part of loop.
-  int old_yield_count = GetAndResetYieldCount();
+  node->set_first_yield_id(yield_count_);
   if (node->cond() != NULL) Visit(node->cond());
   if (node->next() != NULL) Visit(node->next());
   Visit(node->body());
-  StoreAndUpdateYieldCount(node, old_yield_count);
+  node->set_yield_count(yield_count_ - node->first_yield_id());
 }
 
 

src/ast/ast-numbering.h

@@ -14,12 +14,27 @@ class Isolate;
 class Zone;
 
 namespace AstNumbering {
-// Assign type feedback IDs and bailout IDs to an AST node tree.  For a
-// generator function, also annotate the function itself and any loops therein
-// with the number of contained yields.
+// Assign type feedback IDs, bailout IDs, and generator yield IDs to an AST node
+// tree.
 bool Renumber(Isolate* isolate, Zone* zone, FunctionLiteral* function);
 }
 
+// Some details on yield IDs
+// -------------------------
+//
+// In order to assist Ignition in generating bytecode for a generator function,
+// we assign a unique number (the yield ID) to each Yield node in its AST. We
+// also annotate loops with the number of yields they contain (loop.yield_count)
+// and the smallest ID of those (loop.first_yield_id), and we annotate the
+// function itself with the number of yields it contains (function.yield_count).
+//
+// The way in which we choose the IDs is simply by enumerating the Yield nodes.
+// Ignition relies on the following properties:
+// - For each loop l and each yield y of l:
+//     l.first_yield_id  <=  y.yield_id  <  l.first_yield_id + l.yield_count
+// - For the generator function f itself and each yield y of f:
+//                    0  <=  y.yield_id  <  f.yield_count
+
 }  // namespace internal
 }  // namespace v8
 

src/ast/ast.h

@@ -647,8 +647,12 @@ class IterationStatement : public BreakableStatement {
   Statement* body() const { return body_; }
   void set_body(Statement* s) { body_ = s; }
 
-  int yield_count() { return yield_count_; }
+  int yield_count() const { return yield_count_; }
+  int first_yield_id() const { return first_yield_id_; }
   void set_yield_count(int yield_count) { yield_count_ = yield_count; }
+  void set_first_yield_id(int first_yield_id) {
+    first_yield_id_ = first_yield_id;
+  }
 
   static int num_ids() { return parent_num_ids() + 1; }
   BailoutId OsrEntryId() const { return BailoutId(local_id(0)); }
@@ -662,7 +666,8 @@ class IterationStatement : public BreakableStatement {
   IterationStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
       : BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos),
         body_(NULL),
-        yield_count_(0) {}
+        yield_count_(0),
+        first_yield_id_(0) {}
   static int parent_num_ids() { return BreakableStatement::num_ids(); }
   void Initialize(Statement* body) { body_ = body; }
 
@@ -672,6 +677,7 @@ class IterationStatement : public BreakableStatement {
   Statement* body_;
   Label continue_target_;
   int yield_count_;
+  int first_yield_id_;
 };
 
 
@@ -2531,20 +2537,24 @@ class Yield final : public Expression {
 
   Expression* generator_object() const { return generator_object_; }
   Expression* expression() const { return expression_; }
+  int yield_id() const { return yield_id_; }
 
   void set_generator_object(Expression* e) { generator_object_ = e; }
   void set_expression(Expression* e) { expression_ = e; }
+  void set_yield_id(int yield_id) { yield_id_ = yield_id; }
 
  protected:
   Yield(Zone* zone, Expression* generator_object, Expression* expression,
         int pos)
       : Expression(zone, pos),
         generator_object_(generator_object),
-        expression_(expression) {}
+        expression_(expression),
+        yield_id_(-1) {}
 
  private:
   Expression* generator_object_;
   Expression* expression_;
+  int yield_id_;
 };
 
 

src/ast/prettyprinter.cc

@@ -1609,7 +1609,9 @@ void AstPrinter::VisitAssignment(Assignment* node) {
 
 
 void AstPrinter::VisitYield(Yield* node) {
-  IndentedScope indent(this, "YIELD", node->position());
+  EmbeddedVector<char, 128> buf;
+  SNPrintF(buf, "YIELD id %d", node->yield_id());
+  IndentedScope indent(this, buf.start(), node->position());
   Visit(node->expression());
 }
 

src/interpreter/bytecode-generator.cc

@@ -571,7 +571,6 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
       register_allocator_(nullptr),
       generator_resume_points_(info->literal()->yield_count(), info->zone()),
       generator_state_(),
-      generator_yields_seen_(0),
       try_catch_nesting_level_(0),
       try_finally_nesting_level_(0) {
   InitializeAstVisitor(isolate());
@@ -668,8 +667,8 @@ void BytecodeGenerator::VisitIterationHeader(IterationStatement* stmt,
   // that they can be bound to the loop header below. Also create fresh labels
   // for these resume points, to be used inside the loop.
   ZoneVector<BytecodeLabel> resume_points_in_loop(zone());
-  for (size_t id = generator_yields_seen_;
-       id < generator_yields_seen_ + stmt->yield_count(); id++) {
+  size_t first_yield = stmt->first_yield_id();
+  for (size_t id = first_yield; id < first_yield + stmt->yield_count(); id++) {
     DCHECK(0 <= id && id < generator_resume_points_.size());
     auto& label = generator_resume_points_[id];
     resume_points_in_loop.push_back(label);
@@ -686,8 +685,8 @@ void BytecodeGenerator::VisitIterationHeader(IterationStatement* stmt,
         ->LoadLiteral(Smi::FromInt(JSGeneratorObject::kGeneratorExecuting))
         .CompareOperation(Token::Value::EQ, generator_state_)
         .JumpIfTrue(&not_resuming);
-    BuildIndexedJump(generator_state_, generator_yields_seen_,
-                     stmt->yield_count(), generator_resume_points_);
+    BuildIndexedJump(generator_state_, first_yield,
+        stmt->yield_count(), generator_resume_points_);
     builder()->Bind(&not_resuming);
   }
 }
@@ -2278,8 +2277,6 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
 }
 
 void BytecodeGenerator::VisitYield(Yield* expr) {
-  size_t id = generator_yields_seen_++;
-
   builder()->SetExpressionPosition(expr);
   Register value = VisitForRegisterValue(expr->expression());
 
@@ -2287,12 +2284,12 @@ void BytecodeGenerator::VisitYield(Yield* expr) {
 
   // Save context, registers, and state. Then return.
   builder()
-      ->LoadLiteral(Smi::FromInt(static_cast<int>(id)))
+      ->LoadLiteral(Smi::FromInt(expr->yield_id()))
       .SuspendGenerator(generator)
       .LoadAccumulatorWithRegister(value)
       .Return();  // Hard return (ignore any finally blocks).
 
-  builder()->Bind(&(generator_resume_points_[id]));
+  builder()->Bind(&(generator_resume_points_[expr->yield_id()]));
   // Upon resume, we continue here.
 
   {

src/interpreter/bytecode-generator.h

@@ -210,7 +210,6 @@ class BytecodeGenerator final : public AstVisitor {
   RegisterAllocationScope* register_allocator_;
   ZoneVector<BytecodeLabel> generator_resume_points_;
   Register generator_state_;
-  size_t generator_yields_seen_;
   int try_catch_nesting_level_;
   int try_finally_nesting_level_;
 };