Bug 670493 - Refactor and optimize booleanJumpScript. r=bhackett
authorJan de Mooij <jandemooij@gmail.com>
Sat, 03 Sep 2011 22:23:33 +0200
changeset 76518 6e67cafd75c8cfc78c1eecd2dc4acbffecb29825
parent 76517 d1d3ad947e9fa41a23b1775be9462fe7f1ff2cb3
child 76519 14271cf8f6f2a446ce564acc5c9fa4126ba13428
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersbhackett
bugs670493
milestone9.0a1
Bug 670493 - Refactor and optimize booleanJumpScript. r=bhackett
js/src/methodjit/FastOps.cpp
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -788,93 +788,79 @@ mjit::Compiler::jsop_typeof()
     frame.pop();
     frame.takeReg(Registers::ReturnReg);
     frame.pushTypedPayload(JSVAL_TYPE_STRING, Registers::ReturnReg);
 }
 
 bool
 mjit::Compiler::booleanJumpScript(JSOp op, jsbytecode *target)
 {
+    // JSOP_AND and JSOP_OR may leave the value on the stack (despite
+    // the frame.pop() below), so we need to sync it.
+    if (op == JSOP_AND || op == JSOP_OR) {
+        frame.syncForBranch(target, Uses(0));
+    } else {
+        JS_ASSERT(op == JSOP_IFEQ || op == JSOP_IFEQX ||
+                  op == JSOP_IFNE || op == JSOP_IFNEX);
+        frame.syncForBranch(target, Uses(1));
+    }
+
     FrameEntry *fe = frame.peek(-1);
-
-    MaybeRegisterID type;
-    MaybeRegisterID data;
-
-    if (!fe->isTypeKnown() && !frame.shouldAvoidTypeRemat(fe))
-        type.setReg(frame.copyTypeIntoReg(fe));
-    if (!fe->isType(JSVAL_TYPE_DOUBLE))
-        data.setReg(frame.copyDataIntoReg(fe));
-
-    frame.syncAndForgetEverything();
-
     Assembler::Condition cond = (op == JSOP_IFNE || op == JSOP_IFNEX || op == JSOP_OR)
                                 ? Assembler::NonZero
                                 : Assembler::Zero;
-    Assembler::Condition ncond = (op == JSOP_IFNE || op == JSOP_IFNEX || op == JSOP_OR)
-                                 ? Assembler::Zero
-                                 : Assembler::NonZero;
-
-    /* Inline path: Boolean guard + call script. */
-    MaybeJump jmpNotBool;
-    MaybeJump jmpNotExecScript;
-    if (type.isSet()) {
-        jmpNotBool.setJump(masm.testBoolean(Assembler::NotEqual, type.reg()));
-    } else {
-        if (!fe->isTypeKnown()) {
-            jmpNotBool.setJump(masm.testBoolean(Assembler::NotEqual,
-                                                frame.addressOf(fe)));
-        } else if (fe->isNotType(JSVAL_TYPE_BOOLEAN) &&
-                   fe->isNotType(JSVAL_TYPE_INT32)) {
-            jmpNotBool.setJump(masm.jump());
-        }
+
+    // Load data register and pin it so that frame.testBoolean
+    // below cannot evict it.
+    MaybeRegisterID data;
+    if (!fe->isType(JSVAL_TYPE_DOUBLE)) {
+        data = frame.tempRegForData(fe);
+        frame.pinReg(data.reg());
     }
 
-    /* 
-     * TODO: We don't need the second jump if
-     * jumpInScript() can go from ool path to inline path.
-     */
+    // Test for boolean if needed.
+    bool needStub = false;
+    if (!fe->isType(JSVAL_TYPE_BOOLEAN) && !fe->isType(JSVAL_TYPE_INT32)) {
+        Jump notBool;
+        if (fe->mightBeType(JSVAL_TYPE_BOOLEAN))
+            notBool = frame.testBoolean(Assembler::NotEqual, fe);
+        else
+            notBool = masm.jump();
+
+        stubcc.linkExitForBranch(notBool);
+        needStub = true;
+    }
+    if (data.isSet())
+        frame.unpinReg(data.reg());
+
+    // Test + branch.
+    Jump branch;
     if (!fe->isType(JSVAL_TYPE_DOUBLE))
-        jmpNotExecScript.setJump(masm.branchTest32(ncond, data.reg(), data.reg()));
-    Label lblExecScript = masm.label();
-    Jump j = masm.jump();
-
-
-    /* OOL path: Conversion to boolean. */
-    MaybeJump jmpCvtExecScript;
-    MaybeJump jmpCvtRejoin;
-    Label lblCvtPath = stubcc.masm.label();
-
-    if (!fe->isTypeKnown() ||
-        !(fe->isType(JSVAL_TYPE_BOOLEAN) || fe->isType(JSVAL_TYPE_INT32))) {
-        /* Note: this cannot overwrite slots holding loop invariants. */
+        branch = masm.branchTest32(cond, data.reg());
+    else
+        branch = masm.jump(); // dummy jump
+
+    // OOL path: call ValueToBoolean and branch.
+    if (needStub) {
+        stubcc.leave();
+
+        // Note: this cannot overwrite slots holding loop invariants.
         stubcc.masm.infallibleVMCall(JS_FUNC_TO_DATA_PTR(void *, stubs::ValueToBoolean),
                                      frame.totalDepth());
-
-        jmpCvtExecScript.setJump(stubcc.masm.branchTest32(cond, Registers::ReturnReg,
-                                                          Registers::ReturnReg));
-        jmpCvtRejoin.setJump(stubcc.masm.jump());
     }
 
-    /* Rejoin tag. */
-    Label lblAfterScript = masm.label();
-
-    /* Patch up jumps. */
-    if (jmpNotBool.isSet())
-        stubcc.linkExitDirect(jmpNotBool.getJump(), lblCvtPath);
-    if (jmpNotExecScript.isSet())
-        jmpNotExecScript.getJump().linkTo(lblAfterScript, &masm);
-
-    if (jmpCvtExecScript.isSet())
-        stubcc.crossJump(jmpCvtExecScript.getJump(), lblExecScript);
-    if (jmpCvtRejoin.isSet())
-        stubcc.crossJump(jmpCvtRejoin.getJump(), lblAfterScript);
+    Jump stubBranch = stubcc.masm.branchTest32(cond, Registers::ReturnReg);
+
+    // Rejoin from the stub call fallthrough.
+    if (needStub)
+        stubcc.rejoin(Changes(0));
 
     frame.pop();
 
-    return jumpAndTrace(j, target);
+    return jumpAndTrace(branch, target, &stubBranch);
 }
 
 bool
 mjit::Compiler::jsop_ifneq(JSOp op, jsbytecode *target)
 {
     FrameEntry *fe = frame.peek(-1);
 
     if (fe->isConstant()) {