X64: Fixed more bad smi operations.

Review URL: http://codereview.chromium.org/155281


git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2415 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/x64/codegen-x64.cc

@@ -2724,12 +2724,6 @@ class DeferredPrefixCountOperation: public DeferredCode {
 
 
 void DeferredPrefixCountOperation::Generate() {
-  // Undo the optimistic smi operation.
-  if (is_increment_) {
-    __ subq(dst_, Immediate(Smi::FromInt(1)));
-  } else {
-    __ addq(dst_, Immediate(Smi::FromInt(1)));
-  }
   __ push(dst_);
   __ InvokeBuiltin(Builtins::TO_NUMBER, CALL_FUNCTION);
   __ push(rax);
@@ -2765,12 +2759,6 @@ class DeferredPostfixCountOperation: public DeferredCode {
 
 
 void DeferredPostfixCountOperation::Generate() {
-  // Undo the optimistic smi operation.
-  if (is_increment_) {
-    __ subq(dst_, Immediate(Smi::FromInt(1)));
-  } else {
-    __ addq(dst_, Immediate(Smi::FromInt(1)));
-  }
   __ push(dst_);
   __ InvokeBuiltin(Builtins::TO_NUMBER, CALL_FUNCTION);
 
@@ -2827,19 +2815,6 @@ void CodeGenerator::VisitCountOperation(CountOperation* node) {
     // Ensure the new value is writable.
     frame_->Spill(new_value.reg());
 
-    // In order to combine the overflow and the smi tag check, we need
-    // to be able to allocate a byte register.  We attempt to do so
-    // without spilling.  If we fail, we will generate separate overflow
-    // and smi tag checks.
-    //
-    // We allocate and clear the temporary register before
-    // performing the count operation since clearing the register using
-    // xor will clear the overflow flag.
-    Result tmp = allocator_->AllocateWithoutSpilling();
-
-    // Clear scratch register to prepare it for setcc after the operation below.
-    __ xor_(kScratchRegister, kScratchRegister);
-
     DeferredCode* deferred = NULL;
     if (is_postfix) {
       deferred = new DeferredPostfixCountOperation(new_value.reg(),
@@ -2850,25 +2825,26 @@ void CodeGenerator::VisitCountOperation(CountOperation* node) {
                                                   is_increment);
     }
 
+    Result tmp = allocator_->AllocateWithoutSpilling();
+    ASSERT(kSmiTagMask == 1 && kSmiTag == 0);
+    __ movl(tmp.reg(), Immediate(kSmiTagMask));
+    // Smi test.
+    __ movq(kScratchRegister, new_value.reg());
     if (is_increment) {
-      __ addq(new_value.reg(), Immediate(Smi::FromInt(1)));
+      __ addl(kScratchRegister, Immediate(Smi::FromInt(1)));
     } else {
-      __ subq(new_value.reg(), Immediate(Smi::FromInt(1)));
+      __ subl(kScratchRegister, Immediate(Smi::FromInt(1)));
     }
-
-    // If the count operation didn't overflow and the result is a valid
-    // smi, we're done. Otherwise, we jump to the deferred slow-case
-    // code.
-
-    // We combine the overflow and the smi tag check.
-    __ setcc(overflow, kScratchRegister);
-    __ or_(kScratchRegister, new_value.reg());
-    __ testl(kScratchRegister, Immediate(kSmiTagMask));
+    // deferred->Branch(overflow);
+    __ cmovl(overflow, kScratchRegister, tmp.reg());
+    __ testl(kScratchRegister, tmp.reg());
     tmp.Unuse();
     deferred->Branch(not_zero);
+    __ movq(new_value.reg(), kScratchRegister);
 
     deferred->BindExit();
 
+
     // Postfix: store the old value in the allocated slot under the
     // reference.
     if (is_postfix) frame_->SetElementAt(target.size(), &old_value);
@@ -5081,12 +5057,12 @@ void CodeGenerator::LikelySmiBinaryOperation(Token::Value op,
     // Perform the operation.
     switch (op) {
       case Token::SAR:
-        __ sar(answer.reg());
+        __ sarl(answer.reg());
         // No checks of result necessary
         break;
       case Token::SHR: {
         Label result_ok;
-        __ shr(answer.reg());
+        __ shrl(answer.reg());
         // Check that the *unsigned* result fits in a smi.  Neither of
         // the two high-order bits can be set:
         //  * 0x80000000: high bit would be lost when smi tagging.
@@ -6813,7 +6789,6 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
         __ testl(rax, Immediate(1));
         __ j(not_zero, &operand_conversion_failure);
       } else {
-        // TODO(X64): Verify that SSE3 is always supported, drop this code.
         // Check if right operand is int32.
         __ fist_s(Operand(rsp, 0 * kPointerSize));
         __ fild_s(Operand(rsp, 0 * kPointerSize));
@@ -6840,9 +6815,9 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
         case Token::BIT_OR:  __ or_(rax, rcx); break;
         case Token::BIT_AND: __ and_(rax, rcx); break;
         case Token::BIT_XOR: __ xor_(rax, rcx); break;
-        case Token::SAR: __ sar(rax); break;
-        case Token::SHL: __ shl(rax); break;
-        case Token::SHR: __ shr(rax); break;
+        case Token::SAR: __ sarl(rax); break;
+        case Token::SHL: __ shll(rax); break;
+        case Token::SHR: __ shrl(rax); break;
         default: UNREACHABLE();
       }
       if (op_ == Token::SHR) {

test/mjsunit/date-parse.js

@@ -33,16 +33,16 @@
 
 function testDateParse(string) {
   var d = Date.parse(string);
-  assertEquals(946713600000, d, string);
+  assertEquals(946713600000, d, "parse: " + string);
 };
 
 
 // For local time we just test that parsing returns non-NaN positive
 // number of milliseconds to make it timezone independent.
 function testDateParseLocalTime(string) {
-  var d = Date.parse(string);
-  assertTrue(!isNaN(d), string + " is NaN.");
-  assertTrue(d > 0, string + " <= 0.");
+  var d = Date.parse("parse-local-time:" + string);
+  assertTrue(!isNaN(d), "parse-local-time: " + string + " is NaN.");
+  assertTrue(d > 0, "parse-local-time: " + string + " <= 0.");
 };
 
 
@@ -51,7 +51,7 @@ function testDateParseMisc(array) {
   var string = array[0];
   var expected = array[1];
   var d = Date.parse(string);
-  assertEquals(expected, d, string);
+  assertEquals(expected, d, "parse-misc: " + string);
 }
 
 

test/mjsunit/smi-ops.js

@@ -56,15 +56,15 @@ function Add100Reversed(x) {
 
 assertEquals(1, Add1(0));  // fast case
 assertEquals(1, Add1Reversed(0));  // fast case
-assertEquals(SMI_MAX + ONE, Add1(SMI_MAX));  // overflow
-assertEquals(SMI_MAX + ONE, Add1Reversed(SMI_MAX));  // overflow
+assertEquals(SMI_MAX + ONE, Add1(SMI_MAX), "smimax + 1");
+assertEquals(SMI_MAX + ONE, Add1Reversed(SMI_MAX), "1 + smimax");
 assertEquals(42 + ONE, Add1(OBJ_42));  // non-smi
 assertEquals(42 + ONE, Add1Reversed(OBJ_42));  // non-smi
 
 assertEquals(100, Add100(0));  // fast case
 assertEquals(100, Add100Reversed(0));  // fast case
-assertEquals(SMI_MAX + ONE_HUNDRED, Add100(SMI_MAX));  // overflow
-assertEquals(SMI_MAX + ONE_HUNDRED, Add100Reversed(SMI_MAX));  // overflow
+assertEquals(SMI_MAX + ONE_HUNDRED, Add100(SMI_MAX), "smimax + 100");
+assertEquals(SMI_MAX + ONE_HUNDRED, Add100Reversed(SMI_MAX), " 100 + smimax");
 assertEquals(42 + ONE_HUNDRED, Add100(OBJ_42));  // non-smi
 assertEquals(42 + ONE_HUNDRED, Add100Reversed(OBJ_42));  // non-smi
 
@@ -148,8 +148,8 @@ assertEquals(21, Sar1(OBJ_42));
 assertEquals(0, Shr1Reversed(OBJ_42));
 assertEquals(0, Sar1Reversed(OBJ_42));
 
-assertEquals(6, Shr100(100));
-assertEquals(6, Sar100(100));
+assertEquals(6, Shr100(100), "100 >>> 100");
+assertEquals(6, Sar100(100), "100 >> 100");
 assertEquals(12, Shr100Reversed(99));
 assertEquals(12, Sar100Reversed(99));
 assertEquals(201326592, Shr100(SMI_MIN));
@@ -201,17 +201,17 @@ assertEquals(0x16, x ^ y);
 var v = 0;
 assertEquals(-1, ~v);
 v = SMI_MIN;
-assertEquals(0x3fffffff, ~v);
+assertEquals(0x3fffffff, ~v, "~smimin");
 v = SMI_MAX;
-assertEquals(-0x40000000, ~v);
+assertEquals(-0x40000000, ~v, "~smimax");
 
 // Overflowing ++ and --.
 v = SMI_MAX;
 v++;
-assertEquals(0x40000000, v);
+assertEquals(0x40000000, v, "smimax++");
 v = SMI_MIN;
 v--;
-assertEquals(-0x40000001, v);
+assertEquals(-0x40000001, v, "smimin--");
 
 // Not actually Smi operations.
 // Check that relations on unary ops work.
@@ -234,14 +234,14 @@ assertEquals(-2.25, -(v * v));
 var x1 = 0x10000000;
 var x2 = 0x40000002;
 var x3 = 0x40000000;
-assertEquals(0x40000000, x1 << (x2 - x3));
+assertEquals(0x40000000, x1 << (x2 - x3), "0x10000000<<1(1)");
 
 // Smi input to bitop gives non-smi result where the rhs could be overwritten
 // if it were a float, but it isn't.
 x1 = 0x10000000
 x2 = 4
 x3 = 2
-assertEquals(0x40000000, x1 << (x2 - x3));
+assertEquals(0x40000000, x1 << (x2 - x3), "0x10000000<<2(2)");
 
 
 // Test shift operators on non-smi inputs, giving smi and non-smi results.
@@ -258,12 +258,12 @@ function testShiftNonSmis() {
   assertEquals(neg_non_smi, (neg_non_smi) >> 0);
   assertEquals(neg_non_smi + 0x100000000, (neg_non_smi) >>> 0);
   assertEquals(neg_non_smi, (neg_non_smi) << 0);
-  assertEquals(pos_smi, (pos_smi) >> 0);
-  assertEquals(pos_smi, (pos_smi) >>> 0);
-  assertEquals(pos_smi, (pos_smi) << 0);
-  assertEquals(neg_smi, (neg_smi) >> 0);
-  assertEquals(neg_smi + 0x100000000, (neg_smi) >>> 0);
-  assertEquals(neg_smi, (neg_smi) << 0);
+  assertEquals(pos_smi, (pos_smi) >> 0, "possmi >> 0");
+  assertEquals(pos_smi, (pos_smi) >>> 0, "possmi >>>0");
+  assertEquals(pos_smi, (pos_smi) << 0, "possmi << 0");
+  assertEquals(neg_smi, (neg_smi) >> 0, "negsmi >> 0");
+  assertEquals(neg_smi + 0x100000000, (neg_smi) >>> 0, "negsmi >>> 0");
+  assertEquals(neg_smi, (neg_smi) << 0), "negsmi << 0";
 
   assertEquals(pos_non_smi / 2, (pos_non_smi) >> 1);
   assertEquals(pos_non_smi / 2, (pos_non_smi) >>> 1);
@@ -283,18 +283,22 @@ function testShiftNonSmis() {
   assertEquals(-0x46536000, (pos_non_smi + 0.5) << 3);
   assertEquals(0x73594000, (pos_non_smi + 0.5) << 4);
 
-  assertEquals(neg_non_smi / 2, (neg_non_smi) >> 1);
-  assertEquals(neg_non_smi / 2 + 0x100000000 / 2, (neg_non_smi) >>> 1);
+  assertEquals(neg_non_smi / 2, (neg_non_smi) >> 1, "negnonsmi >> 1");
+
+  assertEquals(neg_non_smi / 2 + 0x100000000 / 2, (neg_non_smi) >>> 1,
+               "negnonsmi >>> 1");
   assertEquals(0x1194D800, (neg_non_smi) << 1);
   assertEquals(neg_non_smi / 8, (neg_non_smi) >> 3);
   assertEquals(neg_non_smi / 8 + 0x100000000 / 8, (neg_non_smi) >>> 3);
   assertEquals(0x46536000, (neg_non_smi) << 3);
   assertEquals(-0x73594000, (neg_non_smi) << 4);
   assertEquals(neg_non_smi, (neg_non_smi - 0.5) >> 0);
-  assertEquals(neg_non_smi + 0x100000000, (neg_non_smi - 0.5) >>> 0);
+  assertEquals(neg_non_smi + 0x100000000, (neg_non_smi - 0.5) >>> 0,
+               "negnonsmi.5 >>> 0");
   assertEquals(neg_non_smi, (neg_non_smi - 0.5) << 0);
   assertEquals(neg_non_smi / 2, (neg_non_smi - 0.5) >> 1);
-  assertEquals(neg_non_smi / 2 + 0x100000000 / 2, (neg_non_smi - 0.5) >>> 1);
+  assertEquals(neg_non_smi / 2 + 0x100000000 / 2, (neg_non_smi - 0.5) >>> 1,
+               "negnonsmi.5 >>> 1");
   assertEquals(0x1194D800, (neg_non_smi - 0.5) << 1);
   assertEquals(neg_non_smi / 8, (neg_non_smi - 0.5) >> 3);
   assertEquals(neg_non_smi / 8 + 0x100000000 / 8, (neg_non_smi - 0.5) >>> 3);
@@ -308,9 +312,9 @@ function testShiftNonSmis() {
   assertEquals(pos_smi / 8, (pos_smi) >>> 3);
   assertEquals(-0x2329b000, (pos_smi) << 3);
   assertEquals(0x73594000, (pos_smi) << 5);
-  assertEquals(pos_smi, (pos_smi + 0.5) >> 0);
-  assertEquals(pos_smi, (pos_smi + 0.5) >>> 0);
-  assertEquals(pos_smi, (pos_smi + 0.5) << 0);
+  assertEquals(pos_smi, (pos_smi + 0.5) >> 0, "possmi.5 >> 0");
+  assertEquals(pos_smi, (pos_smi + 0.5) >>> 0, "possmi.5 >>> 0");
+  assertEquals(pos_smi, (pos_smi + 0.5) << 0, "possmi.5 << 0");
   assertEquals(pos_smi / 2, (pos_smi + 0.5) >> 1);
   assertEquals(pos_smi / 2, (pos_smi + 0.5) >>> 1);
   assertEquals(pos_non_smi, (pos_smi + 0.5) << 1);
@@ -326,9 +330,9 @@ function testShiftNonSmis() {
   assertEquals(neg_smi / 8 + 0x100000000 / 8, (neg_smi) >>> 3);
   assertEquals(0x46536000, (neg_smi) << 4);
   assertEquals(-0x73594000, (neg_smi) << 5);
-  assertEquals(neg_smi, (neg_smi - 0.5) >> 0);
-  assertEquals(neg_smi + 0x100000000, (neg_smi - 0.5) >>> 0);
-  assertEquals(neg_smi, (neg_smi - 0.5) << 0);
+  assertEquals(neg_smi, (neg_smi - 0.5) >> 0, "negsmi.5 >> 0");
+  assertEquals(neg_smi + 0x100000000, (neg_smi - 0.5) >>> 0, "negsmi.5 >>> 0");
+  assertEquals(neg_smi, (neg_smi - 0.5) << 0, "negsmi.5 << 0");
   assertEquals(neg_smi / 2, (neg_smi - 0.5) >> 1);
   assertEquals(neg_smi / 2 + 0x100000000 / 2, (neg_smi - 0.5) >>> 1);
   assertEquals(neg_non_smi, (neg_smi - 0.5) << 1);
@@ -349,12 +353,12 @@ function testShiftNonSmis() {
   assertEquals(neg_non_smi, (neg_32 + neg_non_smi) >> 0);
   assertEquals(neg_non_smi + 0x100000000, (neg_32 + neg_non_smi) >>> 0);
   assertEquals(neg_non_smi, (neg_32 + neg_non_smi) << 0);
-  assertEquals(pos_smi, (two_32 + pos_smi) >> 0);
-  assertEquals(pos_smi, (two_32 + pos_smi) >>> 0);
-  assertEquals(pos_smi, (two_32 + pos_smi) << 0);
-  assertEquals(neg_smi, (neg_32 + neg_smi) >> 0);
+  assertEquals(pos_smi, (two_32 + pos_smi) >> 0, "2^32+possmi >> 0");
+  assertEquals(pos_smi, (two_32 + pos_smi) >>> 0, "2^32+possmi >>> 0");
+  assertEquals(pos_smi, (two_32 + pos_smi) << 0, "2^32+possmi << 0");
+  assertEquals(neg_smi, (neg_32 + neg_smi) >> 0, "2^32+negsmi >> 0");
   assertEquals(neg_smi + 0x100000000, (neg_32 + neg_smi) >>> 0);
-  assertEquals(neg_smi, (neg_32 + neg_smi) << 0);
+  assertEquals(neg_smi, (neg_32 + neg_smi) << 0, "2^32+negsmi << 0");
 
   assertEquals(pos_non_smi / 2, (two_32 + pos_non_smi) >> 1);
   assertEquals(pos_non_smi / 2, (two_32 + pos_non_smi) >>> 1);
@@ -419,9 +423,9 @@ function testShiftNonSmis() {
   assertEquals((neg_smi + 0x100000000) / 8, (neg_32 + neg_smi) >>> 3);
   assertEquals(0x46536000, (neg_32 + neg_smi) << 4);
   assertEquals(-0x73594000, (neg_32 + neg_smi) << 5);
-  assertEquals(neg_smi, (neg_32 + neg_smi - 0.5) >> 0);
+  assertEquals(neg_smi, (neg_32 + neg_smi - 0.5) >> 0, "-2^32+negsmi.5 >> 0");
   assertEquals(neg_smi + 0x100000000, (neg_32 + neg_smi - 0.5) >>> 0);
-  assertEquals(neg_smi, (neg_32 + neg_smi - 0.5) << 0);
+  assertEquals(neg_smi, (neg_32 + neg_smi - 0.5) << 0, "-2^32+negsmi.5 << 0");
   assertEquals(neg_smi / 2, (neg_32 + neg_smi - 0.5) >> 1);
   assertEquals(neg_smi / 2 + 0x100000000 / 2, (neg_32 + neg_smi - 0.5) >>> 1);
   assertEquals(neg_non_smi, (neg_32 + neg_smi - 0.5) << 1);
@@ -447,9 +451,9 @@ function testShiftNonSmis() {
   assertEquals(pos_smi, (pos_smi) >> zero);
   assertEquals(pos_smi, (pos_smi) >>> zero);
   assertEquals(pos_smi, (pos_smi) << zero);
-  assertEquals(neg_smi, (neg_smi) >> zero);
+  assertEquals(neg_smi, (neg_smi) >> zero, "negsmi >> zero");
   assertEquals(neg_smi + 0x100000000, (neg_smi) >>> zero);
-  assertEquals(neg_smi, (neg_smi) << zero);
+  assertEquals(neg_smi, (neg_smi) << zero, "negsmi << zero");
 
   assertEquals(pos_non_smi / 2, (pos_non_smi) >> one);
   assertEquals(pos_non_smi / 2, (pos_non_smi) >>> one);
@@ -543,9 +547,9 @@ function testShiftNonSmis() {
   assertEquals(pos_smi, (pos_smi) >> zero);
   assertEquals(pos_smi, (pos_smi) >>> zero);
   assertEquals(pos_smi, (pos_smi) << zero);
-  assertEquals(neg_smi, (neg_smi) >> zero);
+  assertEquals(neg_smi, (neg_smi) >> zero, "negsmi >> zero(2)");
   assertEquals(neg_smi + 0x100000000, (neg_smi) >>> zero);
-  assertEquals(neg_smi, (neg_smi) << zero);
+  assertEquals(neg_smi, (neg_smi) << zero, "negsmi << zero(2)");
 
   assertEquals(pos_non_smi / 2, (pos_non_smi) >> one);
   assertEquals(pos_non_smi / 2, (pos_non_smi) >>> one);
@@ -609,9 +613,9 @@ function testShiftNonSmis() {
   assertEquals(neg_smi / 8 + 0x100000000 / 8, (neg_smi) >>> three);
   assertEquals(0x46536000, (neg_smi) << four);
   assertEquals(-0x73594000, (neg_smi) << five);
-  assertEquals(neg_smi, (neg_smi - 0.5) >> zero);
+  assertEquals(neg_smi, (neg_smi - 0.5) >> zero, "negsmi.5 >> zero");
   assertEquals(neg_smi + 0x100000000, (neg_smi - 0.5) >>> zero);
-  assertEquals(neg_smi, (neg_smi - 0.5) << zero);
+  assertEquals(neg_smi, (neg_smi - 0.5) << zero, "negsmi.5 << zero");
   assertEquals(neg_smi / 2, (neg_smi - 0.5) >> one);
   assertEquals(neg_smi / 2 + 0x100000000 / 2, (neg_smi - 0.5) >>> one);
   assertEquals(neg_non_smi, (neg_smi - 0.5) << one);