[INFER] Don't call fixDoubleTypes twice for the same op, bug 655505. r=bhackett
authorJan de Mooij <jandemooij@gmail.com>
Tue, 10 May 2011 00:01:48 +0200
changeset 75006 3d26d25a4f6368c12ff4265e6139bc160fd33fb6
parent 75005 18c270b4c0582fbd7fee7272e556b1830ad30adb
child 75007 cb9c34a8b2b47b0c66f7431844eecc26aaf6d1cf
push id1199
push userjorendorff@mozilla.com
push dateSat, 13 Aug 2011 18:32:33 +0000
treeherdermozilla-inbound@080fece621e4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs655505
milestone6.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] Don't call fixDoubleTypes twice for the same op, bug 655505. r=bhackett
js/src/jit-test/tests/jaeger/bug655505.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/Compiler.h
js/src/methodjit/FastArithmetic.cpp
js/src/methodjit/FastOps.cpp
js/src/methodjit/FrameState.cpp
--- a/js/src/jit-test/tests/jaeger/bug655505.js
+++ b/js/src/jit-test/tests/jaeger/bug655505.js
@@ -1,12 +1,15 @@
 var a = [, , , , , , ];
+var res = 0;
 exhaustiveSliceTest("exhaustive slice test 1", a);
 function mySlice(a, from, to) {
     var to2 = to;
-    if (to2 < 0) {
+    if (to2 > 0) {
+        res += to2;
         to2 = to2.length + to;
     }
 }
 function exhaustiveSliceTest(testname, a) { x = a; }
 for (y = a.length; y >= 0; y--) {
     mySlice(a, x, y);
 }
+assertEq(res, 21);
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -1646,17 +1646,16 @@ mjit::Compiler::generateMethod()
 
                     if (!target) {
                         frame.push(Value(BooleanValue(result)));
                     } else {
                         if (fused == JSOP_IFEQ)
                             result = !result;
 
                         if (result) {
-                            fixDoubleTypes(target);
                             if (!frame.syncForBranch(target, Uses(0)))
                                 return Compile_Error;
                             Jump j = masm.jump();
                             if (!jumpAndTrace(j, target))
                                 return Compile_Error;
                         } else {
                             /*
                              * Branch is never taken, but clean up any loop
@@ -2039,18 +2038,22 @@ mjit::Compiler::generateMethod()
           END_CASE(JSOP_FALSE)
 
           BEGIN_CASE(JSOP_TRUE)
             frame.push(Value(BooleanValue(true)));
           END_CASE(JSOP_TRUE)
 
           BEGIN_CASE(JSOP_OR)
           BEGIN_CASE(JSOP_AND)
-            if (!jsop_andor(op, PC + GET_JUMP_OFFSET(PC)))
+          {
+            jsbytecode *target = PC + GET_JUMP_OFFSET(PC);
+            fixDoubleTypes(target);
+            if (!jsop_andor(op, target))
                 return Compile_Error;
+          }
           END_CASE(JSOP_AND)
 
           BEGIN_CASE(JSOP_TABLESWITCH)
             /*
              * Note: there is no need to syncForBranch for the various targets of
              * switch statement. The liveness analysis has already marked these as
              * allocated with no registers in use. There is also no need to fix
              * double types, as we don't track types of slots in scripts with
@@ -2110,19 +2113,27 @@ mjit::Compiler::generateMethod()
           BEGIN_CASE(JSOP_ITER)
             if (!iter(PC[1]))
                 return Compile_Error;
           END_CASE(JSOP_ITER)
 
           BEGIN_CASE(JSOP_MOREITER)
           {
             /* At the byte level, this is always fused with IFNE or IFNEX. */
-            if (!iterMore())
+            jsbytecode *target = &PC[JSOP_MOREITER_LENGTH];
+            JSOp next = JSOp(*target);
+            JS_ASSERT(next == JSOP_IFNE || next == JSOP_IFNEX);
+
+            target += (next == JSOP_IFNE)
+                      ? GET_JUMP_OFFSET(target)
+                      : GET_JUMPX_OFFSET(target);
+
+            fixDoubleTypes(target);
+            if (!iterMore(target))
                 return Compile_Error;
-            JSOp next = JSOp(PC[JSOP_MOREITER_LENGTH]);
             PC += JSOP_MOREITER_LENGTH;
             PC += js_CodeSpec[next].length;
             break;
           }
           END_CASE(JSOP_MOREITER)
 
           BEGIN_CASE(JSOP_ENDITER)
             iterEnd();
@@ -3967,22 +3978,20 @@ mjit::Compiler::compareTwoValues(JSConte
 
     JS_NOT_REACHED("NYI");
     return false;
 }
 
 bool
 mjit::Compiler::emitStubCmpOp(BoolStub stub, jsbytecode *target, JSOp fused)
 {
-    if (target) {
-        fixDoubleTypes(target);
+    if (target)
         frame.syncAndKillEverything();
-    } else {
+    else
         frame.syncAndKill(Uses(2));
-    }
 
     prepareStubCall(Uses(2));
     INLINE_STUBCALL(stub, target ? REJOIN_BRANCH : REJOIN_PUSH_BOOLEAN);
     frame.pop();
     frame.pop();
 
     if (!target) {
         frame.takeReg(Registers::ReturnReg);
@@ -5477,27 +5486,18 @@ mjit::Compiler::iterNext()
 
     frame.pushUntypedPayload(JSVAL_TYPE_STRING, T3);
 
     /* Join with the stub call. */
     stubcc.rejoin(Changes(1));
 }
 
 bool
-mjit::Compiler::iterMore()
+mjit::Compiler::iterMore(jsbytecode *target)
 {
-    jsbytecode *target = &PC[JSOP_MOREITER_LENGTH];
-    JSOp next = JSOp(*target);
-    JS_ASSERT(next == JSOP_IFNE || next == JSOP_IFNEX);
-
-    target += (next == JSOP_IFNE)
-              ? GET_JUMP_OFFSET(target)
-              : GET_JUMPX_OFFSET(target);
-
-    fixDoubleTypes(target);
     if (!frame.syncForBranch(target, Uses(1)))
         return false;
 
     FrameEntry *fe = frame.peek(-1);
     RegisterID reg = frame.tempRegForData(fe);
     RegisterID tempreg = frame.allocReg();
 
     /* Test clasp */
@@ -6717,16 +6717,17 @@ mjit::Compiler::fixDoubleTypes(jsbytecod
         return;
 
     /*
      * Fill fixedDoubleEntries with all variables that are known to be an int
      * here and a double at the branch target. Per prepareInferenceTypes, the
      * target state consists of the current state plus any phi nodes or other
      * new values introduced at the target.
      */
+    JS_ASSERT(fixedDoubleEntries.empty());
     const SlotValue *newv = analysis->newValues(target);
     if (newv) {
         while (newv->slot) {
             if (newv->value.kind() != SSAValue::PHI ||
                 newv->value.phiOffset() != uint32(target - script->code)) {
                 newv++;
                 continue;
             }
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -551,17 +551,17 @@ class Compiler : public BaseCompiler
     bool jumpInScript(Jump j, jsbytecode *pc);
     bool compareTwoValues(JSContext *cx, JSOp op, const Value &lhs, const Value &rhs);
     bool canUseApplyTricks();
 
     /* Emitting helpers. */
     bool emitStubCmpOp(BoolStub stub, jsbytecode *target, JSOp fused);
     bool iter(uintN flags);
     void iterNext();
-    bool iterMore();
+    bool iterMore(jsbytecode *target);
     void iterEnd();
     MaybeJump loadDouble(FrameEntry *fe, FPRegisterID *fpReg, bool *allocated);
 #ifdef JS_POLYIC
     void passICAddress(BaseICInfo *ic);
 #endif
 #ifdef JS_MONOIC
     void passMICAddress(GlobalNameICInfo &mic);
 #endif
--- a/js/src/methodjit/FastArithmetic.cpp
+++ b/js/src/methodjit/FastArithmetic.cpp
@@ -1140,17 +1140,16 @@ mjit::Compiler::jsop_equality_int_string
         ValueRemat lvr, rvr;
         frame.pinEntry(lhs, lvr);
         frame.pinEntry(rhs, rvr);
 
         /*
          * Sync everything except the top two entries.
          * We will handle the lhs/rhs in the stub call path.
          */
-        fixDoubleTypes(target);
         frame.syncAndKill(Registers(Registers::AvailRegs), Uses(frame.frameSlots()), Uses(2));
 
         RegisterID tempReg = frame.allocReg();
 
         JaegerSpew(JSpew_Insns, " ---- BEGIN STUB CALL CODE ---- \n");
 
         RESERVE_OOL_SPACE(stubcc.masm);
 
@@ -1413,19 +1412,16 @@ DoubleCondForOp(JSOp op, JSOp fused)
 }
 
 bool
 mjit::Compiler::jsop_relational_double(JSOp op, BoolStub stub, jsbytecode *target, JSOp fused)
 {
     FrameEntry *rhs = frame.peek(-1);
     FrameEntry *lhs = frame.peek(-2);
 
-    if (target)
-        fixDoubleTypes(target);
-
     JS_ASSERT_IF(!target, fused != JSOP_IFEQ);
 
     FPRegisterID fpLeft, fpRight;
     bool allocateLeft, allocateRight;
 
     MaybeJump lhsNotNumber = loadDouble(lhs, &fpLeft, &allocateLeft);
     if (!allocateLeft)
         frame.pinReg(fpLeft);
@@ -1511,17 +1507,16 @@ mjit::Compiler::jsop_relational_int(JSOp
         rhs = tmp;
         op = ReverseCompareOp(op);
     }
 
     JS_ASSERT_IF(!target, fused != JSOP_IFEQ);
     Assembler::Condition cond = GetCompareCondition(op, fused);
 
     if (target) {
-        fixDoubleTypes(target);
         if (!frame.syncForBranch(target, Uses(2)))
             return false;
 
         RegisterID lreg = frame.tempRegForData(lhs);
         Jump fast;
         if (rhs->isConstant()) {
             fast = masm.branch32(cond, lreg, Imm32(rhs->getValue().toInt32()));
         } else {
@@ -1558,19 +1553,16 @@ mjit::Compiler::jsop_relational_int(JSOp
 
 /* See jsop_binary_full() for more information on how this works. */
 bool
 mjit::Compiler::jsop_relational_full(JSOp op, BoolStub stub, jsbytecode *target, JSOp fused)
 {
     FrameEntry *rhs = frame.peek(-1);
     FrameEntry *lhs = frame.peek(-2);
 
-    if (target)
-        fixDoubleTypes(target);
-
     /* Allocate all registers up-front. */
     FrameState::BinaryAlloc regs;
     frame.allocForBinary(lhs, rhs, op, regs, !target);
 
     MaybeJump lhsNotDouble, rhsNotNumber, lhsUnknownDone;
     if (!lhs->isTypeKnown())
         emitLeftDoublePath(lhs, rhs, regs, lhsNotDouble, rhsNotNumber, lhsUnknownDone);
 
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -421,17 +421,16 @@ mjit::Compiler::jsop_equality(JSOp op, B
         frame.pop();
 
         /*
          * :FIXME: Easier test for undefined || null?
          * Maybe put them next to each other, subtract, do a single compare?
          */
 
         if (target) {
-            fixDoubleTypes(target);
             frame.syncAndKillEverything();
             frame.freeReg(reg);
 
             Jump sj = stubcc.masm.branchTest32(GetStubCompareCondition(fused),
                                                Registers::ReturnReg, Registers::ReturnReg);
 
             if ((op == JSOP_EQ && fused == JSOP_IFNE) ||
                 (op == JSOP_NE && fused == JSOP_IFEQ)) {
@@ -488,17 +487,16 @@ mjit::Compiler::jsop_equality(JSOp op, B
 
         if (lhsKind != types::OBJECT_UNKNOWN && rhsKind != types::OBJECT_UNKNOWN) {
             /* :TODO: Merge with jsop_relational_int? */
             JS_ASSERT_IF(!target, fused != JSOP_IFEQ);
             frame.forgetMismatchedObject(lhs);
             frame.forgetMismatchedObject(rhs);
             Assembler::Condition cond = GetCompareCondition(op, fused);
             if (target) {
-                fixDoubleTypes(target);
                 Jump sj = stubcc.masm.branchTest32(GetStubCompareCondition(fused),
                                                    Registers::ReturnReg, Registers::ReturnReg);
                 if (!frame.syncForBranch(target, Uses(2)))
                     return false;
                 RegisterID lreg = frame.tempRegForData(lhs);
                 frame.pinReg(lreg);
                 RegisterID rreg = frame.tempRegForData(rhs);
                 frame.unpinReg(lreg);
@@ -860,17 +858,16 @@ mjit::Compiler::booleanJumpScript(JSOp o
     frame.pop();
 
     return jumpAndTrace(j, target);
 }
 
 bool
 mjit::Compiler::jsop_ifneq(JSOp op, jsbytecode *target)
 {
-    fixDoubleTypes(target);
     FrameEntry *fe = frame.peek(-1);
 
     if (fe->isConstant()) {
         JSBool b = js_ValueToBoolean(fe->getValue());
 
         frame.pop();
 
         if (op == JSOP_IFEQ)
@@ -888,17 +885,16 @@ mjit::Compiler::jsop_ifneq(JSOp op, jsby
     }
 
     return booleanJumpScript(op, target);
 }
 
 bool
 mjit::Compiler::jsop_andor(JSOp op, jsbytecode *target)
 {
-    fixDoubleTypes(target);
     FrameEntry *fe = frame.peek(-1);
 
     if (fe->isConstant()) {
         JSBool b = js_ValueToBoolean(fe->getValue());
         
         /* Short-circuit. */
         if ((op == JSOP_OR && b == JS_TRUE) ||
             (op == JSOP_AND && b == JS_FALSE)) {
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -1785,16 +1785,17 @@ FrameState::ensureInteger(FrameEntry *fe
 
     if (fe->isConstant()) {
         Value newValue = Int32Value(int32(fe->getValue().toDouble()));
         fe->setConstant(Jsvalify(newValue));
         return;
     }
 
     JS_ASSERT(!fe->isCopy() && !fe->isCopied());
+    JS_ASSERT_IF(fe->isTypeKnown(), fe->isType(JSVAL_TYPE_DOUBLE));
 
     if (!fe->isType(JSVAL_TYPE_DOUBLE)) {
         /*
          * A normal register may have been allocated after calling
          * syncAndForgetEverything.
          */
         if (fe->data.inRegister()) {
             syncFe(fe);