Improve register allocation of left shift operation. Add tests...

src/ia32/codegen-ia32.cc

 // Copyright 2006-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 5884 matching lines @@
 
     case Token::SHL:
     case Token::SHR:
     case Token::SAR:
       // Move right into ecx.
       // Left is in two registers already, so even if left or answer is ecx,
       // we can move right to it, and use the other one.
       // Right operand must be in register cl because x86 likes it that way.
       if (right->reg().is(ecx)) {
         // Right is already in the right place.  Left may be in the
-        // same register, which causes problems.  Use answer instead.
-        if (left->reg().is(ecx)) {
-          *left = answer;
-        }
+        // same register, which causes problems. Always use answer
+        // instead of left, even if left is not ecx, since this avoids
+        // spilling left.
+        *left = answer;
       } else if (left->reg().is(ecx)) {
         generator()->frame()->Spill(left->reg());
         __ mov(left->reg(), right->reg());
         *right = *left;
         *left = answer;  // Use copy of left in answer as left.
       } else if (answer.reg().is(ecx)) {
         __ mov(answer.reg(), right->reg());
         *right = answer;
       } else {
         Result reg_ecx = generator()->allocator()->Allocate(ecx);
         ASSERT(reg_ecx.is_valid());
         __ mov(ecx, right->reg());
         *right = reg_ecx;
+        // Answer and left both contain the left operand.  Use answer, so
+        // left is not spilled.
+        *left = answer;
       }
       ASSERT(left->reg().is_valid());
       ASSERT(!left->reg().is(ecx));
       ASSERT(right->reg().is(ecx));
       answer.Unuse();  // Answer may now be being used for left or right.
       // We will modify left and right, which we do not do in any other
       // binary operation.  The exits to slow code need to restore the
       // original values of left and right, or at least values that give
       // the same answer.
 
@@ skipping 29 matching lines @@
           ASSERT(kSmiTag == 0);
           __ shl(left->reg(), kSmiTagSize);
           __ shl(right->reg(), kSmiTagSize);
           enter()->Jump(left, right);
           result_ok.Bind(left);
           break;
         }
         case Token::SHL: {
           __ shl(left->reg());
           // Check that the *signed* result fits in a smi.
-          //
-          // TODO(207): Can reduce registers from 4 to 3 by
-          // preallocating ecx.
           JumpTarget result_ok(generator());
-          Result smi_test_reg = generator()->allocator()->Allocate();
-          ASSERT(smi_test_reg.is_valid());
-          __ lea(smi_test_reg.reg(), Operand(left->reg(), 0x40000000));
-          __ test(smi_test_reg.reg(), Immediate(0x80000000));
-          smi_test_reg.Unuse();
-          result_ok.Branch(zero, left, taken);
+          __ cmp(left->reg(), 0xc0000000);
+          result_ok.Branch(positive, left, taken);
+
           __ shr(left->reg());
           ASSERT(kSmiTag == 0);
           __ shl(left->reg(), kSmiTagSize);
           __ shl(right->reg(), kSmiTagSize);
           enter()->Jump(left, right);
           result_ok.Bind(left);
           break;
         }
         default:
           UNREACHABLE();
@@ skipping 279 matching lines @@
       __ pop(eax);
       switch (op_) {
         case Token::BIT_OR:  __ or_(eax, Operand(ecx)); break;
         case Token::BIT_AND: __ and_(eax, Operand(ecx)); break;
         case Token::BIT_XOR: __ xor_(eax, Operand(ecx)); break;
         case Token::SAR: __ sar(eax); break;
         case Token::SHL: __ shl(eax); break;
         case Token::SHR: __ shr(eax); break;
         default: UNREACHABLE();
       }
-
-      // Check if result is non-negative and fits in a smi.
-      __ test(eax, Immediate(0xc0000000));
-      __ j(not_zero, &non_smi_result);
-
+      if (op_ == Token::SHR) {
+        // Check if result is non-negative and fits in a smi.
+        __ test(eax, Immediate(0xc0000000));
+        __ j(not_zero, &non_smi_result);
+      } else {
+        // Check if result fits in a smi.
+        __ cmp(eax, 0xc0000000);
+        __ j(negative, &non_smi_result);
+      }
       // Tag smi result and return.
       ASSERT(kSmiTagSize == times_2);  // adjust code if not the case
       __ lea(eax, Operand(eax, eax, times_1, kSmiTag));
       __ ret(2 * kPointerSize);
 
       // All ops except SHR return a signed int32 that we load in a HeapNumber.
       if (op_ != Token::SHR) {
         __ bind(&non_smi_result);
         // Allocate a heap number if needed.
         __ mov(ebx, Operand(eax));  // ebx: result
@@ skipping 922 matching lines @@
 
   // Slow-case: Go through the JavaScript implementation.
   __ bind(&slow);
   __ InvokeBuiltin(Builtins::INSTANCE_OF, JUMP_FUNCTION);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal