[INFER] Coerce locals/args inferred as doubles to doubles after all writes, not just SET ops, bug 656748.
authorBrian Hackett <bhackett1024@gmail.com>
Sat, 14 May 2011 10:38:51 -0700
changeset 75050 2ef9b9d500d43e6a2d83f6e1ca49dc0b66b09fce
parent 75049 9e0bab2c04b4b854763055555d5e2358649da289
child 75051 ef1ce31f66b9054c6c4a81ce7e0e49b705ef6ebe
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs656748
milestone6.0a1
[INFER] Coerce locals/args inferred as doubles to doubles after all writes, not just SET ops, bug 656748.
js/src/jit-test/tests/jaeger/bug656748.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/FastOps.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug656748.js
@@ -0,0 +1,8 @@
+function f() {
+    var x = -0;
+    x++;
+    if (3 > 2) {};
+    var y = x + 2.14;
+    assertEq(y, 3.14);
+}
+f();
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -1568,31 +1568,31 @@ mjit::Compiler::generateMethod()
                 applyTricks = LazyArgsObj;
             else
                 jsop_arguments();
             pushSyncedEntry(0);
           END_CASE(JSOP_ARGUMENTS)
 
           BEGIN_CASE(JSOP_FORARG)
           {
-            updateVarType();
             uint32 arg = GET_SLOTNO(PC);
             iterNext();
             frame.storeArg(arg, true);
             frame.pop();
+            updateVarType();
           }
           END_CASE(JSOP_FORARG)
 
           BEGIN_CASE(JSOP_FORLOCAL)
           {
-            updateVarType();
             uint32 slot = GET_SLOTNO(PC);
             iterNext();
             frame.storeLocal(slot, true);
             frame.pop();
+            updateVarType();
           }
           END_CASE(JSOP_FORLOCAL)
 
           BEGIN_CASE(JSOP_DUP)
             frame.dup();
           END_CASE(JSOP_DUP)
 
           BEGIN_CASE(JSOP_DUP2)
@@ -2186,31 +2186,20 @@ mjit::Compiler::generateMethod()
           END_CASE(JSOP_GETARG)
 
           BEGIN_CASE(JSOP_BINDGNAME)
             jsop_bindgname();
           END_CASE(JSOP_BINDGNAME)
 
           BEGIN_CASE(JSOP_SETARG)
           {
-            updateVarType();
             jsbytecode *next = &PC[JSOP_SETLOCAL_LENGTH];
             bool pop = JSOp(*next) == JSOP_POP && !analysis->jumpTarget(next);
             frame.storeArg(GET_SLOTNO(PC), pop);
-
-            /*
-             * Types of variables inferred as doubles need to be maintained as
-             * doubles. We might forget the type of the variable by the next
-             * call to fixDoubleTypes.
-             */
-            if (cx->typeInferenceEnabled()) {
-                uint32 slot = ArgSlot(GET_SLOTNO(PC));
-                if (analysis->trackSlot(slot) && a->varTypes[slot].type == JSVAL_TYPE_DOUBLE)
-                    frame.ensureDouble(frame.getArg(GET_SLOTNO(PC)));
-            }
+            updateVarType();
 
             if (pop) {
                 frame.pop();
                 PC += JSOP_SETARG_LENGTH + JSOP_POP_LENGTH;
                 break;
             }
           }
           END_CASE(JSOP_SETARG)
@@ -2219,41 +2208,35 @@ mjit::Compiler::generateMethod()
           {
             uint32 slot = GET_SLOTNO(PC);
             frame.pushLocal(slot);
           }
           END_CASE(JSOP_GETLOCAL)
 
           BEGIN_CASE(JSOP_SETLOCAL)
           {
-            updateVarType();
             jsbytecode *next = &PC[JSOP_SETLOCAL_LENGTH];
             bool pop = JSOp(*next) == JSOP_POP && !analysis->jumpTarget(next);
             frame.storeLocal(GET_SLOTNO(PC), pop);
-
-            if (cx->typeInferenceEnabled()) {
-                uint32 slot = LocalSlot(script, GET_SLOTNO(PC));
-                if (analysis->trackSlot(slot) && a->varTypes[slot].type == JSVAL_TYPE_DOUBLE)
-                    frame.ensureDouble(frame.getLocal(GET_SLOTNO(PC)));
-            }
+            updateVarType();
 
             if (pop) {
                 frame.pop();
                 PC += JSOP_SETLOCAL_LENGTH + JSOP_POP_LENGTH;
                 break;
             }
           }
           END_CASE(JSOP_SETLOCAL)
 
           BEGIN_CASE(JSOP_SETLOCALPOP)
           {
-            updateVarType();
             uint32 slot = GET_SLOTNO(PC);
             frame.storeLocal(slot, true);
             frame.pop();
+            updateVarType();
           }
           END_CASE(JSOP_SETLOCALPOP)
 
           BEGIN_CASE(JSOP_UINT16)
             frame.push(Value(Int32Value((int32_t) GET_UINT16(PC))));
           END_CASE(JSOP_UINT16)
 
           BEGIN_CASE(JSOP_NEWINIT)
