[INFER] Don't learn types for dead entries at join points, bug 656591, learn argument types at script entry and mark monitored call ICs, bug 656920.
authorBrian Hackett <bhackett1024@gmail.com>
Sat, 14 May 2011 10:30:03 -0700
changeset 75049 9e0bab2c04b4b854763055555d5e2358649da289
parent 75048 498ea6c749f4a50b3407b936676411dac57786b4
child 75050 2ef9b9d500d43e6a2d83f6e1ca49dc0b66b09fce
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs656591, 656920
milestone6.0a1
[INFER] Don't learn types for dead entries at join points, bug 656591, learn argument types at script entry and mark monitored call ICs, bug 656920.
js/src/jit-test/tests/basic/typeMonitorCall.js
js/src/jit-test/tests/jaeger/bug656591.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/FastOps.cpp
js/src/methodjit/FrameState-inl.h
js/src/methodjit/FrameState.cpp
js/src/methodjit/FrameState.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/typeMonitorCall.js
@@ -0,0 +1,19 @@
+
+/* Make sure we are checking argument types when going through unknown but monomorphic call sites. */
+
+function foo(x) {
+  return x + 10;
+}
+
+var array = [1,2,3,4,5,6,7,"eight"];
+
+/* Jump through hoops to make 'f' unknown. */
+var f = this[eval("'foo'")];
+
+for (var i = 0; i < array.length; i++) {
+  var res = f(array[i]);
+  if (i != 7)
+    assertEq(res, i + 11);
+  else
+    assertEq(res, "eight10");
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug656591.js
@@ -0,0 +1,9 @@
+// |jit-test| error: TypeError
+(function () {
+    var i = 0; 
+    (function () {
+        var x;
+        (x = "3") || 1;
+        (x = "")(i || x);
+    })();
+})();
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -762,16 +762,19 @@ mjit::Compiler::generatePrologue()
     if (debugMode() || Probes::callTrackingActive(cx))
         INLINE_STUBCALL(stubs::ScriptDebugPrologue, REJOIN_RESUME);
 
     if (cx->typeInferenceEnabled())
         ensureDoubleArguments();
 
     recompileCheckHelper();
 
+    /* Update the initial types of arguments and locals with a use-before-def. */
+    restoreAnalysisTypes();
+
     return Compile_Okay;
 }
 
 void
 mjit::Compiler::ensureDoubleArguments()
 {
     /* Convert integer arguments which were inferred as (int|double) to doubles. */
     for (uint32 i = 0; script->fun && i < script->fun->nargs; i++) {
@@ -1578,17 +1581,17 @@ mjit::Compiler::generateMethod()
           }
           END_CASE(JSOP_FORARG)
 
           BEGIN_CASE(JSOP_FORLOCAL)
           {
             updateVarType();
             uint32 slot = GET_SLOTNO(PC);
             iterNext();
-            frame.storeLocal(slot, true, true);
+            frame.storeLocal(slot, true);
             frame.pop();
           }
           END_CASE(JSOP_FORLOCAL)
 
           BEGIN_CASE(JSOP_DUP)
             frame.dup();
           END_CASE(JSOP_DUP)
 
@@ -2219,17 +2222,17 @@ mjit::Compiler::generateMethod()
           }
           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, true);
+            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)));
             }
 
             if (pop) {
@@ -2239,17 +2242,17 @@ mjit::Compiler::generateMethod()
             }
           }
           END_CASE(JSOP_SETLOCAL)
 
           BEGIN_CASE(JSOP_SETLOCALPOP)
           {
             updateVarType();
             uint32 slot = GET_SLOTNO(PC);
-            frame.storeLocal(slot, true, true);
+            frame.storeLocal(slot, true);
             frame.pop();
           }
           END_CASE(JSOP_SETLOCALPOP)
 
           BEGIN_CASE(JSOP_UINT16)
             frame.push(Value(Int32Value((int32_t) GET_UINT16(PC))));
           END_CASE(JSOP_UINT16)
 
@@ -2454,17 +2457,17 @@ mjit::Compiler::generateMethod()
             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, true);
