X64: Fix error in division & modulus, adjust mjsunit test status, fix lint er...

src/x64/codegen-x64.cc

 // Copyright 2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 4833 matching lines @@
                                             smi_value,
                                             overwrite_mode);
       }
       __ testl(operand->reg(), Immediate(kSmiTagMask));
       deferred->Branch(not_zero);
       // A smi currently fits in a 32-bit Immediate.
       __ addl(operand->reg(), Immediate(smi_value));
       Label add_success;
       __ j(no_overflow, &add_success);
       __ subl(operand->reg(), Immediate(smi_value));
-      __ movsxlq(operand->reg(), operand->reg());
       deferred->Jump();
       __ bind(&add_success);
-      __ movsxlq(operand->reg(), operand->reg());
       deferred->BindExit();
       frame_->Push(operand);
       break;
     }
     // TODO(X64): Move other implementations from ia32 to here.
     default: {
       Result constant_operand(value);
       if (reversed) {
         LikelySmiBinaryOperation(op, &constant_operand, operand,
                                  overwrite_mode);
@@ skipping 90 matching lines @@
     } else {
       // Use the quotient register as a scratch for the tag check.
       if (!left_is_in_rax) __ movq(rax, left->reg());
       left_is_in_rax = false;  // About to destroy the value in rax.
       __ or_(rax, right->reg());
       ASSERT(kSmiTag == 0);  // Adjust test if not the case.
       __ testl(rax, Immediate(kSmiTagMask));
     }
     deferred->Branch(not_zero);
 
-    if (!left_is_in_rax) __ movq(rax, left->reg());
-    // Sign extend rax into rdx:rax.
-    __ cqo();
+    // All operations on the smi values are on 32-bit registers, which are
+    // zero-extended into 64-bits by all 32-bit operations.
+    if (!left_is_in_rax) __ movl(rax, left->reg());
+    // Sign extend eax into edx:eax.
+    __ cdq();
     // Check for 0 divisor.
-    __ testq(right->reg(), right->reg());
+    __ testl(right->reg(), right->reg());
     deferred->Branch(zero);
     // Divide rdx:rax by the right operand.
-    __ idiv(right->reg());
+    __ idivl(right->reg());
 
     // Complete the operation.
     if (op == Token::DIV) {
       // Check for negative zero result.  If result is zero, and divisor
-      // is negative, return a floating point negative zero.  The
-      // virtual frame is unchanged in this block, so local control flow
-      // can use a Label rather than a JumpTarget.
+      // is negative, return a floating point negative zero.  The jump
+      // to non_zero_result is safe w.r.t. the frame.
       Label non_zero_result;
-      __ testq(left->reg(), left->reg());
+      __ testl(left->reg(), left->reg());
       __ j(not_zero, &non_zero_result);
-      __ testq(right->reg(), right->reg());
+      __ testl(right->reg(), right->reg());
       deferred->Branch(negative);
       __ bind(&non_zero_result);
       // Check for the corner case of dividing the most negative smi by
       // -1. We cannot use the overflow flag, since it is not set by
       // idiv instruction.
       ASSERT(kSmiTag == 0 && kSmiTagSize == 1);
-      __ cmpq(rax, Immediate(0x40000000));
+      __ cmpl(rax, Immediate(0x40000000));
       deferred->Branch(equal);
       // Check that the remainder is zero.
-      __ testq(rdx, rdx);
+      __ testl(rdx, rdx);
       deferred->Branch(not_zero);
       // Tag the result and store it in the quotient register.
       ASSERT(kSmiTagSize == times_2);  // adjust code if not the case
       __ lea(rax, Operand(rax, rax, times_1, kSmiTag));
       deferred->BindExit();
       left->Unuse();
       right->Unuse();
       frame_->Push(&quotient);
     } else {
       ASSERT(op == Token::MOD);
       // Check for a negative zero result.  If the result is zero, and
       // the dividend is negative, return a floating point negative
-      // zero.  The frame is unchanged in this block, so local control
-      // flow can use a Label rather than a JumpTarget.
+      // zero.  The frame is unchanged between the jump to &non_zero_result
+      // and the target, so a Label can be used.

[Kevin Millikin (Chromium), 2009/07/30 07:52:05]

To me this is less precise than what was before.

Try: "The frame is unchanged on all paths reaching non_zero_result, so..." or
"There is no frame effect on any path reaching non_zero_result, so...".

And write the same thing at both sites so the reader doesn't have to puzzle out
whether it's the same reason or a different one.

       Label non_zero_result;
-      __ testq(rdx, rdx);
+      __ testl(rdx, rdx);
       __ j(not_zero, &non_zero_result);
-      __ testq(left->reg(), left->reg());
+      __ testl(left->reg(), left->reg());
       deferred->Branch(negative);
       __ bind(&non_zero_result);
       deferred->BindExit();
       left->Unuse();
       right->Unuse();
       frame_->Push(&remainder);
     }
     return;
   }
 
