[interpreter] Always 'continue' loops by jumping forward to end of body.

We sometimes used to continue by jumping _back_ to the condition check at the
top of the loop. After my recent generator-related changes, that check is no
longer at the loop header, so a continue could create an additional loop. In
order to avoid this, we now always set the continue target to be the first
instruction following the loop body.

BUG=

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

src/interpreter/bytecode-generator.cc

@@ -1038,6 +1038,7 @@ void BytecodeGenerator::VisitIterationBody(IterationStatement* stmt,
   ControlScopeForIteration execution_control(this, stmt, loop_builder);
   builder()->StackCheck(stmt->position());
   Visit(stmt->body());
+  loop_builder->SetContinueTarget();
 }
 
 void BytecodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) {
@@ -1045,14 +1046,11 @@ void BytecodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) {
   VisitIterationHeader(stmt, &loop_builder);
   if (stmt->cond()->ToBooleanIsFalse()) {
     VisitIterationBody(stmt, &loop_builder);
-    loop_builder.Condition();
   } else if (stmt->cond()->ToBooleanIsTrue()) {
-    loop_builder.Condition();
     VisitIterationBody(stmt, &loop_builder);
     loop_builder.JumpToHeader();
   } else {
     VisitIterationBody(stmt, &loop_builder);
-    loop_builder.Condition();
     builder()->SetExpressionAsStatementPosition(stmt->cond());
     VisitForAccumulatorValue(stmt->cond());
     loop_builder.JumpToHeaderIfTrue();
@@ -1068,7 +1066,6 @@ void BytecodeGenerator::VisitWhileStatement(WhileStatement* stmt) {
 
   LoopBuilder loop_builder(builder());
   VisitIterationHeader(stmt, &loop_builder);
-  loop_builder.Condition();
   if (!stmt->cond()->ToBooleanIsTrue()) {
     builder()->SetExpressionAsStatementPosition(stmt->cond());
     VisitForAccumulatorValue(stmt->cond());
@@ -1092,7 +1089,6 @@ void BytecodeGenerator::VisitForStatement(ForStatement* stmt) {
 
   LoopBuilder loop_builder(builder());
   VisitIterationHeader(stmt, &loop_builder);
-  loop_builder.Condition();
   if (stmt->cond() && !stmt->cond()->ToBooleanIsTrue()) {
     builder()->SetExpressionAsStatementPosition(stmt->cond());
     VisitForAccumulatorValue(stmt->cond());
@@ -1100,7 +1096,6 @@ void BytecodeGenerator::VisitForStatement(ForStatement* stmt) {
   }
   VisitIterationBody(stmt, &loop_builder);
   if (stmt->next() != nullptr) {
-    loop_builder.Next();
     builder()->SetStatementPosition(stmt->next());
     Visit(stmt->next());
   }
@@ -1218,7 +1213,6 @@ void BytecodeGenerator::VisitForInStatement(ForInStatement* stmt) {
   // The loop
   VisitIterationHeader(stmt, &loop_builder);
   builder()->SetExpressionAsStatementPosition(stmt->each());
-  loop_builder.Condition();
   builder()->ForInDone(index, cache_length);
   loop_builder.BreakIfTrue();
   DCHECK(Register::AreContiguous(cache_type, cache_array));
@@ -1227,7 +1221,6 @@ void BytecodeGenerator::VisitForInStatement(ForInStatement* stmt) {
   loop_builder.ContinueIfUndefined();
   VisitForInAssignment(stmt->each(), stmt->EachFeedbackSlot());
   VisitIterationBody(stmt, &loop_builder);
-  loop_builder.Next();
   builder()->ForInStep(index);
   builder()->StoreAccumulatorInRegister(index);
   loop_builder.JumpToHeader();
@@ -1245,7 +1238,6 @@ void BytecodeGenerator::VisitForOfStatement(ForOfStatement* stmt) {
   VisitForEffect(stmt->assign_iterator());
 
   VisitIterationHeader(stmt, &loop_builder);
-  loop_builder.Next();
   builder()->SetExpressionAsStatementPosition(stmt->next_result());
   VisitForEffect(stmt->next_result());
   VisitForAccumulatorValue(stmt->result_done());

src/interpreter/control-flow-builders.cc

@@ -109,19 +109,11 @@ void LoopBuilder::EndLoop() {
   DCHECK(loop_header_.is_bound());
   builder()->Bind(&loop_end_);
   SetBreakTarget(loop_end_);
-  if (next_.is_bound()) {
-    DCHECK(!condition_.is_bound() || next_.offset() >= condition_.offset());
-    SetContinueTarget(next_);
-  } else {
-    DCHECK(condition_.is_bound());
-    DCHECK_GE(condition_.offset(), loop_header_.offset());
-    DCHECK_LE(condition_.offset(), loop_end_.offset());
-    SetContinueTarget(condition_);
-  }
 }
 
-
-void LoopBuilder::SetContinueTarget(const BytecodeLabel& target) {
+void LoopBuilder::SetContinueTarget() {
+  BytecodeLabel target;
+  builder()->Bind(&target);
   BindLabels(target, &continue_sites_);
 }
 

src/interpreter/control-flow-builders.h

@@ -88,26 +88,21 @@ class LoopBuilder final : public BreakableControlFlowBuilder {
   ~LoopBuilder();
 
   void LoopHeader(ZoneVector<BytecodeLabel>* additional_labels);
-  void Condition() { builder()->Bind(&condition_); }
-  void Next() { builder()->Bind(&next_); }
   void JumpToHeader() { builder()->Jump(&loop_header_); }
   void JumpToHeaderIfTrue() { builder()->JumpIfTrue(&loop_header_); }
+  void SetContinueTarget();
   void EndLoop();
 
   // This method is called when visiting continue statements in the AST.
-  // Inserts a jump to a unbound label that is patched when the corresponding
-  // SetContinueTarget is called.
+  // Inserts a jump to an unbound label that is patched when SetContinueTarget
+  // is called.
   void Continue() { EmitJump(&continue_sites_); }
   void ContinueIfTrue() { EmitJumpIfTrue(&continue_sites_); }
   void ContinueIfUndefined() { EmitJumpIfUndefined(&continue_sites_); }
   void ContinueIfNull() { EmitJumpIfNull(&continue_sites_); }
 
  private:
-  void SetContinueTarget(const BytecodeLabel& continue_target);
-
   BytecodeLabel loop_header_;
-  BytecodeLabel condition_;
-  BytecodeLabel next_;
   BytecodeLabel loop_end_;
 
   // Unbound labels that identify jumps for continue statements in the code.

test/cctest/interpreter/bytecode_expectations/BasicLoops.golden

@@ -90,7 +90,7 @@ bytecodes: [
   B(LdaSmi), U8(3),
   B(TestEqual), R(2),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-39),
+  B(Jump), U8(14),
   B(Ldar), R(0),
   B(Star), R(2),
   B(LdaSmi), U8(4),
@@ -132,7 +132,7 @@ bytecodes: [
   B(LdaZero),
   B(TestLessThan), R(1),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-10),
+  B(Jump), U8(60),
   B(Ldar), R(0),
   B(Star), R(1),
   B(LdaSmi), U8(3),
@@ -150,7 +150,7 @@ bytecodes: [
   B(LdaSmi), U8(10),
   B(TestEqual), R(1),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-46),
+  B(Jump), U8(24),
   B(Ldar), R(0),
   B(Star), R(1),
   B(LdaSmi), U8(5),
@@ -453,7 +453,7 @@ bytecodes: [
   B(LdaSmi), U8(6),
   B(TestEqual), R(2),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-41),
+  B(Jump), U8(2),
   B(Jump), U8(-43),
   B(Ldar), R(1),
   B(Return),
@@ -491,7 +491,7 @@ bytecodes: [
   B(LdaSmi), U8(2),
   B(TestEqual), R(1),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-23),
+  B(Jump), U8(12),
   B(Ldar), R(0),
   B(Star), R(1),
   B(LdaSmi), U8(1),
@@ -533,7 +533,7 @@ bytecodes: [
   B(LdaSmi), U8(2),
   B(TestEqual), R(1),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-23),
+  B(Jump), U8(12),
   B(Ldar), R(0),
   B(Star), R(1),
   B(LdaSmi), U8(1),
@@ -825,7 +825,7 @@ bytecodes: [
   B(JumpIfToBooleanFalse), U8(8),
   B(PopContext), R(3),
   B(PopContext), R(3),
-  B(Jump), U8(-69),
+  B(Jump), U8(43),
   B(LdaContextSlot), R(context), U8(4),
   B(JumpIfNotHole), U8(11),
   B(LdaConstant), U8(3),

test/cctest/interpreter/bytecode_expectations/ForOf.golden

@@ -429,7 +429,7 @@ bytecodes: [
   B(LdaSmi), U8(10),
   B(TestEqual), R(13),
   B(JumpIfFalse), U8(4),
-  B(Jump), U8(-75),
+  B(Jump), U8(17),
   B(Ldar), R(7),
   B(Star), R(13),
   B(LdaSmi), U8(20),