Bug 1456404 - Part 5: Add WhileEmitter. r=jwalden

js/src/frontend/BytecodeEmitter.cpp

@@ -35,6 +35,7 @@
 #include "frontend/SwitchEmitter.h"
 #include "frontend/TDZCheckCache.h"
 #include "frontend/TryEmitter.h"
+#include "frontend/WhileEmitter.h"
 #include "vm/BytecodeUtil.h"
 #include "vm/Debugger.h"
 #include "vm/GeneratorObject.h"
@@ -5375,73 +5376,20 @@ BytecodeEmitter::emitDo(ParseNode* pn)
 bool
 BytecodeEmitter::emitWhile(ParseNode* pn)
 {
-    /*
-     * Minimize bytecodes issued for one or more iterations by jumping to
-     * the condition below the body and closing the loop if the condition
-     * is true with a backward branch. For iteration count i:
-     *
-     *  i    test at the top                 test at the bottom
-     *  =    ===============                 ==================
-     *  0    ifeq-pass                       goto; ifne-fail
-     *  1    ifeq-fail; goto; ifne-pass      goto; ifne-pass; ifne-fail
-     *  2    2*(ifeq-fail; goto); ifeq-pass  goto; 2*ifne-pass; ifne-fail
-     *  . . .
-     *  N    N*(ifeq-fail; goto); ifeq-pass  goto; N*ifne-pass; ifne-fail
-     */
-
-    // If we have a single-line while, like "while (x) ;", we want to
-    // emit the line note before the initial goto, so that the
-    // debugger sees a single entry point.  This way, if there is a
-    // breakpoint on the line, it will only fire once; and "next"ing
-    // will skip the whole loop.  However, for the multi-line case we
-    // want to emit the line note after the initial goto, so that
-    // "cont" stops on each iteration -- but without a stop before the
-    // first iteration.
-    if (parser->errorReporter().lineAt(pn->pn_pos.begin) ==
-        parser->errorReporter().lineAt(pn->pn_pos.end))
-    {
-        if (!updateSourceCoordNotes(pn->pn_pos.begin))
-            return false;
-    }
-
-    JumpTarget top{ -1 };
-    if (!emitJumpTarget(&top))
+    WhileEmitter wh(this);
+    if (!wh.emitBody(Some(pn->pn_pos.begin), getOffsetForLoop(pn->pn_right), Some(pn->pn_pos.end)))
         return false;
 
-    LoopControl loopInfo(this, StatementKind::WhileLoop);
-    loopInfo.setContinueTarget(top.offset);
-
-    unsigned noteIndex;
-    if (!newSrcNote(SRC_WHILE, &noteIndex))
+    if (!emitTree(pn->pn_right))
         return false;
 
-    if (!loopInfo.emitEntryJump(this))
+    if (!wh.emitCond(getOffsetForLoop(pn->pn_left)))
         return false;
 
-    if (!loopInfo.emitLoopHead(this, getOffsetForLoop(pn->pn_right)))
-        return false;
-
-    if (!emitTreeInBranch(pn->pn_right))
-        return false;
-
-    if (!loopInfo.emitLoopEntry(this, getOffsetForLoop(pn->pn_left)))
-        return false;
     if (!emitTree(pn->pn_left))
         return false;
 
-    if (!loopInfo.emitLoopEnd(this, JSOP_IFNE))
-        return false;
-
-    if (!tryNoteList.append(JSTRY_LOOP, stackDepth, loopInfo.headOffset(),
-                            loopInfo.breakTargetOffset()))
-    {
-        return false;
-    }
-
-    if (!setSrcNoteOffset(noteIndex, 0, loopInfo.loopEndOffsetFromEntryJump()))
-        return false;
-
-    if (!loopInfo.patchBreaksAndContinues(this))
+    if (!wh.emitEnd())
         return false;
 
     return true;

js/src/frontend/SourceNotes.h

@@ -54,6 +54,16 @@ class SrcNote {
             Count,
         };
     };
