Bug 1308383 - Part 4: Use IfThenElseEmitter in BytecodeEmitter::emitConditionalExpression. r=shu
authorTooru Fujisawa <arai_a@mac.com>
Tue, 01 Nov 2016 16:53:50 +0900
changeset 347248 5f884840b536fb4ce6d09ccfddec74d2b3a9d314
parent 347247 f8649b3008e1bd588b48156a5496c7b761be311a
child 347249 83472f5679d4c349c01b546c50cf1ca8ea004ee8
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1308383
milestone52.0a1
Bug 1308383 - Part 4: Use IfThenElseEmitter in BytecodeEmitter::emitConditionalExpression. r=shu
js/src/frontend/BytecodeEmitter.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4519,16 +4519,17 @@ class MOZ_STACK_CLASS IfThenElseEmitter
     BytecodeEmitter* bce_;
     JumpList jumpAroundThen_;
     JumpList jumpsAroundElse_;
     unsigned noteIndex_;
     int32_t thenDepth_;
     enum State {
         Start,
         If,
+        Cond,
         IfElse,
         Else,
         End
     };
     State state_;
 
   public:
     explicit IfThenElseEmitter(BytecodeEmitter* bce)
@@ -4539,59 +4540,64 @@ class MOZ_STACK_CLASS IfThenElseEmitter
     {}
 
     ~IfThenElseEmitter()
     {}
 
   private:
     bool emitIf(State nextState) {
         MOZ_ASSERT(state_ == Start || state_ == Else);
-        MOZ_ASSERT(nextState == If || nextState == IfElse);
+        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.
-        if (!bce_->newSrcNote(nextState == If ? SRC_IF : SRC_IF_ELSE, &noteIndex_))
+        SrcNoteType type = nextState == If ? SRC_IF : nextState == IfElse ? SRC_IF_ELSE : SRC_COND;
+        if (!bce_->newSrcNote(type, &noteIndex_))
             return false;
         if (!bce_->emitJump(JSOP_IFEQ, &jumpAroundThen_))
             return false;
 
         // To restore stack depth in else part, save depth of the then part.
-        if (nextState == IfElse)
+        if (nextState == IfElse || nextState == Cond)
             thenDepth_ = bce_->stackDepth;
         state_ = nextState;
         return true;
     }
 
   public:
     bool emitIf() {
         return emitIf(If);
     }
 
+    bool emitCond() {
+        return emitIf(Cond);
+    }
+
     bool emitIfElse() {
         return emitIf(IfElse);
     }
 
     bool emitElse() {
-        MOZ_ASSERT(state_ == IfElse);
+        MOZ_ASSERT(state_ == IfElse || state_ == Cond);
 
         // Emit a jump from the end of our then part around the else part. The
         // patchJumpsToTarget call at the bottom of this function will fix up
         // 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 with the offset from branch to jump, for
-        // IonMonkey's benefit.  We can't just "back up" from the pc
+        // 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;
         }
@@ -8397,50 +8403,30 @@ BytecodeEmitter::emitLabeledStatement(co
 
 bool
 BytecodeEmitter::emitConditionalExpression(ConditionalExpression& conditional)
 {
     /* Emit the condition, then branch if false to the else part. */
     if (!emitTree(&conditional.condition()))
         return false;
 
-    unsigned noteIndex;
-    if (!newSrcNote(SRC_COND, &noteIndex))
-        return false;
-
-    JumpList beq;
-    if (!emitJump(JSOP_IFEQ, &beq))
+    IfThenElseEmitter ifThenElse(this);
+    if (!ifThenElse.emitCond())
         return false;
 
     if (!emitConditionallyExecutedTree(&conditional.thenExpression()))
         return false;
 
-    /* Jump around else, fixup the branch, emit else, fixup jump. */
-    JumpList jmp;
-    if (!emitJump(JSOP_GOTO, &jmp))
-        return false;
-    if (!emitJumpTargetAndPatch(beq))
-        return false;
-    if (!setSrcNoteOffset(noteIndex, 0, jmp.offset - beq.offset))
-        return false;
-
-    /*
-     * Because each branch pushes a single value, but our stack budgeting
-     * analysis ignores branches, we now have to adjust this->stackDepth to
-     * ignore the value pushed by the first branch.  Execution will follow
-     * only one path, so we must decrement this->stackDepth.
-     *
-     * Failing to do this will foil code, such as let block code generation,
-     * which must use the stack depth to compute local stack indexes correctly.
-     */
-    MOZ_ASSERT(stackDepth > 0);
-    stackDepth--;
+    if (!ifThenElse.emitElse())
+        return false;
+
     if (!emitConditionallyExecutedTree(&conditional.elseExpression()))
         return false;
-    if (!emitJumpTargetAndPatch(jmp))
+
+    if (!ifThenElse.emitEnd())
         return false;
 
     return true;
 }
 
 bool
 BytecodeEmitter::emitPropertyList(ParseNode* pn, MutableHandlePlainObject objp, PropListType type)
 {