Bug 1461888 - Remove trueEnd offset from SRC_IF_ELSE and SRC_COND. draft
authorTooru Fujisawa <arai_a@mac.com>
Wed, 16 May 2018 16:02:40 +0900
changeset 1512707 80ff5c465604b146f0538765d92b5fcd900797d8
parent 1512545 380cf87c1ee3966dd94499942b73085754dc4824
child 1512708 c4917cdbe01f22617a158584fd7c7b79958ed239
push id271863
push userarai_a@mac.com
push dateWed, 16 May 2018 07:03:38 +0000
treeherdertry@c4917cdbe01f [default view] [failures only]
bugs1461888
milestone62.0a1
Bug 1461888 - Remove trueEnd offset from SRC_IF_ELSE and SRC_COND.
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/SourceNotes.h
js/src/jit/IonControlFlow.cpp
js/src/shell/js.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -1957,19 +1957,16 @@ class MOZ_STACK_CLASS TryEmitter
 //
 class MOZ_STACK_CLASS IfThenElseEmitter
 {
     BytecodeEmitter* bce_;
 
     JumpList jumpAroundThen_;
     JumpList jumpsAroundElse_;
 
-    // The source note index for SRC_IF, SRC_IF_ELSE, or SRC_COND.
-    unsigned noteIndex_;
-
     // The stack depth before emitting the then block.
     // Used for restoring stack depth before emitting the else block.
     // Also used for assertion to make sure then and else blocks pushed the
     // same number of values.
     int32_t thenDepth_;
 
 #ifdef DEBUG
     // The number of values pushed in the then and else blocks.
@@ -2009,17 +2006,16 @@ class MOZ_STACK_CLASS IfThenElseEmitter
         // After calling emitEnd.
         End
     };
     State state_;
 
   public:
     explicit IfThenElseEmitter(BytecodeEmitter* bce)
       : bce_(bce),
-        noteIndex_(-1),
         thenDepth_(0),
 #ifdef DEBUG
         pushed_(0),
         calculatedPushed_(false),
 #endif
         state_(Start)
     {}
 
@@ -2032,17 +2028,17 @@ class MOZ_STACK_CLASS IfThenElseEmitter
         MOZ_ASSERT(nextState == If || nextState == IfElse || nextState == Cond);
 
         // Clear jumpAroundThen_ offset that points previous JSOP_IFEQ.
         if (state_ == Else)
             jumpAroundThen_ = JumpList();
 
         // Emit an annotated branch-if-false around the then part.
         SrcNoteType type = nextState == If ? SRC_IF : nextState == IfElse ? SRC_IF_ELSE : SRC_COND;
-        if (!bce_->newSrcNote(type, &noteIndex_))
+        if (!bce_->newSrcNote(type))
             return false;
         if (!bce_->emitJump(JSOP_IFEQ, &jumpAroundThen_))
             return false;
 
         // To restore stack depth in else part, save depth of the then part.
 #ifdef DEBUG
         // If DEBUG, this is also necessary to calculate |pushed_|.
         thenDepth_ = bce_->stackDepth;
@@ -2088,27 +2084,16 @@ class MOZ_STACK_CLASS IfThenElseEmitter
         // the offset with jumpsAroundElse value.
         if (!bce_->emitJump(JSOP_GOTO, &jumpsAroundElse_))
             return false;
 
         // Ensure the branch-if-false comes here, then emit the else.
         if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_))
             return false;
 
