[INFER] Don't maintain normal registers for known-double entries after branching to opcodes where the entry is not known as a double, bug 681006.
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 22 Aug 2011 18:11:58 -0700
changeset 76146 b9a48e6f870ef1612ea94146bd45072bbb707109
parent 76145 a30c64a27b4ae57764efd2b34086b3968ea17efe
child 76147 78a56e48dd3c55309866bf656115555705421fb7
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
bugs681006
milestone9.0a1
[INFER] Don't maintain normal registers for known-double entries after branching to opcodes where the entry is not known as a double, bug 681006.
js/src/jit-test/tests/jaeger/bug681006.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/Compiler.h
js/src/methodjit/FrameState.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug681006.js
@@ -0,0 +1,10 @@
+// |jit-test| error: ReferenceError
+function f0(p0,p1) {
+    var v3;
+    do {
+        p1 > v3
+        v3=1.7
+    } while (p1 * v0 > p0);
+    + v3;
+}
+f0(4105,8307);
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -112,17 +112,18 @@ mjit::Compiler::Compiler(JSContext *cx, 
 #if defined JS_POLYIC
     pics(CompilerAllocPolicy(cx, *thisFromCtor())), 
     getElemICs(CompilerAllocPolicy(cx, *thisFromCtor())),
     setElemICs(CompilerAllocPolicy(cx, *thisFromCtor())),
 #endif
     callPatches(CompilerAllocPolicy(cx, *thisFromCtor())),
     callSites(CompilerAllocPolicy(cx, *thisFromCtor())),
     doubleList(CompilerAllocPolicy(cx, *thisFromCtor())),
-    fixedDoubleEntries(CompilerAllocPolicy(cx, *thisFromCtor())),
+    fixedIntToDoubleEntries(CompilerAllocPolicy(cx, *thisFromCtor())),
+    fixedDoubleToAnyEntries(CompilerAllocPolicy(cx, *thisFromCtor())),
     jumpTables(CompilerAllocPolicy(cx, *thisFromCtor())),
     jumpTableOffsets(CompilerAllocPolicy(cx, *thisFromCtor())),
     loopEntries(CompilerAllocPolicy(cx, *thisFromCtor())),
     rootedObjects(CompilerAllocPolicy(cx, *thisFromCtor())),
     stubcc(cx, *thisFromCtor(), frame),
     debugMode_(cx->compartment->debugMode()),
 #if defined JS_TRACER
     addTraceHints(cx->traceJitEnabled),
@@ -1457,32 +1458,37 @@ mjit::Compiler::generateMethod()
 
         frame.setPC(PC);
         frame.setInTryBlock(opinfo->inTryBlock);
 
         if (fallthrough) {
             /*
              * If there is fallthrough from the previous opcode and we changed
              * any entries into doubles for a branch at that previous op,
-             * revert those entries into integers. Maintain an invariant that
-             * for any variables inferred to be integers, the compiler
-             * maintains them as integers, both for faster code inside
-             * basic blocks and for fewer conversions needed when branching.
+             * revert those entries into integers. Similarly, if we forgot that
+             * an entry is a double then make it a double again, as the frame
+             * may have assigned it a normal register.
              */
-            for (unsigned i = 0; i < fixedDoubleEntries.length(); i++) {
-                FrameEntry *fe = frame.getSlotEntry(fixedDoubleEntries[i]);
+            for (unsigned i = 0; i < fixedIntToDoubleEntries.length(); i++) {
+                FrameEntry *fe = frame.getSlotEntry(fixedIntToDoubleEntries[i]);
                 frame.ensureInteger(fe);
             }
+            for (unsigned i = 0; i < fixedDoubleToAnyEntries.length(); i++) {
+                FrameEntry *fe = frame.getSlotEntry(fixedDoubleToAnyEntries[i]);
+                frame.syncAndForgetFe(fe);
+            }
         }
-        fixedDoubleEntries.clear();
+        fixedIntToDoubleEntries.clear();
+        fixedDoubleToAnyEntries.clear();
 
         if (opinfo->jumpTarget || trap) {
             if (fallthrough) {
                 fixDoubleTypes(PC);
-                fixedDoubleEntries.clear();
+                fixedIntToDoubleEntries.clear();
+                fixedDoubleToAnyEntries.clear();
 
                 /*
                  * Watch for fallthrough to the head of a 'do while' loop.
                  * We don't know what register state we will be using at the head
                  * of the loop so sync, branch, and fix it up after the loop
                  * has been processed.
                  */
                 if (cx->typeInferenceEnabled() && analysis->getCode(PC).loopHead) {
@@ -6889,51 +6895,57 @@ mjit::Compiler::jsop_toid()
  */
 void
 mjit::Compiler::fixDoubleTypes(jsbytecode *target)
 {
     if (!cx->typeInferenceEnabled())
         return;
 
     /*
-     * Fill fixedDoubleEntries with all variables that are known to be an int
-     * here and a double at the branch target. Per prepareInferenceTypes, the
-     * target state consists of the current state plus any phi nodes or other
-     * new values introduced at the target.
+     * Fill fixedIntToDoubleEntries with all variables that are known to be an
+     * int here and a double at the branch target, and fixedDoubleToAnyEntries
+     * with all variables that are known to be a double here but not at the
+     * branch target.
+     *
+     * Per prepareInferenceTypes, the target state consists of the current
+     * state plus any phi nodes or other new values introduced at the target.
      */
-    JS_ASSERT(fixedDoubleEntries.empty());
+    JS_ASSERT(fixedIntToDoubleEntries.empty());
+    JS_ASSERT(fixedDoubleToAnyEntries.empty());
     const SlotValue *newv = analysis->newValues(target);
     if (newv) {
         while (newv->slot) {
             if (newv->value.kind() != SSAValue::PHI ||
-                newv->value.phiOffset() != uint32(target - script->code)) {
+                newv->value.phiOffset() != uint32(target - script->code) ||
+                !analysis->trackSlot(newv->slot)) {
                 newv++;
                 continue;
             }
-            if (newv->slot < TotalSlots(script)) {
-                types::TypeSet *targetTypes = analysis->getValueTypes(newv->value);
-                VarType &vt = a->varTypes[newv->slot];
-                if (targetTypes->getKnownTypeTag(cx) == JSVAL_TYPE_DOUBLE &&
-                    analysis->trackSlot(newv->slot)) {
-                    FrameEntry *fe = frame.getSlotEntry(newv->slot);
-                    if (vt.type == JSVAL_TYPE_INT32) {
-                        fixedDoubleEntries.append(newv->slot);
-                        frame.ensureDouble(fe);
-                    } else if (vt.type == JSVAL_TYPE_UNKNOWN) {
-                        /*
-                         * Unknown here but a double at the target. The type
-                         * set for the existing value must be empty, so this
-                         * code is doomed and we can just mark the value as
-                         * a double.
-                         */
-                        frame.ensureDouble(fe);
-                    } else {
-                        JS_ASSERT(vt.type == JSVAL_TYPE_DOUBLE);
-                    }
+            JS_ASSERT(newv->slot < TotalSlots(script));
+            types::TypeSet *targetTypes = analysis->getValueTypes(newv->value);
+            FrameEntry *fe = frame.getSlotEntry(newv->slot);
+            VarType &vt = a->varTypes[newv->slot];
+            if (targetTypes->getKnownTypeTag(cx) == JSVAL_TYPE_DOUBLE) {
+                if (vt.type == JSVAL_TYPE_INT32) {
+                    fixedIntToDoubleEntries.append(newv->slot);
+                    frame.ensureDouble(fe);
+                } else if (vt.type == JSVAL_TYPE_UNKNOWN) {
+                    /*
+                     * Unknown here but a double at the target. The type
+                     * set for the existing value must be empty, so this
+                     * code is doomed and we can just mark the value as
+                     * a double.
+                     */
+                    frame.ensureDouble(fe);
+                } else {
+                    JS_ASSERT(vt.type == JSVAL_TYPE_DOUBLE);
                 }
+            } else if (fe->isType(JSVAL_TYPE_DOUBLE)) {
+                fixedDoubleToAnyEntries.append(newv->slot);
+                frame.syncAndForgetFe(fe);
             }
             newv++;
         }
     }
 }
 
 void
 mjit::Compiler::watchGlobalReallocation()
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -443,17 +443,18 @@ class Compiler : public BaseCompiler
 #if defined JS_POLYIC
     js::Vector<PICGenInfo, 16, CompilerAllocPolicy> pics;
     js::Vector<GetElementICInfo, 16, CompilerAllocPolicy> getElemICs;
     js::Vector<SetElementICInfo, 16, CompilerAllocPolicy> setElemICs;
 #endif
     js::Vector<CallPatchInfo, 64, CompilerAllocPolicy> callPatches;
     js::Vector<InternalCallSite, 64, CompilerAllocPolicy> callSites;
     js::Vector<DoublePatch, 16, CompilerAllocPolicy> doubleList;
-    js::Vector<uint32, 4, CompilerAllocPolicy> fixedDoubleEntries;
+    js::Vector<uint32> fixedIntToDoubleEntries;
+    js::Vector<uint32> fixedDoubleToAnyEntries;
     js::Vector<JumpTable, 16> jumpTables;
     js::Vector<uint32, 16> jumpTableOffsets;
     js::Vector<LoopEntry, 16> loopEntries;
     js::Vector<JSObject *, 0, CompilerAllocPolicy> rootedObjects;
     StubCompiler stubcc;
     Label invokeLabel;
     Label arityLabel;
     Label argsCheckLabel;
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -790,49 +790,35 @@ FrameState::syncForAllocation(RegisterAl
             syncFe(fe);
 
         if (fe->dataInRegister(reg))
             continue;
 
         if (!freeRegs.hasReg(reg))
             relocateReg(reg, alloc, uses);
 
-        /*
-         * It is possible that the fe is known to be a double currently but is not
-         * known to be a double at the join point (it may have non-double values
-         * assigned elsewhere in the script). It is *not* possible for the fe to
-         * be a non-double currently but a double at the join point --- the Compiler
-         * must have called fixDoubleTypes before branching.
-         */
-        if (reg.isReg() && fe->isType(JSVAL_TYPE_DOUBLE)) {
-            syncFe(fe);
-            forgetAllRegs(fe);
-            fe->resetSynced();
-        }
-        if (!reg.isReg()) {
-            JS_ASSERT(!fe->isNotType(JSVAL_TYPE_DOUBLE));
-            if (!fe->isTypeKnown())
-                learnType(fe, JSVAL_TYPE_DOUBLE, false);
-        }
-
         if (reg.isReg()) {
             RegisterID nreg = reg.reg();
+            JS_ASSERT(!fe->isType(JSVAL_TYPE_DOUBLE));
             if (fe->data.inMemory()) {
                 masm.loadPayload(addressOf(fe), nreg);
             } else if (fe->isConstant()) {
                 masm.loadValuePayload(fe->getValue(), nreg);
             } else {
                 JS_ASSERT(fe->data.inRegister() && fe->data.reg() != nreg);
                 masm.move(fe->data.reg(), nreg);
                 freeRegs.putReg(fe->data.reg());
                 regstate(fe->data.reg()).forget();
             }
             fe->data.setRegister(nreg);
         } else {
             FPRegisterID nreg = reg.fpreg();
+            JS_ASSERT(!fe->isNotType(JSVAL_TYPE_DOUBLE));
+            if (!fe->isTypeKnown())
+                learnType(fe, JSVAL_TYPE_DOUBLE, false);
             if (fe->data.inMemory()) {
                 masm.loadDouble(addressOf(fe), nreg);
             } else if (fe->isConstant()) {
                 masm.slowLoadConstantDouble(fe->getValue().toDouble(), nreg);
             } else {
                 JS_ASSERT(fe->data.inFPRegister() && fe->data.fpreg() != nreg);
                 masm.moveDouble(fe->data.fpreg(), nreg);
                 freeRegs.putReg(fe->data.fpreg());