[INFER] Treat 'this' in constructors as live throughout the script, bug 678234.
authorBrian Hackett <bhackett1024@gmail.com>
Fri, 12 Aug 2011 09:41:48 -0700
changeset 77432 07361922fd67f3678cd65f29e9369c54f552a2f7
parent 77431 409b62513ac6ad4a1c2287376fb59ead21fe7550
child 77433 1876d7b33e2f56230ac74d3c00bc6da4d42756bd
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs678234
milestone8.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
[INFER] Treat 'this' in constructors as live throughout the script, bug 678234.
js/src/jit-test/tests/jaeger/bug678234.js
js/src/methodjit/Compiler.h
js/src/methodjit/FrameState-inl.h
js/src/methodjit/FrameState.cpp
js/src/methodjit/FrameState.h
js/src/methodjit/StubCalls.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug678234.js
@@ -0,0 +1,10 @@
+a = {}
+function f(o) {
+    for (x in o) {
+        print
+    }
+}
+for (var i = 0; i < 3; i++) {
+    new f(a)
+    a.__proto__ = null
+}
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -483,16 +483,17 @@ class Compiler : public BaseCompiler
     bool knownJump(jsbytecode *pc);
     Label labelOf(jsbytecode *target, uint32 inlineIndex);
     void addCallSite(const InternalCallSite &callSite);
     void addReturnSite();
     void inlineStubCall(void *stub, RejoinState rejoin);
 
     bool debugMode() { return debugMode_; }
     bool inlining() { return inlining_; }
+    bool constructing() { return isConstructing; }
 
     jsbytecode *outerPC() {
         if (a == outer)
             return PC;
         ActiveFrame *scan = a;
         while (scan && scan->parent != outer)
             scan = scan->parent;
         return scan->parentPC;
--- a/js/src/methodjit/FrameState-inl.h
+++ b/js/src/methodjit/FrameState-inl.h
@@ -1331,16 +1331,22 @@ FrameState::setThis(RegisterID reg)
 
 void
 FrameState::syncThis()
 {
     FrameEntry *fe = getThis();
     syncFe(fe);
 }
 
+inline bool
+FrameState::isConstructorThis(const FrameEntry *fe) const
+{
+    return isThis(fe) && cc.constructing();
+}
+
 inline void
 FrameState::leaveBlock(uint32 n)
 {
     popn(n);
 }
 
 inline void
 FrameState::enterBlock(uint32 n)
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -248,16 +248,21 @@ FrameState::evictReg(AnyRegisterID reg)
     }
 
     regstate(reg).forget();
 }
 
 inline Lifetime *
 FrameState::variableLive(FrameEntry *fe, jsbytecode *pc) const
 {
+    /*
+     * Whether an argument, local or 'this' entry is live at pc. Note: this
+     * does not account for the 'this' entry when the script is used as a
+     * constructor, in which case it is live the entire frame.
+     */
     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
@@ -327,19 +332,29 @@ FrameState::bestEvictReg(uint32 mask, bo
             if (!fallback.isSet() || offset > fallbackOffset) {
                 fallback = reg;
                 fallbackOffset = offset;
             }
             JaegerSpew(JSpew_Regalloc, "    %s is a LICM or inline parent entry\n", reg.name());
             continue;
         }
 
-        /* All entries still in registers should be live. */
+        /*
+         * All entries still in registers should have a lifetime, except 'this'
+         * in constructors which are not accessed later on.
+         */
         Lifetime *lifetime = variableLive(fe, a->PC);
-        JS_ASSERT(lifetime);
+
+        if (!lifetime) {
+            JS_ASSERT(isConstructorThis(fe));
+            fallback = reg;
+            fallbackOffset = a->script->length;
+            JaegerSpew(JSpew_Regalloc, "    %s is 'this' in a constructor\n", reg.name());
+            continue;
+        }
 
         /*
          * Evict variables which are only live in future loop iterations, and are
          * not carried around the loop in a register.
          */
         JS_ASSERT_IF(lifetime->loopTail, loop);
         if (lifetime->loopTail && !loop->carriesLoopReg(fe)) {
             JaegerSpew(JSpew_Regalloc, "result: %s (%s) only live in later iterations\n",
@@ -380,18 +395,20 @@ FrameState::evictDeadEntries(bool includ
 
         if (!(Registers::maskReg(reg) & Registers::AvailAnyRegs))
             continue;
 
         FrameEntry *fe = includePinned ? regstate(reg).usedBy() : regstate(reg).fe();
         if (!fe)
             continue;
 
-        if (fe == a->callee_ || fe >= a->spBase || fe->isCopied() || (a->parent && fe < a->locals))
+        if (fe == a->callee_ || isConstructorThis(fe) ||
+            fe >= a->spBase || fe->isCopied() || (a->parent && fe < a->locals)) {
             continue;
+        }
 
         Lifetime *lifetime = variableLive(fe, a->PC);
         if (lifetime)
             continue;
 
         /*
          * If we are about to fake sync for an entry with known type, reset
          * that type. We don't want to regard it as correctly synced later.
@@ -580,16 +597,17 @@ FrameState::computeAllocation(jsbytecode
      */
     Registers regs = Registers::AvailAnyRegs;
     while (!regs.empty()) {
         AnyRegisterID reg = regs.takeAnyReg();
         if (freeRegs.hasReg(reg) || regstate(reg).type() == RematInfo::TYPE)
             continue;
         FrameEntry *fe = regstate(reg).fe();
         if (fe < a->callee_ ||
+            isConstructorThis(fe) ||
             (fe > a->callee_ && fe < a->spBase && variableLive(fe, target)) ||
             (isTemporary(fe) && (a->parent || uint32(target - a->script->code) <= loop->backedgeOffset()))) {
             /*
              * For entries currently in floating point registers, check they
              * are known to be doubles at the target. We don't need to do this
              * for entries in normal registers, as fixDoubleTypes must have been
              * called to convert them to floats.
              */
--- a/js/src/methodjit/FrameState.h
+++ b/js/src/methodjit/FrameState.h
@@ -1021,16 +1021,18 @@ class FrameState
     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_;
     }
 
+    inline bool isConstructorThis(const FrameEntry *fe) const;
+
     bool isArg(const FrameEntry *fe) const {
         return a->script->hasFunction && fe >= a->args && fe - a->args < a->script->function()->nargs;
     }
 
     bool isLocal(const FrameEntry *fe) const {
         return fe >= a->locals && fe - a->locals < a->script->nfixed;
     }
 
--- a/js/src/methodjit/StubCalls.cpp
+++ b/js/src/methodjit/StubCalls.cpp
@@ -2450,22 +2450,28 @@ stubs::CheckArgumentTypes(VMFrame &f)
 #ifdef DEBUG
 void JS_FASTCALL
 stubs::AssertArgumentTypes(VMFrame &f)
 {
     StackFrame *fp = f.fp();
     JSFunction *fun = fp->fun();
     JSScript *script = fun->script();
 
-    Type type = GetValueType(f.cx, fp->thisValue());
-    if (!TypeScript::ThisTypes(script)->hasType(type))
-        TypeFailure(f.cx, "Missing type for this: %s", TypeString(type));
+    /*
+     * Don't check the type of 'this' for constructor frames, the 'this' value
+     * has not been constructed yet.
+     */
+    if (!fp->isConstructing()) {
+        Type type = GetValueType(f.cx, fp->thisValue());
+        if (!TypeScript::ThisTypes(script)->hasType(type))
+            TypeFailure(f.cx, "Missing type for this: %s", TypeString(type));
+    }
 
     for (unsigned i = 0; i < fun->nargs; i++) {
-        type = GetValueType(f.cx, fp->formalArg(i));
+        Type type = GetValueType(f.cx, fp->formalArg(i));
         if (!TypeScript::ArgTypes(script, i)->hasType(type))
             TypeFailure(f.cx, "Missing type for arg %d: %s", i, TypeString(type));
     }
 }
 #endif
 
 /*
  * These two are never actually called, they just give us a place to rejoin if