[INFER] Correct register loading on disabled trace ICs.
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 20 Dec 2010 15:47:09 -0800
changeset 74679 29e706f5dede43ff158d81ca818644b263ee283b
parent 74678 cc75bc35dfbae4fe1c68b075bce3a8a45001f237
child 74680 fce4c50e6f4947a39ae99fcb73c645cce8cf0b11
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
milestone2.0b8pre
[INFER] Correct register loading on disabled trace ICs.
js/src/methodjit/Compiler.cpp
js/src/methodjit/Compiler.h
js/src/methodjit/FrameState.cpp
js/src/methodjit/InvokeHelpers.cpp
js/src/methodjit/MonoIC.h
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -700,23 +700,24 @@ mjit::Compiler::finishThisUp(JITScript *
         jit->traceICs = NULL;
     }
 
     if (ic::TraceICInfo *scriptTICs = jit->traceICs) {
         for (size_t i = 0; i < traceICs.length(); i++) {
             if (!traceICs[i].initialized)
                 continue;
 
-            if (traceICs[i].trampoline) {
-                scriptTICs[i].jumpTarget = stubCode.locationOf(traceICs[i].trampolineStart);
+            if (traceICs[i].fastTrampoline) {
+                scriptTICs[i].fastTarget = stubCode.locationOf(traceICs[i].trampolineStart);
             } else {
                 uint32 offs = uint32(traceICs[i].jumpTarget - script->code);
                 JS_ASSERT(jumpMap[offs].isValid());
-                scriptTICs[i].jumpTarget = fullCode.locationOf(jumpMap[offs]);
+                scriptTICs[i].fastTarget = fullCode.locationOf(jumpMap[offs]);
             }
+            scriptTICs[i].slowTarget = stubCode.locationOf(traceICs[i].trampolineStart);
             scriptTICs[i].traceHint = fullCode.locationOf(traceICs[i].traceHint);
             scriptTICs[i].stubEntry = stubCode.locationOf(traceICs[i].stubEntry);
             scriptTICs[i].traceData = NULL;
 #ifdef DEBUG
             scriptTICs[i].jumpTargetPC = traceICs[i].jumpTarget;
 #endif
             scriptTICs[i].hasSlowTraceHint = traceICs[i].slowTraceHint.isSet();
             if (traceICs[i].slowTraceHint.isSet())
@@ -5092,46 +5093,48 @@ mjit::Compiler::finishLoop(jsbytecode *h
  * the OOL path loading some registers for the target. If this is the case,
  * the fast path jump was redirected to the stub code's initial label, and the
  * same must happen for any other fast paths for the target (i.e. paths from
  * inline caches).
  */
 bool
 mjit::Compiler::jumpAndTrace(Jump j, jsbytecode *target, Jump *slow, bool *trampoline)
 {
+    bool consistent = frame.consistentRegisters(target);
+
     if (trampoline)
         *trampoline = false;
 
     /*
      * Unless we are coming from a branch which synced everything, syncForBranch
      * must have been called and ensured an allocation at the target.
      */
     RegisterAllocation *&lvtarget = liveness.getCode(target).allocation;
     if (!lvtarget) {
         lvtarget = ArenaNew<RegisterAllocation>(liveness.pool, false);
         if (!lvtarget)
             return false;
-        JS_ASSERT(frame.consistentRegisters(target));
+        JS_ASSERT(consistent);
     }
 
     if (!addTraceHints || target >= PC ||
         (JSOp(*target) != JSOP_TRACE && JSOp(*target) != JSOP_NOTRACE)
 #ifdef JS_MONOIC
         || GET_UINT16(target) == BAD_TRACEIC_INDEX
 #endif
         )
     {
         if (lvtarget->synced()) {
-            JS_ASSERT(frame.consistentRegisters(target));
+            JS_ASSERT(consistent);
             if (!jumpInScript(j, target))
                 return false;
             if (slow && !stubcc.jumpInScript(*slow, target))
                 return false;
         } else {
-            if (frame.consistentRegisters(target)) {
+            if (consistent) {
                 if (!jumpInScript(j, target))
                     return false;
             } else {
                 /*
                  * Make a trampoline to issue remaining loads for the register
                  * state at target.
                  */
                 stubcc.linkExitDirect(j, stubcc.masm.label());
@@ -5175,30 +5178,19 @@ mjit::Compiler::jumpAndTrace(Jump j, jsb
     uint16 index = GET_UINT16(target);
     if (traceICs.length() <= index)
         if (!traceICs.resize(index+1))
             return false;
 # endif
 
     Label traceStart = stubcc.masm.label();
 
-    /*
-     * We make a trace IC even if the trace is currently disabled, in case it is
-     * enabled later, but set up the jumps so that InvokeTracer is initially skipped.
-     */
-    if (JSOp(*target) == JSOP_TRACE) {
-        stubcc.linkExitDirect(j, traceStart);
-        if (slow)
-            slow->linkTo(traceStart, &stubcc.masm);
-    } else {
-        if (!jumpInScript(j, target))
-            return false;
-        if (slow && !stubcc.jumpInScript(*slow, target))
-            return false;
-    }
+    stubcc.linkExitDirect(j, traceStart);
+    if (slow)
+        slow->linkTo(traceStart, &stubcc.masm);
 
 # if JS_MONOIC
     ic.addrLabel = stubcc.masm.moveWithPatch(ImmPtr(NULL), Registers::ArgReg1);
 # endif
 
     /* Save and restore compiler-tracked PC, so cx->regs is right in InvokeTracer. */
     {
         jsbytecode* pc = PC;
@@ -5212,27 +5204,38 @@ mjit::Compiler::jumpAndTrace(Jump j, jsb
     Jump no = stubcc.masm.branchTestPtr(Assembler::Zero, Registers::ReturnReg,
                                         Registers::ReturnReg);
     restoreFrameRegs(stubcc.masm);
     stubcc.masm.jump(Registers::ReturnReg);
     no.linkTo(stubcc.masm.label(), &stubcc.masm);
 
 #ifdef JS_MONOIC
     ic.jumpTarget = target;
-    ic.trampoline = false;
-
-    if (!frame.consistentRegisters(target)) {
-        ic.trampoline = true;
-        ic.trampolineStart = stubcc.masm.label();
-    }
+    ic.fastTrampoline = !consistent;
+    ic.trampolineStart = stubcc.masm.label();
 
     traceICs[index] = ic;
 #endif
 
     /*
+     * Jump past the tracer call if the trace has been blacklisted. We still make
+     * a trace IC in such cases, in case it is un-blacklisted later.
+     */
+    if (JSOp(*target) == JSOP_NOTRACE) {
+        if (consistent) {
+            if (!jumpInScript(j, target))
+                return false;
+        } else {
+            stubcc.linkExitDirect(j, stubcc.masm.label());
+        }
+        if (slow)
+            slow->linkTo(stubcc.masm.label(), &stubcc.masm);
+    }
+
+    /*
      * Reload any registers needed at the head of the loop. Note that we didn't
      * need to do syncing before calling InvokeTracer, as state is always synced
      * on backwards jumps.
      */
     frame.prepareForJump(target, stubcc.masm, true);
 
     if (!stubcc.jumpInScript(stubcc.masm.jump(), target))
         return false;
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -110,17 +110,17 @@ class Compiler : public BaseCompiler
         JSC::MacroAssembler::RegisterID tempReg;
     };
     
     struct TraceGenInfo {
         bool initialized;
         Label stubEntry;
         DataLabelPtr addrLabel;
         jsbytecode *jumpTarget;
-        bool trampoline;
+        bool fastTrampoline;
         Label trampolineStart;
         Jump traceHint;
         MaybeJump slowTraceHint;
 
         TraceGenInfo() : initialized(false) {}
     };
 
     /* InlineFrameAssembler wants to see this. */
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -757,17 +757,20 @@ FrameState::consistentRegisters(jsbyteco
 {
     /*
      * Before calling this, either the entire state should have been synced or
      * syncForBranch should have been called. These will ensure that any FE
      * which is not consistent with the target's register state has already
      * been synced, and no stores will need to be issued by prepareForJump.
      */
     RegisterAllocation *alloc = liveness.getCode(target).allocation;
-    JS_ASSERT(alloc);
+    if (!alloc) {
+        alloc = ArenaNew<RegisterAllocation>(liveness.pool, false);  /* :XXX: set OOM on failure */
+        return true;
+    }
 
     Registers regs(Registers::AvailAnyRegs);
     while (!regs.empty()) {
         AnyRegisterID reg = regs.takeAnyReg();
         if (alloc->assigned(reg)) {
             FrameEntry *needed = getOrTrack(alloc->slot(reg));
             if (!freeRegs.hasReg(reg)) {
                 FrameEntry *fe = regstate(reg).fe();
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -891,20 +891,20 @@ UpdateTraceHintSingle(Repatcher &repatch
     JaegerSpew(JSpew_PICs, "relinking trace hint %p to %p\n",
                jump.executableAddress(), target.executableAddress());
 }
 
 static void
 DisableTraceHint(VMFrame &f, ic::TraceICInfo &tic)
 {
     Repatcher repatcher(f.jit());
-    UpdateTraceHintSingle(repatcher, tic.traceHint, tic.jumpTarget);
+    UpdateTraceHintSingle(repatcher, tic.traceHint, tic.fastTarget);
 
     if (tic.hasSlowTraceHint)
-        UpdateTraceHintSingle(repatcher, tic.slowTraceHint, tic.jumpTarget);
+        UpdateTraceHintSingle(repatcher, tic.slowTraceHint, tic.slowTarget);
 }
 
 static void
 EnableTraceHintAt(JSScript *script, js::mjit::JITScript *jit, jsbytecode *pc, uint16_t index)
 {
     JS_ASSERT(index < jit->nTraceICs);
     ic::TraceICInfo &tic = jit->traceICs[index];
 
--- a/js/src/methodjit/MonoIC.h
+++ b/js/src/methodjit/MonoIC.h
@@ -138,17 +138,18 @@ struct MICInfo {
         } name;
     } u;
 };
 
 struct TraceICInfo {
     TraceICInfo() {}
 
     JSC::CodeLocationLabel stubEntry;
-    JSC::CodeLocationLabel jumpTarget;
+    JSC::CodeLocationLabel fastTarget;
+    JSC::CodeLocationLabel slowTarget;
     JSC::CodeLocationJump traceHint;
     JSC::CodeLocationJump slowTraceHint;
 #ifdef DEBUG
     jsbytecode *jumpTargetPC;
 #endif
     
     /* This data is used by the tracing JIT. */
     void *traceData;