@@ -2449,26 +2432,26 @@ mjit::Compiler::generateMethod()
             prepareStubCall(Uses(1));
             masm.move(ImmPtr(atom), Registers::ArgReg1);
             INLINE_STUBCALL(stubs::SetConst, REJOIN_FALLTHROUGH);
           }
           END_CASE(JSOP_SETCONST)
 
           BEGIN_CASE(JSOP_DEFLOCALFUN_FC)
           {
-            updateVarType();
             uint32 slot = GET_SLOTNO(PC);
             JSFunction *fun = script->getFunction(fullAtomIndex(&PC[SLOTNO_LEN]));
             prepareStubCall(Uses(frame.frameSlots()));
             masm.move(ImmPtr(fun), Registers::ArgReg1);
             INLINE_STUBCALL(stubs::DefLocalFun_FC, REJOIN_DEFLOCALFUN);
             frame.takeReg(Registers::ReturnReg);
             frame.pushTypedPayload(JSVAL_TYPE_OBJECT, Registers::ReturnReg);
             frame.storeLocal(slot, true);
             frame.pop();
+            updateVarType();
           }
           END_CASE(JSOP_DEFLOCALFUN_FC)
 
           BEGIN_CASE(JSOP_LAMBDA)
           {
             JSFunction *fun = script->getFunction(fullAtomIndex(PC));
 
             JSObjStubFun stub = stubs::Lambda;
@@ -2547,26 +2530,26 @@ mjit::Compiler::generateMethod()
             prepareStubCall(Uses(0));
             INLINE_STUBCALL(stubs::ArgCnt, REJOIN_FALLTHROUGH);
             pushSyncedEntry(0);
           }
           END_CASE(JSOP_ARGCNT)
 
           BEGIN_CASE(JSOP_DEFLOCALFUN)
           {
-            updateVarType();
             uint32 slot = GET_SLOTNO(PC);
             JSFunction *fun = script->getFunction(fullAtomIndex(&PC[SLOTNO_LEN]));
             prepareStubCall(Uses(0));
             masm.move(ImmPtr(fun), Registers::ArgReg1);
             INLINE_STUBCALL(stubs::DefLocalFun, REJOIN_DEFLOCALFUN);
             frame.takeReg(Registers::ReturnReg);
             frame.pushTypedPayload(JSVAL_TYPE_OBJECT, Registers::ReturnReg);
             frame.storeLocal(slot, true);
             frame.pop();
+            updateVarType();
           }
           END_CASE(JSOP_DEFLOCALFUN)
 
           BEGIN_CASE(JSOP_RETRVAL)
             emitReturn(NULL);
           END_CASE(JSOP_RETRVAL)
 
           BEGIN_CASE(JSOP_GETGNAME)
@@ -6851,16 +6834,24 @@ mjit::Compiler::updateVarType()
     }
 
     uint32 slot = GetBytecodeSlot(script, PC);
 
     if (analysis->trackSlot(slot)) {
         VarType &vt = a->varTypes[slot];
         vt.types = types;
         vt.type = types->getKnownTypeTag(cx);
+
+        /*
+         * Variables whose type has been inferred as a double need to be
+         * maintained by the frame as a double. We might forget the exact
+         * representation used by the next call to fixDoubleTypes, fix it now.
+         */
+        if (vt.type == JSVAL_TYPE_DOUBLE)
+            frame.ensureDouble(frame.getSlotEntry(slot));
     }
 }
 
 JSValueType
 mjit::Compiler::knownPushedType(uint32 pushed)
 {
     if (!cx->typeInferenceEnabled())
         return JSVAL_TYPE_UNKNOWN;
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -907,18 +907,16 @@ mjit::Compiler::jsop_andor(JSOp op, jsby
     }
 
     return booleanJumpScript(op, target);
 }
 
 bool
 mjit::Compiler::jsop_localinc(JSOp op, uint32 slot)
 {
-    updateVarType();
-
     types::TypeSet *types = pushedTypeSet(0);
     JSValueType type = types ? types->getKnownTypeTag(cx) : JSVAL_TYPE_UNKNOWN;
 
     int amt = (op == JSOP_LOCALINC || op == JSOP_INCLOCAL) ? 1 : -1;
 
     if (!analysis->incrementInitialValueObserved(PC)) {
         // Before: 
         // After:  V
@@ -963,24 +961,23 @@ mjit::Compiler::jsop_localinc(JSOp op, u
         // After:  N N+1
         frame.storeLocal(slot, true);
 
         // Before: N N+1
         // After:  N
         frame.pop();
     }
 
+    updateVarType();
     return true;
 }
 
 bool
 mjit::Compiler::jsop_arginc(JSOp op, uint32 slot)
 {
-    updateVarType();
-
     types::TypeSet *types = pushedTypeSet(0);
     JSValueType type = types ? types->getKnownTypeTag(cx) : JSVAL_TYPE_UNKNOWN;
 
     int amt = (op == JSOP_ARGINC || op == JSOP_INCARG) ? 1 : -1;
 
     if (!analysis->incrementInitialValueObserved(PC)) {
         // Before: 
         // After:  V
@@ -1025,16 +1022,17 @@ mjit::Compiler::jsop_arginc(JSOp op, uin
         // After:  N N+1
         frame.storeArg(slot, true);
 
         // Before: N N+1
         // After:  N
         frame.pop();
     }
 
+    updateVarType();
     return true;
 }
 
 static inline bool
 IsCacheableSetElem(FrameEntry *obj, FrameEntry *id, FrameEntry *value)
 {
     if (obj->isNotType(JSVAL_TYPE_OBJECT))
         return false;