Bug 684384 - Ion-compile break-to-labeled-scope. r=dvander
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 15 Feb 2013 14:52:29 +0100
changeset 122034 7dd9c9b874f54d9c6d05b859f453f94680614dff
parent 122033 6bb9636d8f6899c60d720298cdbca73c18cac7bc
child 122035 d999c65753ea3c6ea49d130f9a24f067bbe33704
push id24315
push userryanvm@gmail.com
push dateFri, 15 Feb 2013 21:34:37 +0000
treeherdermozilla-central@7bd555e2acfa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs684384
milestone21.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 684384 - Ion-compile break-to-labeled-scope. r=dvander
js/src/ion/IonBuilder.cpp
js/src/ion/IonBuilder.h
js/src/jit-test/tests/ion/bug684384.js
--- a/js/src/ion/IonBuilder.cpp
+++ b/js/src/ion/IonBuilder.cpp
@@ -228,16 +228,18 @@ IonBuilder::canInlineTarget(JSFunction *
     return true;
 }
 
 void
 IonBuilder::popCfgStack()
 {
     if (cfgStack_.back().isLoop())
         loops_.popBack();
+    if (cfgStack_.back().state == CFGState::LABEL)
+        labels_.popBack();
     cfgStack_.popBack();
 }
 
 bool
 IonBuilder::pushLoop(CFGState::State initial, jsbytecode *stopAt, MBasicBlock *entry,
                      jsbytecode *bodyStart, jsbytecode *bodyEnd, jsbytecode *exitpc,
                      jsbytecode *continuepc)
 {
@@ -769,17 +771,17 @@ IonBuilder::inspectOpcode(JSOp op)
       case JSOP_LOOPENTRY:
         insertRecompileCheck();
         return true;
 
       case JSOP_NOP:
         return true;
 
       case JSOP_LABEL:
-        return true;
+        return jsop_label();
 
       case JSOP_UNDEFINED:
         return pushConstant(UndefinedValue());
 
       case JSOP_IFEQ:
         return jsop_ifeq(JSOP_IFEQ);
 
       case JSOP_CONDSWITCH:
@@ -1194,16 +1196,19 @@ IonBuilder::processCfgEntry(CFGState &st
         return processCondSwitchCase(state);
 
       case CFGState::COND_SWITCH_BODY:
         return processCondSwitchBody(state);
 
       case CFGState::AND_OR:
         return processAndOrEnd(state);
 
+      case CFGState::LABEL:
+        return processLabelEnd(state);
+
       default:
         JS_NOT_REACHED("unknown cfgstate");
     }
     return ControlStatus_Error;
 }
 
 IonBuilder::ControlStatus
 IonBuilder::processIfEnd(CFGState &state)
@@ -1623,55 +1628,77 @@ IonBuilder::processAndOrEnd(CFGState &st
 
     current = state.branch.ifFalse;
     graph().moveBlockToEnd(current);
     pc = current->pc();
     return ControlStatus_Joined;
 }
 
 IonBuilder::ControlStatus
