Backout consolidation of pending arrays for switch targets, bug 730888. r=dvander
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 01 Mar 2012 17:48:32 -0800
changeset 88112 fbd0cb4228d7000fd243022295b80ab210ea228a
parent 88111 09799386e12d6d9d0f8737c3bf2764a573bed3eb
child 88113 f622f39e4296e19d7a6f08fa28224a00d3eb8378
push id6689
push userbhackett@mozilla.com
push dateFri, 02 Mar 2012 01:48:52 +0000
treeherdermozilla-inbound@fbd0cb4228d7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs730888
milestone13.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
Backout consolidation of pending arrays for switch targets, bug 730888. r=dvander
js/src/jit-test/tests/basic/bug730888.js
js/src/jsanalyze.cpp
js/src/jsanalyze.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug730888.js
@@ -0,0 +1,14 @@
+
+(function() {
+    for (var i = 0; i < 64; ++i) {
+        var name;
+        switch (this) {
+            case 0: name = 'firstAttr'; break;
+            case 1: name = 'secondAttr'; 
+            case 2: name = 'thirdAttr'; break;
+        }
+        switch (name) {
+          case 'firstAttr': assertEq(result, 'value'); break;
+        }
+    }
+})();
--- a/js/src/jsanalyze.cpp
+++ b/js/src/jsanalyze.cpp
@@ -1311,16 +1311,17 @@ ScriptAnalysis::analyzeSSA(JSContext *cx
              * Make phi nodes and update state for slots which are already in
              * pending from previous branches to the loop head, and which are
              * modified in the body of the loop.
              */
             for (unsigned i = 0; i < pending->length(); i++) {
                 SlotValue &v = (*pending)[i];
                 if (v.slot < numSlots && liveness(v.slot).firstWrite(code->loop) != UINT32_MAX) {
                     if (v.value.kind() != SSAValue::PHI || v.value.phiOffset() != offset) {
+                        JS_ASSERT(v.value.phiOffset() < offset);
                         SSAValue ov = v.value;
                         if (!makePhi(cx, v.slot, offset, &ov))
                             return;
                         insertPhi(cx, ov, v.value);
                         v.value = ov;
                     }
                 }
                 if (code->fallthrough || code->jumpFallthrough)
@@ -1519,52 +1520,42 @@ ScriptAnalysis::analyzeSSA(JSContext *cx
           case JSOP_TABLESWITCH: {
             unsigned defaultOffset = offset + GET_JUMP_OFFSET(pc);
             jsbytecode *pc2 = pc + JUMP_OFFSET_LEN;
             jsint low = GET_JUMP_OFFSET(pc2);
             pc2 += JUMP_OFFSET_LEN;
             jsint high = GET_JUMP_OFFSET(pc2);
             pc2 += JUMP_OFFSET_LEN;
 
-            Vector<SlotValue> *pending = NULL;
-            uint32_t pendingOffset = 0;
-
             for (jsint i = low; i <= high; i++) {
                 unsigned targetOffset = offset + GET_JUMP_OFFSET(pc2);
                 if (targetOffset != offset)
-                    checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth,
-                                      &pending, &pendingOffset);
+                    checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth);
                 pc2 += JUMP_OFFSET_LEN;
             }
 
-            checkBranchTarget(cx, defaultOffset, branchTargets, values, stackDepth,
-                              &pending, &pendingOffset);
+            checkBranchTarget(cx, defaultOffset, branchTargets, values, stackDepth);
             break;
           }
 
           case JSOP_LOOKUPSWITCH: {
             unsigned defaultOffset = offset + GET_JUMP_OFFSET(pc);
             jsbytecode *pc2 = pc + JUMP_OFFSET_LEN;
             unsigned npairs = GET_UINT16(pc2);
             pc2 += UINT16_LEN;
 
-            Vector<SlotValue> *pending = NULL;
-            uint32_t pendingOffset = 0;
-
             while (npairs) {
                 pc2 += UINT32_INDEX_LEN;
                 unsigned targetOffset = offset + GET_JUMP_OFFSET(pc2);
-                checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth,
-                                  &pending, &pendingOffset);
+                checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth);
                 pc2 += JUMP_OFFSET_LEN;
                 npairs--;
             }
 