+            frame.storeLocal(slot, true);
             frame.pop();
           }
           END_CASE(JSOP_DEFLOCALFUN_FC)
 
           BEGIN_CASE(JSOP_LAMBDA)
           {
             JSFunction *fun = script->getFunction(fullAtomIndex(PC));
 
@@ -2552,17 +2555,17 @@ mjit::Compiler::generateMethod()
             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, true);
+            frame.storeLocal(slot, true);
             frame.pop();
           }
           END_CASE(JSOP_DEFLOCALFUN)
 
           BEGIN_CASE(JSOP_RETRVAL)
             emitReturn(NULL);
           END_CASE(JSOP_RETRVAL)
 
@@ -3454,16 +3457,18 @@ mjit::Compiler::inlineCallHelper(uint32 
             icCalleeData = origCalleeData;
             icRvalAddr = frame.addressOf(origCallee);
             callIC.frameSize.initStatic(frame.totalDepth(), speculatedArgc);
         }
     }
 
     callFrameSize = callIC.frameSize;
 
+    callIC.typeMonitored = monitored(PC);
+
     /* Test the type if necessary. Failing this always takes a really slow path. */
     MaybeJump notObjectJump;
     if (icCalleeType.isSet())
         notObjectJump = masm.testObject(Assembler::NotEqual, icCalleeType.reg());
 
     /*
      * For an optimized apply, keep icCalleeData and funPtrReg in a
      * callee-saved registers for the subsequent ic::SplatApplyArgs call.
@@ -6775,21 +6780,25 @@ mjit::Compiler::restoreAnalysisTypes()
                 VarType &vt = a->varTypes[newv->slot];
                 vt.types = analysis->getValueTypes(newv->value);
                 vt.type = vt.types->getKnownTypeTag(cx);
             }
             newv++;
         }
     }
 
-    /* Restore known types of locals/args. */
+    /*
+     * Restore known types of locals/args. Skip this for dead entries: the type
+     * may not be synced, and the slot will not be used anyways.
+     */
     for (uint32 slot = ArgSlot(0); slot < TotalSlots(script); slot++) {
         JSValueType type = a->varTypes[slot].type;
         if (type != JSVAL_TYPE_UNKNOWN &&
-            (type != JSVAL_TYPE_DOUBLE || analysis->trackSlot(slot))) {
+            (type != JSVAL_TYPE_DOUBLE || analysis->trackSlot(slot)) &&
+            (!analysis->trackSlot(slot) || analysis->liveness(slot).live(PC - script->code))) {
             FrameEntry *fe = frame.getSlotEntry(slot);
             JS_ASSERT_IF(fe->isTypeKnown(), fe->isType(type));
             if (!fe->isTypeKnown())
                 frame.learnType(fe, type, false);
         }
     }
 }
 
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -931,17 +931,17 @@ mjit::Compiler::jsop_localinc(JSOp op, u
         // Note, SUB will perform integer conversion for us.
         // Before: V 1
         // After:  N+1
         if (!jsop_binary(JSOP_SUB, stubs::Sub, type, types))
             return false;
 
         // Before: N+1
         // After:  N+1
-        frame.storeLocal(slot, analysis->popGuaranteed(PC), true);
+        frame.storeLocal(slot, analysis->popGuaranteed(PC));
     } else {
         // Before:
         // After: V
         frame.pushLocal(slot);
 
         // Before: V
         // After:  N
         jsop_pos();
@@ -956,17 +956,17 @@ mjit::Compiler::jsop_localinc(JSOp op, u
 
         // Before: N N 1
         // After:  N N+1
         if (!jsop_binary(JSOP_ADD, stubs::Add, type, types))
             return false;
 
         // Before: N N+1
         // After:  N N+1
-        frame.storeLocal(slot, true, true);
+        frame.storeLocal(slot, true);
 
         // Before: N N+1
         // After:  N
         frame.pop();
     }
 
     return true;
 }
--- a/js/src/methodjit/FrameState-inl.h
+++ b/js/src/methodjit/FrameState-inl.h
@@ -869,16 +869,29 @@ FrameState::syncData(FrameEntry *fe)
 
     ensureDataSynced(fe, masm);
 
     if (!fe->data.synced())
         fe->data.sync();
 }
 
 inline void