+IonBuilder::processLabelEnd(CFGState &state)
+{
+    JS_ASSERT(state.state == CFGState::LABEL);
+
+    // If there are no breaks and no current, controlflow is terminated.
+    if (!state.label.breaks && !current)
+        return ControlStatus_Ended;
+
+    // If there are no breaks to this label, there's nothing to do.
+    if (!state.label.breaks)
+        return ControlStatus_Joined;
+
+    MBasicBlock *successor = createBreakCatchBlock(state.label.breaks, state.stopAt);
+    if (!successor)
+        return ControlStatus_Error;
+
+    if (current) {
+        current->end(MGoto::New(successor));
+        successor->addPredecessor(current);
+    }
+
+    pc = state.stopAt;
+    current = successor;
+    return ControlStatus_Joined;
+}
+
+IonBuilder::ControlStatus
 IonBuilder::processBreak(JSOp op, jssrcnote *sn)
 {
     JS_ASSERT(op == JSOP_GOTO);
 
-    // Find the target loop.
-    CFGState *found = NULL;
+    JS_ASSERT(SN_TYPE(sn) == SRC_BREAK ||
+              SN_TYPE(sn) == SRC_BREAK2LABEL);
+
+    // Find the break target.
     jsbytecode *target = pc + GetJumpOffset(pc);
-    for (size_t i = loops_.length() - 1; i < loops_.length(); i--) {
-        CFGState &cfg = cfgStack_[loops_[i].cfgEntry];
-        if (cfg.loop.exitpc == target) {
-            found = &cfg;
-            break;
+    DebugOnly<bool> found = false;
+
+    if (SN_TYPE(sn) == SRC_BREAK2LABEL) {
+        for (size_t i = labels_.length() - 1; i < labels_.length(); i--) {
+            CFGState &cfg = cfgStack_[labels_[i].cfgEntry];
+            JS_ASSERT(cfg.state == CFGState::LABEL);
+            if (cfg.stopAt == target) {
+                cfg.label.breaks = new DeferredEdge(current, cfg.label.breaks);
+                found = true;
+                break;
+            }
         }
-    }
-
-    if (!found) {
-        // Sometimes, we can't determine the structure of a labeled break. For
-        // example:
-        //
-        // 0:    label: {
-        // 1:        for (;;) {
-        // 2:            break label;
-        // 3:        }
-        // 4:        stuff;
-        // 5:    }
-        //
-        // In this case, the successor of the block is 4, but the target of the
-        // single-level break is actually 5. To recognize this case we'd need
-        // to know about the label structure at 0,5 ahead of time - and lacking
-        // those source notes for now, we just abort instead.
-        abort("could not find the target of a break");
-        return ControlStatus_Error;
-    }
-
-    // There must always be a valid target loop structure. If not, there's
-    // probably an off-by-something error in which pc we track.
-    CFGState &state = *found;
-
-    state.loop.breaks = new DeferredEdge(current, state.loop.breaks);
+    } else {
+        for (size_t i = loops_.length() - 1; i < loops_.length(); i--) {
+            CFGState &cfg = cfgStack_[loops_[i].cfgEntry];
+            JS_ASSERT(cfg.isLoop());
+            if (cfg.loop.exitpc == target) {
+                cfg.loop.breaks = new DeferredEdge(current, cfg.loop.breaks);
+                found = true;
+                break;
+            }
+        }
+    }
+
+    JS_ASSERT(found);
 
     current = NULL;
     pc += js_CodeSpec[op].length;
     return processControlEnd();
 }
 
 static inline jsbytecode *
 EffectiveContinue(jsbytecode *pc)
@@ -2177,16 +2204,31 @@ IonBuilder::tableSwitch(JSOp op, jssrcno
     if (!cfgStack_.append(state))
         return ControlStatus_Error;
 
     pc = current->pc();
     return ControlStatus_Jumped;
 }
 
 bool
+IonBuilder::jsop_label()
+{
+    JS_ASSERT(JSOp(*pc) == JSOP_LABEL);
+
+    jsbytecode *endpc = pc + GET_JUMP_OFFSET(pc);
+    JS_ASSERT(endpc > pc);
+
+    ControlFlowInfo label(cfgStack_.length(), endpc);
+    if (!labels_.append(label))
+        return false;
+
+    return cfgStack_.append(CFGState::Label(endpc));
+}
+
+bool
 IonBuilder::jsop_condswitch()
 {
     // CondSwitch op looks as follows:
     //   condswitch [length +exit_pc; first case offset +next-case ]
     //   {
     //     {
     //       ... any code ...
     //       case (+jump) [pcdelta offset +next-case]
@@ -2273,16 +2315,26 @@ IonBuilder::CFGState::CondSwitch(jsbytec
     state.condswitch.currentIdx = 0;
     state.condswitch.defaultTarget = defaultTarget;
     state.condswitch.defaultIdx = uint32_t(-1);
     state.condswitch.exitpc = exitpc;
     state.condswitch.breaks = NULL;
     return state;
 }
 
+IonBuilder::CFGState
+IonBuilder::CFGState::Label(jsbytecode *exitpc)
+{
+    CFGState state;
+    state.state = LABEL;
+    state.stopAt = exitpc;
+    state.label.breaks = NULL;
+    return state;
+}
+
 IonBuilder::ControlStatus
 IonBuilder::processCondSwitchCase(CFGState &state)
 {
     JS_ASSERT(state.state == CFGState::COND_SWITCH_CASE);
     JS_ASSERT(!state.condswitch.breaks);
     JS_ASSERT(current);
     JS_ASSERT(JSOp(*pc) == JSOP_CASE);
     FixedList<MBasicBlock *> &bodies = *state.condswitch.bodies;
--- a/js/src/ion/IonBuilder.h
+++ b/js/src/ion/IonBuilder.h
@@ -68,17 +68,18 @@ class IonBuilder : public MIRGenerator
             WHILE_LOOP_COND,    // while (x) { }
             WHILE_LOOP_BODY,    // while () { x }
             FOR_LOOP_COND,      // for (; x;) { }
             FOR_LOOP_BODY,      // for (; ;) { x }
             FOR_LOOP_UPDATE,    // for (; ; x) { }
             TABLE_SWITCH,       // switch() { x }
             COND_SWITCH_CASE,   // switch() { case X: ... }
             COND_SWITCH_BODY,   // switch() { case ...: X }
-            AND_OR              // && x, || x
+            AND_OR,             // && x, || x
+            LABEL               // label: x
         };
 
         State state;            // Current state of this control structure.
         jsbytecode *stopAt;     // Bytecode at which to stop the processing loop.
 
         // For if structures, this contains branch information.
         union {
             struct {
@@ -135,16 +136,19 @@ class IonBuilder : public MIRGenerator
                 // Remember the block index of the default case.
                 jsbytecode *defaultTarget;
                 uint32_t defaultIdx;
 
                 // Block immediately after the switch.
                 jsbytecode *exitpc;
                 DeferredEdge *breaks;
             } condswitch;
+            struct {
+                DeferredEdge *breaks;
+            } label;
         };
 
         inline bool isLoop() const {
             switch (state) {
               case DO_WHILE_LOOP_COND:
               case DO_WHILE_LOOP_BODY:
               case WHILE_LOOP_COND:
               case WHILE_LOOP_BODY:
@@ -157,16 +161,17 @@ class IonBuilder : public MIRGenerator
             }
         }
 
         static CFGState If(jsbytecode *join, MBasicBlock *ifFalse);
         static CFGState IfElse(jsbytecode *trueEnd, jsbytecode *falseEnd, MBasicBlock *ifFalse);
         static CFGState AndOr(jsbytecode *join, MBasicBlock *joinStart);
         static CFGState TableSwitch(jsbytecode *exitpc, MTableSwitch *ins);
         static CFGState CondSwitch(jsbytecode *exitpc, jsbytecode *defaultTarget);
+        static CFGState Label(jsbytecode *exitpc);
     };
 
     static int CmpSuccessors(const void *a, const void *b);
 
   public:
     IonBuilder(JSContext *cx, TempAllocator *temp, MIRGraph *graph,
                TypeOracle *oracle, CompileInfo *info, size_t inliningDepth = 0, uint32_t loopDepth = 0);
 
@@ -209,16 +214,17 @@ class IonBuilder : public MIRGenerator
     ControlStatus processForBodyEnd(CFGState &state);
     ControlStatus processForUpdateEnd(CFGState &state);
     ControlStatus processNextTableSwitchCase(CFGState &state);
     ControlStatus processCondSwitchCase(CFGState &state);
     ControlStatus processCondSwitchBody(CFGState &state);
     ControlStatus processSwitchBreak(JSOp op, jssrcnote *sn);
     ControlStatus processSwitchEnd(DeferredEdge *breaks, jsbytecode *exitpc);
     ControlStatus processAndOrEnd(CFGState &state);
+    ControlStatus processLabelEnd(CFGState &state);
     ControlStatus processReturn(JSOp op);
     ControlStatus processThrow();
     ControlStatus processContinue(JSOp op, jssrcnote *sn);
     ControlStatus processBreak(JSOp op, jssrcnote *sn);
     ControlStatus maybeLoop(JSOp op, jssrcnote *sn);
     bool pushLoop(CFGState::State state, jsbytecode *stopAt, MBasicBlock *entry,
                   jsbytecode *bodyStart, jsbytecode *bodyEnd, jsbytecode *exitpc,
                   jsbytecode *continuepc = NULL);
@@ -324,16 +330,17 @@ class IonBuilder : public MIRGenerator
     bool jsop_defvar(uint32_t index);
     bool jsop_deffun(uint32_t index);
     bool jsop_notearg();
     bool jsop_funcall(uint32_t argc);
     bool jsop_funapply(uint32_t argc);
     bool jsop_funapplyarguments(uint32_t argc);
     bool jsop_call(uint32_t argc, bool constructing);
     bool jsop_ifeq(JSOp op);
+    bool jsop_label();
     bool jsop_condswitch();
     bool jsop_andor(JSOp op);
     bool jsop_dup2();
     bool jsop_loophead(jsbytecode *pc);
     bool jsop_compare(JSOp op);
     bool jsop_getgname(HandlePropertyName name);
     bool jsop_setgname(HandlePropertyName name);
     bool jsop_getname(HandlePropertyName name);
@@ -504,16 +511,17 @@ class IonBuilder : public MIRGenerator
     jsbytecode *callerPC() {
         return callerResumePoint_ ? callerResumePoint_->pc() : NULL;
     }
     IonBuilder *callerBuilder_;
 
     Vector<CFGState, 8, IonAllocPolicy> cfgStack_;
     Vector<ControlFlowInfo, 4, IonAllocPolicy> loops_;
     Vector<ControlFlowInfo, 0, IonAllocPolicy> switches_;
+    Vector<ControlFlowInfo, 2, IonAllocPolicy> labels_;
     Vector<MInstruction *, 2, IonAllocPolicy> iterators_;
     TypeOracle *oracle;
 
     size_t inliningDepth;
     Vector<MDefinition *, 0, IonAllocPolicy> inlinedArguments_;
 
     // True if script->failedBoundsCheck is set for the current script or
     // an outer script.
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug684384.js
@@ -0,0 +1,58 @@
+// Labeled break tests.
+function f1() {
+    foo:
+    if ([1]) {
+	bar:
+	for (var i=0; i<100; i++) {
+	    if (i > 60)
+		break foo;
+	}
+	assertEq(0, 1);
+    }
+    assertEq(i, 61);
+    return true;
+}
+assertEq(f1(), true);
+
+// Label with no breaks.
+function f2() {
+    foo:
+    if ([1]) {
+	for (var i=0; i<100; i++) {
+	}
+    }
+    assertEq(i, 100);
+    return true;
+}
+assertEq(f2(), true);
+
+// No breaks and early return.
+function f3() {
+    foo: {
+	if (true) {
+	    for (var i=0; i<100; i++) {
+	    }
+	}
+	return false;
+    }
+    assertEq(i, 100);
+    return true;
+}
+assertEq(f3(), false);
+
+// Multiple breaks.
+function f4() {
+    foo: {
+	if (true) {
+	    for (var i=0; i<100; i++)
+		if (i > 70)
+		    break foo;
+	        if (i > 80)
+		    break foo;
+	}
+	break foo;
+    }
+    assertEq(i, 71);
+    return true;
+}
+assertEq(f4(), true);