From b3e8f5ef49fe282a668e6bd97c5db15140831c64 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Thu, 9 Jul 2009 11:46:30 +0000 Subject: [PATCH] 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 | 59 ++++++++------------------ test/mjsunit/date-parse.js | 10 ++--- test/mjsunit/smi-ops.js | 86 ++++++++++++++++++++------------------ 3 files changed, 67 insertions(+), 88 deletions(-) diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 149b0afba7..c4f3d0c6d6 100644 --- a/src/x64/codegen-x64.cc +++ b/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) { diff --git a/test/mjsunit/date-parse.js b/test/mjsunit/date-parse.js index 56ceba3841..bb7ecd27cc 100644 --- a/test/mjsunit/date-parse.js +++ b/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); } diff --git a/test/mjsunit/smi-ops.js b/test/mjsunit/smi-ops.js index 55203273ae..284050d36c 100644 --- a/test/mjsunit/smi-ops.js +++ b/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);