+FrameState::fakeSync(FrameEntry *fe)
+{
+    /*
+     * If a frame entry's value will no longer be used, we can mark it as being
+     * synced without actually performing the sync: the value is not observed.
+     */
+    if (!fe->data.synced())
+        fe->data.sync();
+    if (!fe->type.synced())
+        fe->type.sync();
+}
+
+inline void
 FrameState::forgetType(FrameEntry *fe)
 {
     /*
      * The type may have been forgotten with an intervening storeLocal in the
      * presence of eval or closed variables. For defense in depth and to make
      * callers' lives simpler, bail out if the type is not known.
      */
     if (!fe->isTypeKnown())
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -254,17 +254,17 @@ FrameState::variableLive(FrameEntry *fe,
     JS_ASSERT(cx->typeInferenceEnabled());
     JS_ASSERT(fe > a->callee_ && fe < a->spBase);
 
     uint32 offset = pc - a->script->code;
     return a->analysis->liveness(entrySlot(fe)).live(offset);
 }
 
 AnyRegisterID
-FrameState::bestEvictReg(uint32 mask, bool includePinned) const
+FrameState::bestEvictReg(uint32 mask, bool includePinned)
 {
     JS_ASSERT(cx->typeInferenceEnabled());
 
     /* Must be looking for a specific type of register. */
     JS_ASSERT((mask & Registers::AvailRegs) != (mask & Registers::AvailFPRegs));
 
     AnyRegisterID fallback;
     uint32 fallbackOffset = uint32(-1);
@@ -336,20 +336,17 @@ FrameState::bestEvictReg(uint32 mask, bo
 
         /* Any register for an entry dead at this bytecode is fine to evict. */
         Lifetime *lifetime = variableLive(fe, a->PC);
         if (!lifetime) {
             /*
              * Mark the entry as synced to avoid emitting a store, we don't need
              * to keep this value around.
              */
-            if (!fe->data.synced())
-                fe->data.sync();
-            if (!fe->type.synced())
-                fe->type.sync();
+            fakeSync(fe);
             JaegerSpew(JSpew_Regalloc, "result: %s (%s) is dead\n", entryName(fe), reg.name());
             return reg;
         }
 
         /*
          * Evict variables which are only live in future loop iterations, and are
          * not carried around the loop in a register.
          */
@@ -681,22 +678,18 @@ FrameState::syncForAllocation(RegisterAl
             forgetAllRegs(fe);
             fe->resetSynced();
             continue;
         }
 
         /* Force syncs for locals which are dead at the current PC. */
         if (isLocal(fe) && !a->analysis->slotEscapes(entrySlot(fe))) {
             Lifetime *lifetime = variableLive(fe, a->PC);
-            if (!lifetime) {
-                if (!fe->data.synced())
-                    fe->data.sync();
-                if (!fe->type.synced())
-                    fe->type.sync();
-            }
+            if (!lifetime)
+                fakeSync(fe);
         }
 
         if (!fe->isCopy() && alloc->hasAnyReg(fe - entries)) {
             /* Types are always synced, except for known doubles. */
             if (!fe->isType(JSVAL_TYPE_DOUBLE))
                 syncType(fe);
         } else {
             syncFe(fe);
@@ -2075,17 +2068,17 @@ FrameState::separateBinaryEntries(FrameE
     if (rhs->isCopy() && rhs->copyOf() == lhs) {
         syncAndForgetFe(rhs);
         syncAndForgetFe(lhs);
         uncopy(lhs);
     }
 }
 
 void
-FrameState::storeLocal(uint32 n, bool popGuaranteed, bool fixedType)
+FrameState::storeLocal(uint32 n, bool popGuaranteed)
 {
     FrameEntry *local = getLocal(n);
 
     if (a->analysis->slotEscapes(entrySlot(local))) {
         JS_ASSERT(local->data.inMemory());
         storeTo(peek(-1), addressOf(local), popGuaranteed);
         return;
     }
@@ -2143,27 +2136,21 @@ FrameState::storeTop(FrameEntry *target)
     if (top->isCopy() && top->copyOf() == target) {
         JS_ASSERT(target->isCopied());
         return;
     }
 
     /*
      * If this is overwriting a known non-double type with another value of the
      * same type, then make sure we keep the type marked as synced after doing
-     * the copy. Skip this for variables with undefined type --- for locals
-     * with no use-before-def, the initial undefined type is not synced but is
-     * marked as synced. If the local is then written with a known-undefined
-     * value, we need to make sure the in-memory type gets updated.
+     * the copy.
      */
     bool wasSynced = target->type.synced();
     JSValueType oldType = target->isTypeKnown() ? target->getKnownType() : JSVAL_TYPE_UNKNOWN;
-    bool trySyncType = wasSynced &&
-        oldType != JSVAL_TYPE_UNKNOWN &&
-        oldType != JSVAL_TYPE_DOUBLE &&
-        oldType != JSVAL_TYPE_UNDEFINED;
+    bool trySyncType = wasSynced && oldType != JSVAL_TYPE_UNKNOWN && oldType != JSVAL_TYPE_DOUBLE;
 
     /* Completely invalidate the local variable. */
     forgetEntry(target);
     target->resetUnsynced();
 
     /* Constants are easy to propagate. */
     if (top->isConstant()) {
         target->clear();
--- a/js/src/methodjit/FrameState.h
+++ b/js/src/methodjit/FrameState.h
@@ -609,17 +609,17 @@ class FrameState
     /*
      * Fully stores a FrameEntry into two arbitrary registers. tempReg may be
      * used as a temporary.
      */
     void loadForReturn(FrameEntry *fe, RegisterID typeReg, RegisterID dataReg, RegisterID tempReg);
     void loadThisForReturn(RegisterID typeReg, RegisterID dataReg, RegisterID tempReg);
 
     /* Stores the top stack slot back to a local or slot. */
-    void storeLocal(uint32 n, bool popGuaranteed = false, bool fixedType = false);
+    void storeLocal(uint32 n, bool popGuaranteed = false);
     void storeArg(uint32 n, bool popGuaranteed = false);
     void storeTop(FrameEntry *target);
 
     /*
      * Restores state from a slow path.
      */
     void merge(Assembler &masm, Changes changes) const;
 
@@ -945,16 +945,19 @@ class FrameState
     inline void ensureTypeSynced(const FrameEntry *fe, Assembler &masm) const;
     inline void ensureDataSynced(const FrameEntry *fe, Assembler &masm) const;
 
     /* Guarantee sync, even if register allocation is required, and set sync. */
     inline void syncFe(FrameEntry *fe);
     inline void syncType(FrameEntry *fe);
     inline void syncData(FrameEntry *fe);
 
+    /* For a frame entry whose value is dead, mark as synced. */
+    inline void fakeSync(FrameEntry *fe);
+
     inline FrameEntry *getCallee();
     inline FrameEntry *getThis();
     inline FrameEntry *getOrTrack(uint32 index);
 
     inline void forgetAllRegs(FrameEntry *fe);
     inline void swapInTracker(FrameEntry *lhs, FrameEntry *rhs);
 #if defined JS_NUNBOX32
     void syncFancy(Assembler &masm, Registers avail, FrameEntry *resumeAt,
@@ -994,17 +997,17 @@ class FrameState
         return regstate_[reg.reg_];
     }
 
     const RegisterState & regstate(AnyRegisterID reg) const {
         JS_ASSERT(reg.reg_ < Registers::TotalAnyRegisters);
         return regstate_[reg.reg_];
     }
 
-    AnyRegisterID bestEvictReg(uint32 mask, bool includePinned) const;
+    AnyRegisterID bestEvictReg(uint32 mask, bool includePinned);
 
     inline analyze::Lifetime * variableLive(FrameEntry *fe, jsbytecode *pc) const;
     inline bool binaryEntryLive(FrameEntry *fe) const;
     void relocateReg(AnyRegisterID reg, RegisterAllocation *alloc, Uses uses);
 
     bool isThis(const FrameEntry *fe) const {
         return fe == a->this_;
     }