Bug 787906 - Don't emit a shift instruction for x >>> 0. r=dvander

js/src/assembler/assembler/X86Assembler.h

@@ -973,6 +973,8 @@ public:
     
     void shrl_i8r(int imm, RegisterID dst)
     {
+        js::JaegerSpew(js::JSpew_Insns,
+                       IPFX "shrl       $%d, %s\n", MAYBE_PAD, imm, nameIReg(4, dst));
         if (imm == 1)
             m_formatter.oneByteOp(OP_GROUP2_Ev1, GROUP2_OP_SHR, dst);
         else {
@@ -983,6 +985,8 @@ public:
     
     void shrl_CLr(RegisterID dst)
     {
+        js::JaegerSpew(js::JSpew_Insns,
+                       IPFX "shrl       %%cl, %s\n", MAYBE_PAD, nameIReg(4, dst));
         m_formatter.oneByteOp(OP_GROUP2_EvCL, GROUP2_OP_SHR, dst);
     }
 

js/src/ion/LIR-Common.h

@@ -1198,7 +1198,7 @@ class LBitOpV : public LCallInstructionHelper<1, 2 * BOX_PIECES, 0>
 
 // Shift operation, taking two 32-bit integers as inputs and returning
 // a 32-bit integer result as an output.
-class LShiftI : public LInstructionHelper<1, 2, 0>
+class LShiftI : public LBinaryMath<0>
 {
     JSOp op_;
 

js/src/ion/arm/CodeGenerator-arm.cpp

@@ -680,67 +680,66 @@ CodeGeneratorARM::visitBitOpI(LBitOpI *ins)
 bool
 CodeGeneratorARM::visitShiftI(LShiftI *ins)
 {
-    const LAllocation *lhs = ins->getOperand(0);
+    Register lhs = ToRegister(ins->lhs());
-    const LAllocation *rhs = ins->getOperand(1);
+    const LAllocation *rhs = ins->rhs();
-    const LDefinition *dest = ins->getDef(0);
+    Register dest = ToRegister(ins->output());
 
-    // The shift amounts should be AND'ed into the 0-31 range since arm shifts
+    if (rhs->isConstant()) {
-    // by the lower byte of the register (it will attempt to shift by 250 if you
+        int32_t shift = ToInt32(rhs) & 0x1F;
-    // ask it to, and the result will probably not be what you want.
+        switch (ins->bitop()) {
-    if (!rhs->isConstant()) {
+          case JSOP_LSH:
-        masm.ma_and(Imm32(0x1f), ToRegister(rhs), ToRegister(dest));
+            if (shift)
-        rhs = dest->output();
+                masm.ma_lsl(Imm32(shift), lhs, dest);
-    }
+            else
-
+                masm.ma_mov(lhs, dest);
-    switch (ins->bitop()) {
-        case JSOP_LSH:
-          if (rhs->isConstant()) {
-                masm.ma_lsl(Imm32(ToInt32(rhs) & 0x1F), ToRegister(lhs), ToRegister(dest));
-          } else {
-                masm.ma_lsl(ToRegister(rhs), ToRegister(lhs), ToRegister(dest));
-          }
             break;
-        case JSOP_RSH:
+          case JSOP_RSH:
-          if (rhs->isConstant()) {
+            if (shift)
-              if ((ToInt32(rhs) & 0x1f) != 0) {
+                masm.ma_asr(Imm32(shift), lhs, dest);
-                  masm.ma_asr(Imm32(ToInt32(rhs) & 0x1F), ToRegister(lhs), ToRegister(dest));
+            else
-              } else {
+                masm.ma_mov(lhs, dest);
-                  masm.ma_mov(ToRegister(lhs), ToRegister(dest));
-              }
-          } else {
-                masm.ma_asr(ToRegister(rhs), ToRegister(lhs), ToRegister(dest));
-          }
             break;
-        case JSOP_URSH: {
+          case JSOP_URSH:
-            MUrsh *ursh = ins->mir()->toUrsh();
+            if (shift) {
-            if (rhs->isConstant()) {
+                masm.ma_lsr(Imm32(shift), lhs, dest);
-                if ((ToInt32(rhs) & 0x1f) != 0) {
-                    masm.ma_lsr(Imm32(ToInt32(rhs) & 0x1F), ToRegister(lhs), ToRegister(dest));
-                } else {
-                    masm.ma_mov(ToRegister(lhs), ToRegister(dest));
-                }
             } else {
-                masm.ma_lsr(ToRegister(rhs), ToRegister(lhs), ToRegister(dest));
+                // x >>> 0 can overflow.
-            }
+                masm.ma_mov(lhs, dest);
-
+                if (ins->mir()->toUrsh()->canOverflow()) {
-            // Note: this is an unsigned operation.
+                    masm.ma_cmp(dest, Imm32(0));
-            // We don't have a UINT32 type, so we will emulate this with INT32
+                    if (!bailoutIf(Assembler::LessThan, ins->snapshot()))
-            // The bit representation of an integer from ToInt32 and ToUint32 are the same.
+                        return false;
-            // So the inputs are ok.
-            // But we need to bring the output back again from UINT32 to INT32.
-            // Both representation overlap each other in the positive numbers. (in INT32)
-            // So there is only a problem when solution (in INT32) is negative.
-            if (ursh->canOverflow()) {
-                masm.ma_cmp(ToRegister(dest), Imm32(0));
-                if (!bailoutIf(Assembler::LessThan, ins->snapshot())) {
-                    return false;
                 }
             }
             break;
+          default:
+            JS_NOT_REACHED("Unexpected shift op");
+        }
+    } else {
+        // The shift amounts should be AND'ed into the 0-31 range since arm
+        // shifts by the lower byte of the register (it will attempt to shift
+        // by 250 if you ask it to).
+        masm.ma_and(Imm32(0x1F), ToRegister(rhs), dest);
+
+        switch (ins->bitop()) {
+          case JSOP_LSH:
+            masm.ma_lsl(dest, lhs, dest);
+            break;
+          case JSOP_RSH:
+            masm.ma_asr(dest, lhs, dest);
+            break;
+          case JSOP_URSH:
+            masm.ma_lsr(dest, lhs, dest);
+            if (ins->mir()->toUrsh()->canOverflow()) {
+                // x >>> 0 can overflow.
+                masm.ma_cmp(dest, Imm32(0));
+                if (!bailoutIf(Assembler::LessThan, ins->snapshot()))
+                    return false;
+            }
+            break;
+          default:
+            JS_NOT_REACHED("Unexpected shift op");
         }
-        default:
-            JS_NOT_REACHED("unexpected shift opcode");
-            return false;
     }
 
     return true;
@@ -756,12 +755,13 @@ CodeGeneratorARM::visitUrshD(LUrshD *ins)
     FloatRegister out = ToFloatRegister(ins->output());
 
     if (rhs->isConstant()) {
-        if ((ToInt32(rhs) & 0x1f) != 0)
+        int32_t shift = ToInt32(rhs) & 0x1F;
-            masm.ma_lsr(Imm32(ToInt32(rhs) & 0x1F), lhs, temp);
+        if (shift)
+            masm.ma_lsr(Imm32(shift), lhs, temp);
         else
             masm.ma_mov(lhs, temp);
     } else {
-        masm.ma_and(Imm32(0x1f), ToRegister(rhs), temp);
+        masm.ma_and(Imm32(0x1F), ToRegister(rhs), temp);
         masm.ma_lsr(temp, lhs, temp);
     }
 

js/src/ion/arm/Lowering-arm.cpp

@@ -217,8 +217,7 @@ LIRGeneratorARM::lowerForShift(LInstructionHelper<1, 2, 0> *ins, MDefinition *mi
 
     ins->setOperand(0, useRegister(lhs));
     ins->setOperand(1, useRegisterOrConstant(rhs));
-    return define(ins, mir,
+    return define(ins, mir);
-                  LDefinition(LDefinition::TypeFrom(mir->type()), LDefinition::DEFAULT));
 }
 
 bool

js/src/ion/shared/CodeGenerator-x86-shared.cpp

@@ -808,45 +808,54 @@ CodeGeneratorX86Shared::visitBitOpI(LBitOpI *ins)
 bool
 CodeGeneratorX86Shared::visitShiftI(LShiftI *ins)
 {
-    const LAllocation *lhs = ins->getOperand(0);
+    Register lhs = ToRegister(ins->lhs());
-    const LAllocation *rhs = ins->getOperand(1);
+    const LAllocation *rhs = ins->rhs();
 
-    switch (ins->bitop()) {
+    if (rhs->isConstant()) {
-        case JSOP_LSH:
+        int32_t shift = ToInt32(rhs) & 0x1F;
-            if (rhs->isConstant())
+        switch (ins->bitop()) {
-                masm.shll(Imm32(ToInt32(rhs) & 0x1F), ToRegister(lhs));
+          case JSOP_LSH:
-            else
+            if (shift)
-                masm.shll_cl(ToRegister(lhs));
+                masm.shll(Imm32(shift), lhs);
             break;
-        case JSOP_RSH:
+          case JSOP_RSH:
-            if (rhs->isConstant())
+            if (shift)
-                masm.sarl(Imm32(ToInt32(rhs) & 0x1F), ToRegister(lhs));
+                masm.sarl(Imm32(shift), lhs);
-            else
-                masm.sarl_cl(ToRegister(lhs));
             break;
-        case JSOP_URSH: {
+          case JSOP_URSH:
-            MUrsh *ursh = ins->mir()->toUrsh(); 
+            if (shift) {
-            if (rhs->isConstant())
+                masm.shrl(Imm32(shift), lhs);
-                masm.shrl(Imm32(ToInt32(rhs) & 0x1F), ToRegister(lhs));
+            } else if (ins->mir()->toUrsh()->canOverflow()) {
-            else
+                // x >>> 0 can overflow.
-                masm.shrl_cl(ToRegister(lhs));
+                masm.testl(lhs, lhs);
- 
+                if (!bailoutIf(Assembler::Signed, ins->snapshot()))
-            // Note: this is an unsigned operation.
-            // We don't have a UINT32 type, so we will emulate this with INT32
-            // The bit representation of an integer from ToInt32 and ToUint32 are the same.
-            // So the inputs are ok.
-            // But we need to bring the output back again from UINT32 to INT32.
-            // Both representation overlap each other in the positive numbers. (in INT32)
-            // So there is only a problem when solution (in INT32) is negative.
-            if (ursh->canOverflow()) {
-                masm.cmpl(ToOperand(lhs), Imm32(0));
-                if (!bailoutIf(Assembler::LessThan, ins->snapshot()))
                     return false;
             }
             break;
+          default:
+            JS_NOT_REACHED("Unexpected shift op");
+        }
+    } else {
+        JS_ASSERT(ToRegister(rhs) == ecx);
+        switch (ins->bitop()) {
+          case JSOP_LSH:
+            masm.shll_cl(lhs);
+            break;
+          case JSOP_RSH:
+            masm.sarl_cl(lhs);
+            break;
+          case JSOP_URSH:
+            masm.shrl_cl(lhs);
+            if (ins->mir()->toUrsh()->canOverflow()) {
+                // x >>> 0 can overflow.
+                masm.testl(lhs, lhs);
+                if (!bailoutIf(Assembler::Signed, ins->snapshot()))
+                    return false;
+            }
+            break;
+          default:
+            JS_NOT_REACHED("Unexpected shift op");
         }
-        default:
-            JS_NOT_REACHED("unexpected shift opcode");
     }
 
     return true;
@@ -862,7 +871,9 @@ CodeGeneratorX86Shared::visitUrshD(LUrshD *ins)
     FloatRegister out = ToFloatRegister(ins->output());
 
     if (rhs->isConstant()) {
-        masm.shrl(Imm32(ToInt32(rhs) & 0x1F), lhs);
+        int32_t shift = ToInt32(rhs) & 0x1F;
+        if (shift)
+            masm.shrl(Imm32(shift), lhs);
     } else {
         JS_ASSERT(ToRegister(rhs) == ecx);
         masm.shrl_cl(lhs);