-            checkBranchTarget(cx, defaultOffset, branchTargets, values, stackDepth,
-                              &pending, &pendingOffset);
+            checkBranchTarget(cx, defaultOffset, branchTargets, values, stackDepth);
             break;
           }
 
           case JSOP_TRY: { 
             JSTryNote *tn = script->trynotes()->vector;
             JSTryNote *tnlimit = tn + script->trynotes()->length;
             for (; tn < tnlimit; tn++) {
                 unsigned startOffset = script->mainOffset + tn->start;
@@ -1691,25 +1682,26 @@ inline void
 ScriptAnalysis::mergeValue(JSContext *cx, uint32_t offset, const SSAValue &v, SlotValue *pv)
 {
     /* Make sure that v is accounted for in the pending value or phi value at pv. */
     JS_ASSERT(v.kind() != SSAValue::EMPTY && pv->value.kind() != SSAValue::EMPTY);
 
     if (v == pv->value)
         return;
 
-    if (pv->value.kind() != SSAValue::PHI || pv->value.phiOffset() != offset) {
+    if (pv->value.kind() != SSAValue::PHI || pv->value.phiOffset() < offset) {
         SSAValue ov = pv->value;
         if (makePhi(cx, pv->slot, offset, &pv->value)) {
             insertPhi(cx, pv->value, v);
             insertPhi(cx, pv->value, ov);
         }
         return;
     }
 
+    JS_ASSERT(pv->value.phiOffset() == offset);
     insertPhi(cx, pv->value, v);
 }
 
 void
 ScriptAnalysis::checkPendingValue(JSContext *cx, const SSAValue &v, uint32_t slot,
                                   Vector<SlotValue> *pending)
 {
     JS_ASSERT(v.kind() != SSAValue::EMPTY);
@@ -1721,53 +1713,38 @@ ScriptAnalysis::checkPendingValue(JSCont
 
     if (!pending->append(SlotValue(slot, v)))
         setOOM(cx);
 }
 
 void
 ScriptAnalysis::checkBranchTarget(JSContext *cx, uint32_t targetOffset,
                                   Vector<uint32_t> &branchTargets,
-                                  SSAValueInfo *values, uint32_t stackDepth,
-                                  Vector<SlotValue> **ppending, uint32_t *ppendingOffset)
+                                  SSAValueInfo *values, uint32_t stackDepth)
 {
     unsigned targetDepth = getCode(targetOffset).stackDepth;
     JS_ASSERT(targetDepth <= stackDepth);
 
     /*
      * If there is already an active branch to target, make sure its pending
      * values reflect any changes made since the first branch. Otherwise, add a
      * new pending branch and determine its pending values lazily.
      */
     Vector<SlotValue> *&pending = getCode(targetOffset).pendingValues;
     if (pending) {
         for (unsigned i = 0; i < pending->length(); i++) {
             SlotValue &v = (*pending)[i];
             mergeValue(cx, targetOffset, values[v.slot].v, &v);
         }
     } else {
-        if (ppending && *ppending) {
-            JS_ASSERT(*ppendingOffset != targetOffset);
-            pending = *ppending;
-            getCode(Min(targetOffset, *ppendingOffset)).switchSharesPending = true;
-        } else {
-            pending = cx->new_< Vector<SlotValue> >(cx);
-            if (!pending) {
-                setOOM(cx);
-                return;
-            }
-        }
-        if (!branchTargets.append(targetOffset)) {
+        pending = cx->new_< Vector<SlotValue> >(cx);
+        if (!pending || !branchTargets.append(targetOffset)) {
             setOOM(cx);
             return;
         }
-        if (ppending) {
-            *ppending = pending;
-            *ppendingOffset = Max(targetOffset, *ppendingOffset);
-        }
     }
 
     /*
      * Make sure there is a pending entry for each value on the stack.
      * The number of stack entries at join points is usually zero, and
      * we don't want to look at the active branches while popping and
      * pushing values in each opcode.
      */
@@ -1817,23 +1794,16 @@ ScriptAnalysis::mergeBranchTarget(JSCont
      * the variable was written.
      */
     for (int i = branchTargets.length() - 1; i >= value.branchSize; i--) {
         if (branchTargets[i] <= currentOffset)
             continue;
 
         const Bytecode &code = getCode(branchTargets[i]);
 
-        /*
-         * If the pending array for this offset is shared with a later branch
-         * target, it will be updated when that offset is handled.
-         */
-        if (code.switchSharesPending)
-            continue;
-
         Vector<SlotValue> *pending = code.pendingValues;
         checkPendingValue(cx, value.v, slot, pending);
     }
 
     value.branchSize = branchTargets.length();
 }
 
 void
@@ -1888,34 +1858,32 @@ ScriptAnalysis::freezeNewValues(JSContex
 {
     Bytecode &code = getCode(offset);
 
     Vector<SlotValue> *pending = code.pendingValues;
     code.pendingValues = NULL;
 
     unsigned count = pending->length();
     if (count == 0) {
-        if (!code.switchSharesPending)
-            cx->delete_(pending);
+        cx->delete_(pending);
         return;
     }
 
     code.newValues = cx->typeLifoAlloc().newArray<SlotValue>(count + 1);
     if (!code.newValues) {
         setOOM(cx);
         return;
     }
 
     for (unsigned i = 0; i < count; i++)
         code.newValues[i] = (*pending)[i];
     code.newValues[count].slot = 0;
     code.newValues[count].value.clear();
 
-    if (!code.switchSharesPending)
-        cx->delete_(pending);
+    cx->delete_(pending);
 }
 
 CrossSSAValue
 CrossScriptSSA::foldValue(const CrossSSAValue &cv)
 {
     const Frame &frame = getFrame(cv.frame);
     const SSAValue &v = cv.v;
 
--- a/js/src/jsanalyze.h
+++ b/js/src/jsanalyze.h
@@ -137,22 +137,16 @@ class Bytecode
     /*
      * Dynamically observed state about the execution of this opcode. These are
      * hints about the script for use during compilation.
      */
     bool arrayWriteHole: 1;  /* SETELEM which has written to an array hole. */
     bool getStringElement:1; /* GETELEM which has accessed string properties. */
     bool accessGetter: 1;    /* Property read on a shape with a getter hook. */
 
-    /*
-     * Switch target other than the last one, which shares its pending values
-     * with a later offset during SSA analysis.
-     */
-    bool switchSharesPending : 1;
-
     /* Stack depth before this opcode. */
     uint32_t stackDepth;
 
   private:
 
     union {
         /* If this is a JOF_TYPESET opcode, index into the observed types for the op. */
         types::TypeSet *observedTypes;
@@ -1208,18 +1202,17 @@ class ScriptAnalysis
 
     /* SSA helpers */
     bool makePhi(JSContext *cx, uint32_t slot, uint32_t offset, SSAValue *pv);
     void insertPhi(JSContext *cx, SSAValue &phi, const SSAValue &v);
     void mergeValue(JSContext *cx, uint32_t offset, const SSAValue &v, SlotValue *pv);
     void checkPendingValue(JSContext *cx, const SSAValue &v, uint32_t slot,
                            Vector<SlotValue> *pending);
     void checkBranchTarget(JSContext *cx, uint32_t targetOffset, Vector<uint32_t> &branchTargets,
-                           SSAValueInfo *values, uint32_t stackDepth,
-                           Vector<SlotValue> **ppending = NULL, uint32_t *ppendingOffset = NULL);
+                           SSAValueInfo *values, uint32_t stackDepth);
     void checkExceptionTarget(JSContext *cx, uint32_t catchOffset,
                               Vector<uint32_t> &exceptionTargets);
     void mergeBranchTarget(JSContext *cx, SSAValueInfo &value, uint32_t slot,
                            const Vector<uint32_t> &branchTargets, uint32_t currentOffset);
     void mergeExceptionTarget(JSContext *cx, const SSAValue &value, uint32_t slot,
                               const Vector<uint32_t> &exceptionTargets);
     void mergeAllExceptionTargets(JSContext *cx, SSAValueInfo *values,
                                   const Vector<uint32_t> &exceptionTargets);