Bug 686106 - Don't allocate an extra register for JSOP_MUL with constant operand. r=bhackett
authorJan de Mooij <jandemooij@gmail.com>
Mon, 12 Sep 2011 19:23:25 +0200
changeset 78205 5079bd3f82bf34d11b38cd6891d20a4f4631c0d2
parent 78204 d49caeac648d175aa7f3b23c0fdd3a2c13e99f33
child 78206 e60a0b9fe93c716a4f04959474621c269a8ac160
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)
reviewersbhackett
bugs686106
milestone9.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
Bug 686106 - Don't allocate an extra register for JSOP_MUL with constant operand. r=bhackett
js/src/assembler/assembler/X86Assembler.h
js/src/methodjit/FastArithmetic.cpp
js/src/methodjit/FrameState.cpp
--- a/js/src/assembler/assembler/X86Assembler.h
+++ b/js/src/assembler/assembler/X86Assembler.h
@@ -984,17 +984,19 @@ public:
     void imull_mr(int offset, RegisterID base, RegisterID dst)
     {
         FIXME_INSN_PRINTING;
         m_formatter.twoByteOp(OP2_IMUL_GvEv, dst, base, offset);
     }
 
     void imull_i32r(RegisterID src, int32_t value, RegisterID dst)
     {
-        FIXME_INSN_PRINTING;
+        js::JaegerSpew(js::JSpew_Insns,
+                       IPFX "imull      %d, %s, %s\n",
+                       MAYBE_PAD, value, nameIReg(4, src), nameIReg(4, dst));
         m_formatter.oneByteOp(OP_IMUL_GvEvIz, dst, src);
         m_formatter.immediate32(value);
     }
 
     void idivl_r(RegisterID dst)
     {
         js::JaegerSpew(js::JSpew_Insns,
                        IPFX "idivl      %s\n", MAYBE_PAD, 
--- a/js/src/methodjit/FastArithmetic.cpp
+++ b/js/src/methodjit/FastArithmetic.cpp
@@ -657,38 +657,45 @@ mjit::Compiler::jsop_binary_full(FrameEn
                 overflow = masm.branchSub32(Assembler::Overflow, reg.reg(), regs.result);
             else
                 overflow = masm.branchSub32(Assembler::Overflow, Imm32(value), regs.result);
         }
         break;
 
       case JSOP_MUL:
       {
-        JS_ASSERT(reg.isSet());
-
         MaybeJump storeNegZero;
         bool maybeNegZero = !ignoreOverflow;
         bool hasConstant = (lhs->isConstant() || rhs->isConstant());
 
         if (hasConstant && maybeNegZero) {
             value = (lhs->isConstant() ? lhs : rhs)->getValue().toInt32();
             RegisterID nonConstReg = lhs->isConstant() ? regs.rhsData.reg() : regs.lhsData.reg();
 
             if (value > 0)
                 maybeNegZero = false;
             else if (value < 0)
                 storeNegZero = masm.branchTest32(Assembler::Zero, nonConstReg);
             else
                 storeNegZero = masm.branch32(Assembler::LessThan, nonConstReg, Imm32(0));
         }
 
-        if (cannotOverflow)
-            masm.mul32(reg.reg(), regs.result);
-        else
-            overflow = masm.branchMul32(Assembler::Overflow, reg.reg(), regs.result);
+        if (cannotOverflow) {
+            if (reg.isSet())
+                masm.mul32(reg.reg(), regs.result);
+            else
+                masm.mul32(Imm32(value), regs.result, regs.result);
+        } else {
+            if (reg.isSet()) {
+                overflow = masm.branchMul32(Assembler::Overflow, reg.reg(), regs.result);
+            } else {
+                overflow = masm.branchMul32(Assembler::Overflow, Imm32(value), regs.result,
+                                            regs.result);
+            }
+        }
 
         if (maybeNegZero) {
             if (hasConstant) {
                 stubcc.linkExit(storeNegZero.get(), Uses(2));
             } else {
                 Jump isZero = masm.branchTest32(Assembler::Zero, regs.result);
                 stubcc.linkExitDirect(isZero, stubcc.masm.label());
 
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -2648,44 +2648,36 @@ FrameState::allocForBinary(FrameEntry *l
         break;
 
       default:
         JS_NOT_REACHED("unknown op");
         return;
     }
 
     /*
-     * Data is a little more complicated. If the op is MUL, not all CPUs
-     * have multiplication on immediates, so a register is needed. Also,
-     * if the op is not commutative, the LHS _must_ be in a register.
+     * Allocate data registers. If the op is not commutative, the LHS
+     * _must_ be in a register.
      */
     JS_ASSERT_IF(lhs->isConstant(), !rhs->isConstant());
     JS_ASSERT_IF(rhs->isConstant(), !lhs->isConstant());
 
     if (!alloc.lhsData.isSet()) {
         if (backingLeft->data.inMemory()) {
             alloc.lhsData = tempRegForData(lhs);
             pinReg(alloc.lhsData.reg());
-        } else if (op == JSOP_MUL || !commu) {
+        } else if (!commu) {
             JS_ASSERT(lhs->isConstant());
             alloc.lhsData = allocReg();
             alloc.extraFree = alloc.lhsData;
             masm.move(Imm32(lhs->getValue().toInt32()), alloc.lhsData.reg());
         }
     }
-    if (!alloc.rhsData.isSet()) {
-        if (backingRight->data.inMemory()) {
-            alloc.rhsData = tempRegForData(rhs);
-            pinReg(alloc.rhsData.reg());
-        } else if (op == JSOP_MUL) {
-            JS_ASSERT(rhs->isConstant());
-            alloc.rhsData = allocReg();
-            alloc.extraFree = alloc.rhsData;
-            masm.move(Imm32(rhs->getValue().toInt32()), alloc.rhsData.reg());
-        }
+    if (!alloc.rhsData.isSet() && backingRight->data.inMemory()) {
+        alloc.rhsData = tempRegForData(rhs);
+        pinReg(alloc.rhsData.reg());
     }
 
     alloc.lhsNeedsRemat = false;
     alloc.rhsNeedsRemat = false;
     alloc.resultHasRhs = false;
     alloc.undoResult = false;
 
     if (!needsResult)