+    // SRC_WHILE: Source note for JSOP_GOTO at the top of while loop,
+    //            which jumps to JSOP_LOOPENTRY.
+    class While {
+      public:
+        enum Fields {
+            // The offset of JSOP_IFNE at the end of the loop from JSOP_GOTO.
+            BackJumpOffset,
+            Count,
+        };
+    };
     // SRC_FOR_IN: Source note for JSOP_GOTO at the top of for-in loop,
     //             which jumps to JSOP_LOOPENTRY.
     class ForIn {
@@ -144,8 +154,7 @@ class SrcNote {
     M(SRC_IF_ELSE,      "if-else",     0)  /* JSOP_IFEQ bytecode is from an if-then-else. */       \
     M(SRC_COND,         "cond",        0)  /* JSOP_IFEQ is from conditional ?: operator. */        \
     M(SRC_FOR,          "for",         SrcNote::For::Count) \
-    M(SRC_WHILE,        "while",       1)  /* JSOP_GOTO to for or while loop condition from before \
-                                              loop, else JSOP_NOP at top of do-while loop. */      \
+    M(SRC_WHILE,        "while",       SrcNote::While::Count) \
     M(SRC_FOR_IN,       "for-in",      SrcNote::ForIn::Count) \
     M(SRC_FOR_OF,       "for-of",      SrcNote::ForOf::Count) \
     M(SRC_CONTINUE,     "continue",    0)  /* JSOP_GOTO is a continue. */                          \

js/src/frontend/WhileEmitter.cpp

@@ -0,0 +1,126 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "frontend/WhileEmitter.h"
+
+#include "frontend/BytecodeEmitter.h"
+#include "frontend/SourceNotes.h"
+#include "vm/Opcodes.h"
+
+using namespace js;
+using namespace js::frontend;
+
+using mozilla::Maybe;
+
+WhileEmitter::WhileEmitter(BytecodeEmitter* bce)
+  : bce_(bce)
+{}
+
+bool
+WhileEmitter::emitBody(const Maybe<uint32_t>& whilePos,
+                       const Maybe<uint32_t>& bodyPos,
+                       const Maybe<uint32_t>& endPos)
+{
+    MOZ_ASSERT(state_ == State::Start);
+
+    // Minimize bytecodes issued for one or more iterations by jumping to
+    // the condition below the body and closing the loop if the condition
+    // is true with a backward branch. For iteration count i:
+    //
+    //  i    test at the top                 test at the bottom
+    //  =    ===============                 ==================
+    //  0    ifeq-pass                       goto; ifne-fail
+    //  1    ifeq-fail; goto; ifne-pass      goto; ifne-pass; ifne-fail
+    //  2    2*(ifeq-fail; goto); ifeq-pass  goto; 2*ifne-pass; ifne-fail
+    //  . . .
+    //  N    N*(ifeq-fail; goto); ifeq-pass  goto; N*ifne-pass; ifne-fail
+
+    // If we have a single-line while, like "while (x) ;", we want to
+    // emit the line note before the initial goto, so that the
+    // debugger sees a single entry point.  This way, if there is a
+    // breakpoint on the line, it will only fire once; and "next"ing
+    // will skip the whole loop.  However, for the multi-line case we
+    // want to emit the line note after the initial goto, so that
+    // "cont" stops on each iteration -- but without a stop before the
+    // first iteration.
+    if (whilePos && endPos &&
+        bce_->parser->errorReporter().lineAt(*whilePos) ==
+        bce_->parser->errorReporter().lineAt(*endPos))
+    {
+        if (!bce_->updateSourceCoordNotes(*whilePos))
+            return false;
+    }
+
+    JumpTarget top = { -1 };
+    if (!bce_->emitJumpTarget(&top))
+        return false;
+
+    loopInfo_.emplace(bce_, StatementKind::WhileLoop);
+    loopInfo_->setContinueTarget(top.offset);
+
+    if (!bce_->newSrcNote(SRC_WHILE, &noteIndex_))
+        return false;
+
+    if (!loopInfo_->emitEntryJump(bce_))
+        return false;
+
+    if (!loopInfo_->emitLoopHead(bce_, bodyPos))
+        return false;
+
+    tdzCacheForBody_.emplace(bce_);
+
+#ifdef DEBUG
+    state_ = State::Body;
+#endif
+    return true;
+}
+
+bool
+WhileEmitter::emitCond(const Maybe<uint32_t>& condPos)
+{
+    MOZ_ASSERT(state_ == State::Body);
+
+    tdzCacheForBody_.reset();
+
+    if (!loopInfo_->emitLoopEntry(bce_, condPos))
+        return false;
+
+#ifdef DEBUG
+    state_ = State::Cond;
+#endif
+    return true;
+}
+
+bool
+WhileEmitter::emitEnd()
+{
+    MOZ_ASSERT(state_ == State::Cond);
+
+    if (!loopInfo_->emitLoopEnd(bce_, JSOP_IFNE))
+        return false;
+
+    if (!bce_->tryNoteList.append(JSTRY_LOOP, bce_->stackDepth, loopInfo_->headOffset(),
+                                  loopInfo_->breakTargetOffset()))
+    {
+        return false;
+    }
+
+    if (!bce_->setSrcNoteOffset(noteIndex_, SrcNote::While::BackJumpOffset,
+                                loopInfo_->loopEndOffsetFromEntryJump()))
+    {
+        return false;
+    }
+
+    if (!loopInfo_->patchBreaksAndContinues(bce_))
+        return false;
+
+    loopInfo_.reset();
+
+#ifdef DEBUG
+    state_ = State::End;
+#endif
+    return true;
+}

js/src/frontend/WhileEmitter.h

@@ -0,0 +1,99 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef frontend_WhileEmitter_h
+#define frontend_WhileEmitter_h
+
+#include "mozilla/Attributes.h"
+#include "mozilla/Maybe.h"
+
+#include <stdint.h>
+
+#include "frontend/BytecodeControlStructures.h"
+#include "frontend/TDZCheckCache.h"
+
+namespace js {
+namespace frontend {
+
+struct BytecodeEmitter;
+
+// Class for emitting bytecode for while loop.
+//
+// Usage: (check for the return value is omitted for simplicity)
+//
+//   `while (cond) body`
+//     WhileEmitter wh(this);
+//     wh.emitBody(Some(offset_of_while),
+//                 Some(offset_of_body),
+//                 Some(offset_of_end));
+//     emit(body);
+//     wh.emitCond(Some(offset_of_cond));
+//     emit(cond);
+//     wh.emitEnd();
+//
+class MOZ_STACK_CLASS WhileEmitter
+{
+    BytecodeEmitter* bce_;
+
+    // The source note index for SRC_WHILE.
+    unsigned noteIndex_ = 0;
+
+    mozilla::Maybe<LoopControl> loopInfo_;
+
+    // Cache for the loop body, which is enclosed by the cache in `loopInfo_`,
+    // which is effectively for the loop condition.
+    mozilla::Maybe<TDZCheckCache> tdzCacheForBody_;
+
+#ifdef DEBUG
+    // The state of this emitter.
+    //
+    // +-------+ emitBody +------+ emitCond +------+ emitEnd  +-----+
+    // | Start |--------->| Body |--------->| Cond |--------->| End |
+    // +-------+          +------+          +------+          +-----+
+    enum class State {
+        // The initial state.
+        Start,
+
+        // After calling emitBody.
+        Body,
+
+        // After calling emitCond.
+        Cond,
+
+        // After calling emitEnd.
+        End
+    };
+    State state_ = State::Start;
+#endif
+
+  public:
+    explicit WhileEmitter(BytecodeEmitter* bce);
+
+    // Parameters are the offset in the source code for each character below:
+    //
+    //   while ( x < 20 ) { ... }
+    //   ^       ^        ^     ^
+    //   |       |        |     |
+    //   |       |        |     endPos_
+    //   |       |        |
+    //   |       |        bodyPos_
+    //   |       |
+    //   |       condPos_
+    //   |
+    //   whilePos_
+    //
+    // Can be Nothing() if not available.
+    MOZ_MUST_USE bool emitBody(const mozilla::Maybe<uint32_t>& whilePos,
+                               const mozilla::Maybe<uint32_t>& bodyPos,
+                               const mozilla::Maybe<uint32_t>& endPos);
+    MOZ_MUST_USE bool emitCond(const mozilla::Maybe<uint32_t>& condPos);
+    MOZ_MUST_USE bool emitEnd();
+};
+
+} /* namespace frontend */
+} /* namespace js */
+
+#endif /* frontend_WhileEmitter_h */

js/src/jit/IonControlFlow.cpp

@@ -910,12 +910,11 @@ ControlFlowGenerator::processWhileOrForInOrForOfLoop(jssrcnote* sn)
     // for-in/for-of loops are similar; for-in/for-of have IFEQ as the back
     // jump, and the cond of for-in will be a MOREITER.
     MOZ_ASSERT(SN_TYPE(sn) == SRC_FOR_OF || SN_TYPE(sn) == SRC_FOR_IN || SN_TYPE(sn) == SRC_WHILE);
-    // FIXME: Replaced in the subsequent patch.
-    static_assert(unsigned(SrcNote::ForIn::BackJumpOffset) == 0,
+    static_assert(unsigned(SrcNote::While::BackJumpOffset) == unsigned(SrcNote::ForIn::BackJumpOffset),
                   "SrcNote::{While,ForIn,ForOf}::BackJumpOffset should be same");
-    static_assert(unsigned(SrcNote::ForOf::BackJumpOffset) == 0,
+    static_assert(unsigned(SrcNote::While::BackJumpOffset) == unsigned(SrcNote::ForOf::BackJumpOffset),
                   "SrcNote::{While,ForIn,ForOf}::BackJumpOffset should be same");
-    int backjumppcOffset = GetSrcNoteOffset(sn, SrcNote::ForIn::BackJumpOffset);
+    int backjumppcOffset = GetSrcNoteOffset(sn, SrcNote::While::BackJumpOffset);
     jsbytecode* backjumppc = pc + backjumppcOffset;
     MOZ_ASSERT(backjumppc > pc);
 

js/src/moz.build

@@ -225,6 +225,7 @@ UNIFIED_SOURCES += [
     'frontend/TDZCheckCache.cpp',
     'frontend/TokenStream.cpp',
     'frontend/TryEmitter.cpp',
+    'frontend/WhileEmitter.cpp',
     'gc/Allocator.cpp',
     'gc/AtomMarking.cpp',
     'gc/Barrier.cpp',

js/src/shell/js.cpp

@@ -2764,8 +2764,11 @@ SrcNotes(JSContext* cx, HandleScript script, Sprinter* sp)
             break;
 
           case SRC_WHILE:
-            if (!sp->jsprintf(" offset %u", unsigned(GetSrcNoteOffset(sn, 0))))
+            if (!sp->jsprintf(" offset %u",
+                              unsigned(GetSrcNoteOffset(sn, SrcNote::While::BackJumpOffset))))
+            {
                 return false;
+            }
             break;
 
           case SRC_NEXTCASE: