Bug 1535994 - Part 11: Move line/colum information to BytecodeSection class. r=jorendorff
authorTooru Fujisawa <arai_a@mac.com>
Tue, 02 Apr 2019 18:21:07 +0900
changeset 468822 ced956a624de1e59f318bb124c79417068fecd10
parent 468821 ab95e8d9d3002ef0ffd3b24a8c94db13094196f9
child 468823 16070c339499f48ad623688092ae6733663cac22
push id35851
push userdvarga@mozilla.com
push dateWed, 10 Apr 2019 21:56:12 +0000
treeherdermozilla-central@30ca3c3abfe6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1535994
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1535994 - Part 11: Move line/colum information to BytecodeSection class. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D25741
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/frontend/CForEmitter.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -86,22 +86,24 @@ static bool ParseNodeRequiresSpecialLine
   // handling to avoid strange stepping behavior.
   // Functions usually shouldn't have location information (bug 1431202).
 
   ParseNodeKind kind = pn->getKind();
   return kind == ParseNodeKind::WhileStmt || kind == ParseNodeKind::ForStmt ||
          kind == ParseNodeKind::Function;
 }
 
-BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx)
+BytecodeEmitter::BytecodeSection::BytecodeSection(JSContext* cx,
+                                                  uint32_t lineNum)
     : code_(cx),
       notes_(cx),
       tryNoteList_(cx),
       scopeNoteList_(cx),
-      resumeOffsetList_(cx) {}
+      resumeOffsetList_(cx),
+      currentLine_(lineNum) {}
 
 BytecodeEmitter::PerScriptData::PerScriptData(JSContext* cx)
     : scopeList_(cx),
       numberList_(cx),
       atomIndices_(cx->frontendCollectionPool()) {}
 
 bool BytecodeEmitter::PerScriptData::init(JSContext* cx) {
   return atomIndices_.acquire(cx);
@@ -111,27 +113,27 @@ BytecodeEmitter::BytecodeEmitter(
     BytecodeEmitter* parent, SharedContext* sc, HandleScript script,
     Handle<LazyScript*> lazyScript, uint32_t lineNum, EmitterMode emitterMode,
     FieldInitializers fieldInitializers /* = FieldInitializers::Invalid() */)
     : sc(sc),
       cx(sc->cx_),
       parent(parent),
       script(cx, script),
       lazyScript(cx, lazyScript),
-      bytecodeSection_(cx),
+      bytecodeSection_(cx, lineNum),
       perScriptData_(cx),
-      currentLine_(lineNum),
       fieldInitializers_(fieldInitializers),
       firstLine(lineNum),
       emitterMode(emitterMode) {
   MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript);
 
   if (sc->isFunctionBox()) {
     // Functions have IC entries for type monitoring |this| and arguments.
-    bytecodeSection().setNumICEntries(sc->asFunctionBox()->function()->nargs() + 1);
+    bytecodeSection().setNumICEntries(sc->asFunctionBox()->function()->nargs() +
+                                      1);
   }
 }
 
 BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent,
                                  BCEParserHandle* handle, SharedContext* sc,
                                  HandleScript script,
                                  Handle<LazyScript*> lazyScript,
                                  uint32_t lineNum, EmitterMode emitterMode,
@@ -199,36 +201,31 @@ bool BytecodeEmitter::markStepBreakpoint
 
   if (!newSrcNote(SRC_BREAKPOINT)) {
     return false;
   }
 
   // We track the location of the most recent separator for use in
   // markSimpleBreakpoint. Note that this means that the position must already
   // be set before markStepBreakpoint is called.
-  lastSeparatorOffet_ = bytecodeSection().code().length();
-  lastSeparatorLine_ = currentLine_;
-  lastSeparatorColumn_ = lastColumn_;
+  bytecodeSection().updateSeparatorPosition();
 
   return true;
 }
 
 bool BytecodeEmitter::markSimpleBreakpoint() {
   if (inPrologue()) {
     return true;
   }
 
   // If a breakable call ends up being the same location as the most recent
   // expression start, we need to skip marking it breakable in order to avoid
   // having two breakpoints with the same line/column position.
   // Note: This assumes that the position for the call has already been set.
-  bool isDuplicateLocation =
-      lastSeparatorLine_ == currentLine_ && lastSeparatorColumn_ == lastColumn_;
-
-  if (!isDuplicateLocation) {
+  if (!bytecodeSection().isDuplicateLocation()) {
     if (!newSrcNote(SRC_BREAKPOINT)) {
       return false;
     }
   }
 
   return true;
 }
 
@@ -523,92 +520,84 @@ static inline unsigned LengthOfSetLine(u
 bool BytecodeEmitter::updateLineNumberNotes(uint32_t offset) {
   // Don't emit line/column number notes in the prologue.
   if (inPrologue()) {
     return true;
   }
 
   ErrorReporter* er = &parser->errorReporter();
   bool onThisLine;
-  if (!er->isOnThisLine(offset, currentLine(), &onThisLine)) {
+  if (!er->isOnThisLine(offset, bytecodeSection().currentLine(), &onThisLine)) {
     er->errorNoOffset(JSMSG_OUT_OF_MEMORY);
     return false;
   }
 
   if (!onThisLine) {
     unsigned line = er->lineAt(offset);
-    unsigned delta = line - currentLine();
+    unsigned delta = line - bytecodeSection().currentLine();
 
     /*
      * Encode any change in the current source line number by using
      * either several SRC_NEWLINE notes or just one SRC_SETLINE note,
      * whichever consumes less space.
      *
      * NB: We handle backward line number deltas (possible with for
      * loops where the update part is emitted after the body, but its
      * line number is <= any line number in the body) here by letting
      * unsigned delta_ wrap to a very large number, which triggers a
      * SRC_SETLINE.
      */
-    setCurrentLine(line);
+    bytecodeSection().setCurrentLine(line);
     if (delta >= LengthOfSetLine(line)) {
       if (!newSrcNote2(SRC_SETLINE, ptrdiff_t(line))) {
         return false;
       }
     } else {
       do {
         if (!newSrcNote(SRC_NEWLINE)) {
           return false;
         }
       } while (--delta != 0);
     }
 
-    updateSeparatorPosition();
+    bytecodeSection().updateSeparatorPositionIfPresent();
   }
   return true;
 }
 
 /* Updates the line number and column number information in the source notes. */
 bool BytecodeEmitter::updateSourceCoordNotes(uint32_t offset) {
   if (!updateLineNumberNotes(offset)) {
     return false;
   }
 
   // Don't emit line/column number notes in the prologue.
   if (inPrologue()) {
     return true;
   }
 
   uint32_t columnIndex = parser->errorReporter().columnAt(offset);
-  ptrdiff_t colspan = ptrdiff_t(columnIndex) - ptrdiff_t(lastColumn_);
+  ptrdiff_t colspan =
+      ptrdiff_t(columnIndex) - ptrdiff_t(bytecodeSection().lastColumn());
   if (colspan != 0) {
     // If the column span is so large that we can't store it, then just
     // discard this information. This can happen with minimized or otherwise
     // machine-generated code. Even gigantic column numbers are still
     // valuable if you have a source map to relate them to something real;
     // but it's better to fail soft here.
     if (!SN_REPRESENTABLE_COLSPAN(colspan)) {
       return true;
     }
     if (!newSrcNote2(SRC_COLSPAN, SN_COLSPAN_TO_OFFSET(colspan))) {
       return false;
     }
-    lastColumn_ = columnIndex;
-    updateSeparatorPosition();
-  }
-  return true;
-}
-
-/* Updates the last separator position, if present */
-void BytecodeEmitter::updateSeparatorPosition() {
-  if (!inPrologue() &&
-      lastSeparatorOffet_ == bytecodeSection().code().length()) {
-    lastSeparatorLine_ = currentLine_;
-    lastSeparatorColumn_ = lastColumn_;
-  }
+    bytecodeSection().setLastColumn(columnIndex);
+    bytecodeSection().updateSeparatorPositionIfPresent();
+  }
+  return true;
 }
 
 Maybe<uint32_t> BytecodeEmitter::getOffsetForLoop(ParseNode* nextpn) {
   if (!nextpn) {
     return Nothing();
   }
 
   // Try to give the JSOP_LOOPHEAD and JSOP_LOOPENTRY the same line number as
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -123,17 +123,17 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   // The lazy script if mode is LazyFunction, nullptr otherwise.
   Rooted<LazyScript*> lazyScript;
 
  private:
   // Bytecode and all data directly associated with specific opcode/index inside
   // bytecode is stored in this class.
   class BytecodeSection {
    public:
-    explicit BytecodeSection(JSContext* cx);
+    BytecodeSection(JSContext* cx, uint32_t lineNum);
 
     // ---- Bytecode ----
 
     BytecodeVector& code() { return code_; }
     const BytecodeVector& code() const { return code_; }
 
     jsbytecode* code(ptrdiff_t offset) { return code_.begin() + offset; }
     ptrdiff_t offset() const { return code_.end() - code_.begin(); }
@@ -188,16 +188,44 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
     CGResumeOffsetList& resumeOffsetList() { return resumeOffsetList_; }
     const CGResumeOffsetList& resumeOffsetList() const {
       return resumeOffsetList_;
     }
 
     uint32_t numYields() const { return numYields_; }
     void addNumYields() { numYields_++; }
 
+    // ---- Line and column ----
+
+    uint32_t currentLine() const { return currentLine_; }
+    uint32_t lastColumn() const { return lastColumn_; }
+    void setCurrentLine(uint32_t line) {
+      currentLine_ = line;
+      lastColumn_ = 0;
+    }
+    void setLastColumn(uint32_t column) { lastColumn_ = column; }
+
+    void updateSeparatorPosition() {
+      lastSeparatorOffet_ = code().length();
+      lastSeparatorLine_ = currentLine_;
+      lastSeparatorColumn_ = lastColumn_;
+    }
+
+    void updateSeparatorPositionIfPresent() {
+      if (lastSeparatorOffet_ == code().length()) {
+        lastSeparatorLine_ = currentLine_;
+        lastSeparatorColumn_ = lastColumn_;
+      }
+    }
+
+    bool isDuplicateLocation() const {
+      return lastSeparatorLine_ == currentLine_ &&
+             lastSeparatorColumn_ == lastColumn_;
+    }
+
     // ---- JIT ----
 
     size_t numICEntries() const { return numICEntries_; }
     void addNumICEntries() { numICEntries_++; }
     void setNumICEntries(size_t entries) { numICEntries_ = entries; }
 
     uint16_t typesetCount() const { return typesetCount_; }
     void addTypesetCount() { typesetCount_++; }
@@ -246,16 +274,37 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
     // the bytecode offset of the next pc. This indirection makes it easy to
     // resume in the JIT (because BaselineScript stores a resumeIndex => native
     // code array).
     CGResumeOffsetList resumeOffsetList_;
 
     // Number of yield instructions emitted. Does not include JSOP_AWAIT.
     uint32_t numYields_ = 0;
 
+    // ---- Line and column ----
+
+    // Line number for srcnotes.
+    //
+    // WARNING: If this becomes out of sync with already-emitted srcnotes,
+    // we can get undefined behavior.
+    uint32_t currentLine_;
+
+    // Zero-based column index on currentLine_ of last SRC_COLSPAN-annotated
+    // opcode.
+    //
+    // WARNING: If this becomes out of sync with already-emitted srcnotes,
+    // we can get undefined behavior.
+    uint32_t lastColumn_ = 0;
+
+    // The offset, line and column numbers of the last opcode for the
+    // breakpoint for step execution.
+    uint32_t lastSeparatorOffet_ = 0;
+    uint32_t lastSeparatorLine_ = 0;
+    uint32_t lastSeparatorColumn_ = 0;
+
     // ---- JIT ----
 
     // Number of JOF_IC opcodes emitted.
     size_t numICEntries_ = 0;
 
     // Number of JOF_TYPESET opcodes generated.
     uint16_t typesetCount_ = 0;
   };
@@ -313,33 +362,16 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
 
   PerScriptData perScriptData_;
 
  public:
   PerScriptData& perScriptData() { return perScriptData_; }
   const PerScriptData& perScriptData() const { return perScriptData_; }
 
  private:
-  // Line number for srcnotes.
-  //
-  // WARNING: If this becomes out of sync with already-emitted srcnotes,
-  // we can get undefined behavior.
-  uint32_t currentLine_ = 0;
-
-  // Zero-based column index on currentLine of last SRC_COLSPAN-annotated
-  // opcode.
-  //
-  // WARNING: If this becomes out of sync with already-emitted srcnotes,
-  // we can get undefined behavior.
-  uint32_t lastColumn_ = 0;
-
-  uint32_t lastSeparatorOffet_ = 0;
-  uint32_t lastSeparatorLine_ = 0;
-  uint32_t lastSeparatorColumn_ = 0;
-
   // switchToMain sets this to the bytecode offset of the main section.
   mozilla::Maybe<uint32_t> mainOffset_ = {};
 
   /* field info for enclosing class */
   const FieldInitializers fieldInitializers_;
 
  public:
   // Private storage for parser wrapper. DO NOT REFERENCE INTERNALLY. May not be
@@ -564,23 +596,16 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
 
   bool inPrologue() const { return mainOffset_.isNothing(); }
 
   void switchToMain() {
     MOZ_ASSERT(inPrologue());
     mainOffset_.emplace(bytecodeSection().code().length());
   }
 
-  unsigned currentLine() const { return currentLine_; }
-
-  void setCurrentLine(uint32_t line) {
-    currentLine_ = line;
-    lastColumn_ = 0;
-  }
-
   void setFunctionBodyEndPos(uint32_t pos) {
     functionBodyEndPos = mozilla::Some(pos);
   }
 
   void setScriptStartOffsetIfUnset(uint32_t pos) {
     if (scriptStartOffset.isNothing()) {
       scriptStartOffset = mozilla::Some(pos);
     }
@@ -643,17 +668,16 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   enum class TopLevelFunction { No, Yes };
   MOZ_MUST_USE bool emitFunctionScript(FunctionNode* funNode,
                                        TopLevelFunction isTopLevel);
 
   MOZ_MUST_USE bool markStepBreakpoint();
   MOZ_MUST_USE bool markSimpleBreakpoint();
   MOZ_MUST_USE bool updateLineNumberNotes(uint32_t offset);
   MOZ_MUST_USE bool updateSourceCoordNotes(uint32_t offset);
-  void updateSeparatorPosition();
 
   JSOp strictifySetNameOp(JSOp op);
 
   MOZ_MUST_USE bool emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset);
 
   // Emit one bytecode.
   MOZ_MUST_USE bool emit1(JSOp op);
 
--- a/js/src/frontend/CForEmitter.cpp
+++ b/js/src/frontend/CForEmitter.cpp
@@ -158,21 +158,21 @@ bool CForEmitter::emitCond(const Maybe<u
     if (!bce_->emit1(JSOP_POP)) {
       //            [stack]
       return false;
     }
 
     // Restore the absolute line number for source note readers.
     if (endPos) {
       uint32_t lineNum = bce_->parser->errorReporter().lineAt(*endPos);
-      if (bce_->currentLine() != lineNum) {
+      if (bce_->bytecodeSection().currentLine() != lineNum) {
         if (!bce_->newSrcNote2(SRC_SETLINE, ptrdiff_t(lineNum))) {
           return false;
         }
-        bce_->setCurrentLine(lineNum);
+        bce_->bytecodeSection().setCurrentLine(lineNum);
       }
     }
   }
 
   if (update_ == Update::Present) {
     tdzCache_.reset();
   }