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 88325 fbd0cb4228d7000fd243022295b80ab210ea228a
parent 88324 09799386e12d6d9d0f8737c3bf2764a573bed3eb
child 88326 f622f39e4296e19d7a6f08fa28224a00d3eb8378
push id157
push userMs2ger@gmail.com
push dateWed, 07 Mar 2012 19:27:10 +0000
reviewersdvander
bugs730888
milestone13.0a1
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);