-        // Annotate SRC_IF_ELSE or SRC_COND with the offset from branch to
-        // jump, for IonMonkey's benefit.  We can't just "back up" from the pc
-        // of the else clause, because we don't know whether an extended
-        // jump was required to leap from the end of the then clause over
-        // the else clause.
-        if (!bce_->setSrcNoteOffset(noteIndex_, 0,
-                                    jumpsAroundElse_.offset - jumpAroundThen_.offset))
-        {
-            return false;
-        }
-
         // Restore stack depth of the then part.
         bce_->stackDepth = thenDepth_;
         state_ = Else;
         return true;
     }
 
     bool emitEnd() {
         MOZ_ASSERT(state_ == If || state_ == Else);
--- a/js/src/frontend/SourceNotes.h
+++ b/js/src/frontend/SourceNotes.h
@@ -32,18 +32,18 @@ namespace js {
  * SRC_COLSPAN, SRC_SETLINE, and SRC_XDELTA) applies to a given bytecode.
  *
  * NB: the js_SrcNoteSpec array in BytecodeEmitter.cpp is indexed by this
  * enum, so its initializers need to match the order here.
  */
 #define FOR_EACH_SRC_NOTE_TYPE(M)                                                                  \
     M(SRC_NULL,         "null",        0)  /* Terminates a note vector. */                         \
     M(SRC_IF,           "if",          0)  /* JSOP_IFEQ bytecode is from an if-then. */            \
-    M(SRC_IF_ELSE,      "if-else",     1)  /* JSOP_IFEQ bytecode is from an if-then-else. */       \
-    M(SRC_COND,         "cond",        1)  /* JSOP_IFEQ is from conditional ?: operator. */        \
+    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",         3)  /* JSOP_NOP or JSOP_POP in for(;;) loop head. */        \
     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_FOR_IN,       "for-in",      1)  /* JSOP_GOTO to for-in loop condition from before       \
                                               loop. */                                             \
     M(SRC_FOR_OF,       "for-of",      1)  /* JSOP_GOTO to for-of loop condition from before       \
                                               loop. */                                             \
     M(SRC_CONTINUE,     "continue",    0)  /* JSOP_GOTO is a continue. */                          \
--- a/js/src/jit/IonControlFlow.cpp
+++ b/js/src/jit/IonControlFlow.cpp
@@ -1757,46 +1757,46 @@ ControlFlowGenerator::processIfStart(JSO
     CFGBlock* ifFalse = CFGBlock::New(alloc(), falseStart);
 
     CFGTest* test = CFGTest::New(alloc(), ifTrue, ifFalse);
     current->setStopIns(test);
     current->setStopPc(pc);
 
     // The bytecode for if/ternary gets emitted either like this:
     //
-    //    IFEQ X  ; src note (IF_ELSE, COND) points to the GOTO
+    //    IFEQ X     ; src note (IF_ELSE, COND)
     //    ...
     //    GOTO Z
-    // X: ...     ; else/else if
+    // X: JUMPTARGET ; else/else if
     //    ...
-    // Z:         ; join
+    // Z: JUMPTARGET ; join
     //
     // Or like this:
     //
-    //    IFEQ X  ; src note (IF) has no offset
+    //    IFEQ X     ; src note (IF)
     //    ...
-    // Z: ...     ; join
+    // Z: JUMPTARGET ; join
     //
     // We want to parse the bytecode as if we were parsing the AST, so for the
-    // IF_ELSE/COND cases, we use the source note and follow the GOTO. For the
-    // IF case, the IFEQ offset is the join point.
+    // IF_ELSE/COND cases, we use the IFEQ/GOTO bytecode offsets to follow the
+    // branch. For the IF case, the IFEQ offset is the join point.
     switch (SN_TYPE(sn)) {
       case SRC_IF:
         if (!cfgStack_.append(CFGState::If(falseStart, test)))
             return ControlStatus::Error;
         break;
 
       case SRC_IF_ELSE:
       case SRC_COND:
       {
         // Infer the join point from the JSOP_GOTO[X] sitting here, then
         // assert as we much we can that this is the right GOTO.
-        jsbytecode* trueEnd = pc + GetSrcNoteOffset(sn, 0);
+        MOZ_ASSERT(JSOp(*falseStart) == JSOP_JUMPTARGET);
+        jsbytecode* trueEnd = falseStart - JSOP_GOTO_LENGTH;
         MOZ_ASSERT(trueEnd > pc);
-        MOZ_ASSERT(trueEnd < falseStart);
         MOZ_ASSERT(JSOp(*trueEnd) == JSOP_GOTO);
         MOZ_ASSERT(!GetSrcNote(gsn, script, trueEnd));
 
         jsbytecode* falseEnd = trueEnd + GetJumpOffset(trueEnd);
         MOZ_ASSERT(falseEnd > trueEnd);
         MOZ_ASSERT(falseEnd >= falseStart);
 
         if (!cfgStack_.append(CFGState::IfElse(trueEnd, falseEnd, test)))
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2688,16 +2688,18 @@ SrcNotes(JSContext* cx, HandleScript scr
                           unsigned(sn - notes), lineno, offset, delta, name))
         {
             return false;
         }
 
         switch (type) {
           case SRC_NULL:
           case SRC_IF:
+          case SRC_IF_ELSE:
+          case SRC_COND:
           case SRC_CONTINUE:
           case SRC_BREAK:
           case SRC_BREAK2LABEL:
           case SRC_SWITCHBREAK:
           case SRC_ASSIGNOP:
           case SRC_XDELTA:
             break;
 
@@ -2722,28 +2724,22 @@ SrcNotes(JSContext* cx, HandleScript scr
                               unsigned(GetSrcNoteOffset(sn, 0)),
                               unsigned(GetSrcNoteOffset(sn, 1)),
                               unsigned(GetSrcNoteOffset(sn, 2))))
             {
                 return false;
             }
             break;
 
-          case SRC_IF_ELSE:
-            if (!sp->jsprintf(" else %u", unsigned(GetSrcNoteOffset(sn, 0))))
-                return false;
-            break;
-
           case SRC_FOR_IN:
           case SRC_FOR_OF:
             if (!sp->jsprintf(" closingjump %u", unsigned(GetSrcNoteOffset(sn, 0))))
                 return false;
             break;
 
-          case SRC_COND:
           case SRC_WHILE:
           case SRC_NEXTCASE:
             if (!sp->jsprintf(" offset %u", unsigned(GetSrcNoteOffset(sn, 0))))
                 return false;
             break;
 
           case SRC_TABLESWITCH: {
             JSOp op = JSOp(script->code()[offset]);