@@ skipping 24 matching lines @@
                                           answer.reg(),
                                           left->reg(),
                                           rcx,
                                           overwrite_mode);
     __ movq(answer.reg(), left->reg());
     __ or_(answer.reg(), rcx);
     __ testl(answer.reg(), Immediate(kSmiTagMask));
     deferred->Branch(not_zero);
 
     // Untag both operands.
-    __ movq(answer.reg(), left->reg());
-    __ sar(answer.reg(), Immediate(kSmiTagSize));
-    __ sar(rcx, Immediate(kSmiTagSize));
+    __ movl(answer.reg(), left->reg());
+    __ sarl(answer.reg(), Immediate(kSmiTagSize));
+    __ sarl(rcx, Immediate(kSmiTagSize));
     // Perform the operation.
     switch (op) {
       case Token::SAR:
         __ sarl(answer.reg());
         // No checks of result necessary
         break;
       case Token::SHR: {
         Label result_ok;
         __ shrl(answer.reg());
         // Check that the *unsigned* result fits in a smi.  Neither of
@@ skipping 85 matching lines @@
       __ sarl(answer.reg(), Immediate(kSmiTagSize));
       // Do multiplication of smis, leaving result in answer.
       __ imull(answer.reg(), right->reg());
       // Go slow on overflows.
       deferred->Branch(overflow);
       // Check for negative zero result.  If product is zero, and one
       // argument is negative, go to slow case.  The frame is unchanged
       // in this block, so local control flow can use a Label rather
       // than a JumpTarget.
       Label non_zero_result;
-      __ testq(answer.reg(), answer.reg());
+      __ testl(answer.reg(), answer.reg());
       __ j(not_zero, &non_zero_result);
       __ movq(answer.reg(), left->reg());
       __ or_(answer.reg(), right->reg());
       deferred->Branch(negative);
       __ xor_(answer.reg(), answer.reg());  // Positive 0 is correct.
       __ bind(&non_zero_result);
       break;
     }
 
     case Token::BIT_OR:
@@ skipping 1379 matching lines @@
     default:         return "GenericBinaryOpStub";
   }
 }
 
 void GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) {
   // Perform fast-case smi code for the operation (rax <op> rbx) and
   // leave result in register rax.
 
   // Smi check both operands.
   __ movq(rcx, rbx);
-  __ or_(rcx, rax);
+  __ or_(rcx, rax);  // The value in ecx is used for negative zero test later.
   __ testl(rcx, Immediate(kSmiTagMask));
   __ j(not_zero, slow);
 
   switch (op_) {
     case Token::ADD: {
       __ addl(rax, rbx);
       __ j(overflow, slow);  // The slow case rereads operands from the stack.
-      __ movsxlq(rax, rax);  // Sign extend eax into rax.
       break;
     }
 
     case Token::SUB: {
       __ subl(rax, rbx);
       __ j(overflow, slow);  // The slow case rereads operands from the stack.
-      __ movsxlq(rax, rax);  // Sign extend eax into rax.
       break;
     }
 
     case Token::MUL:
       // If the smi tag is 0 we can just leave the tag on one operand.
       ASSERT(kSmiTag == 0);  // adjust code below if not the case
       // Remove tag from one of the operands (but keep sign).
       __ sarl(rax, Immediate(kSmiTagSize));
       // Do multiplication.
       __ imull(rax, rbx);  // multiplication of smis; result in eax
       // Go slow on overflows.
       __ j(overflow, slow);
       // Check for negative zero result.
-      __ movsxlq(rax, rax);  // Sign extend eax into rax.
-      __ NegativeZeroTest(rax, rcx, slow);  // use rcx = x | y
+      __ NegativeZeroTest(rax, rcx, slow);  // ecx (not rcx) holds x | y.
       break;
 
     case Token::DIV:
-      // Sign extend rax into rdx:rax
-      // (also sign extends eax into edx if eax is Smi).
-      __ cqo();
+      // Sign extend eax into edx:eax.
+      __ cdq();
       // Check for 0 divisor.
-      __ testq(rbx, rbx);
+      __ testl(rbx, rbx);
       __ j(zero, slow);
-      // Divide rdx:rax by rbx (where rdx:rax is equivalent to the smi in eax).
-      __ idiv(rbx);
+      // Divide edx:eax by ebx (where edx:eax is equivalent to the smi in eax).
+      __ idivl(rbx);
       // Check that the remainder is zero.
-      __ testq(rdx, rdx);
+      __ testl(rdx, rdx);
       __ j(not_zero, slow);
       // Check for the corner case of dividing the most negative smi
       // by -1. We cannot use the overflow flag, since it is not set
       // by idiv instruction.
       ASSERT(kSmiTag == 0 && kSmiTagSize == 1);
       // TODO(X64): TODO(Smi): Smi implementation dependent constant.
       // Value is Smi::fromInt(-(1<<31)) / Smi::fromInt(-1)
-      __ cmpq(rax, Immediate(0x40000000));
+      __ cmpl(rax, Immediate(0x40000000));
       __ j(equal, slow);
       // Check for negative zero result.
-      __ NegativeZeroTest(rax, rcx, slow);  // use ecx = x | y
+      __ NegativeZeroTest(rax, rcx, slow);  // ecx (not rcx) holds x | y.
       // Tag the result and store it in register rax.
       ASSERT(kSmiTagSize == times_2);  // adjust code if not the case
       __ lea(rax, Operand(rax, rax, times_1, kSmiTag));
       break;
 
     case Token::MOD:
-      // Sign extend rax into rdx:rax
-      // (also sign extends eax into edx if eax is Smi).
-      __ cqo();
+      // Sign extend eax into edx:eax
+      __ cdq();
       // Check for 0 divisor.
-      __ testq(rbx, rbx);
+      __ testl(rbx, rbx);
       __ j(zero, slow);
-      // Divide rdx:rax by rbx.
-      __ idiv(rbx);
+      // Divide edx:eax by ebx.
+      __ idivl(rbx);
       // Check for negative zero result.
-      __ NegativeZeroTest(rdx, rcx, slow);  // use ecx = x | y
+      __ NegativeZeroTest(rdx, rcx, slow);  // ecx (not rcx) holds x | y.
       // Move remainder to register rax.
-      __ movq(rax, rdx);
+      __ movl(rax, rdx);
       break;
 
     case Token::BIT_OR:
       __ or_(rax, rbx);
       break;
 
     case Token::BIT_AND:
       __ and_(rax, rbx);
       break;
 
     case Token::BIT_XOR:
       ASSERT_EQ(0, kSmiTag);
       __ xor_(rax, rbx);
       break;
 
     case Token::SHL:
     case Token::SHR:
     case Token::SAR:
       // Move the second operand into register ecx.
-      __ movq(rcx, rbx);
+      __ movl(rcx, rbx);
       // Remove tags from operands (but keep sign).
       __ sarl(rax, Immediate(kSmiTagSize));
       __ sarl(rcx, Immediate(kSmiTagSize));
       // Perform the operation.
       switch (op_) {
         case Token::SAR:
           __ sarl(rax);
           // No checks of result necessary
           break;
         case Token::SHR:
@@ skipping 273 matching lines @@
 int CompareStub::MinorKey() {
   // Encode the two parameters in a unique 16 bit value.
   ASSERT(static_cast<unsigned>(cc_) < (1 << 15));
   return (static_cast<unsigned>(cc_) << 1) | (strict_ ? 1 : 0);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal