Re-enable using push instructions for syncing the virtual frame.

This change fixes the problem with the original version of this approach
(r3032) that may lead to a corrupted stack if we would invoke spilling during 
syncing a large SMI constant (unsafe SMIs) in the virtual frame.

The new code for storing unsafe SMI constants does not use an extra temporary 
register. This prevents the compiler from ever having to spill during a 
virutal frame sync operation.

For storing a large SMI constant we previously generated:

  mov ecx, (large_smi & 0x0000ffff)
  xor ecx, (large_smi & 0xffff0000)
  push ecx

we now generate:

  push (large_smi & 0x0000ffff)
  or   [esp], (large_smi & 0xffff0000)

Not using a temporary register avoids spilling within an nvocation 
of VirtualFrame::SyncRange.

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

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

src/ia32/codegen-ia32.cc

@@ -3939,12 +3939,28 @@ void CodeGenerator::VisitLiteral(Literal* node) {
 }
 
 
-void CodeGenerator::LoadUnsafeSmi(Register target, Handle<Object> value) {
+void CodeGenerator::PushUnsafeSmi(Handle<Object> value) {
+  ASSERT(value->IsSmi());
+  int bits = reinterpret_cast<int>(*value);
+  __ push(Immediate(bits & 0x0000FFFF));
+  __ or_(Operand(esp, 0), Immediate(bits & 0xFFFF0000));
+}
+
+
+void CodeGenerator::StoreUnsafeSmiToLocal(int offset, Handle<Object> value) {
+  ASSERT(value->IsSmi());
+  int bits = reinterpret_cast<int>(*value);
+  __ mov(Operand(ebp, offset), Immediate(bits & 0x0000FFFF));
+  __ or_(Operand(ebp, offset), Immediate(bits & 0xFFFF0000));
+}
+
+
+void CodeGenerator::MoveUnsafeSmi(Register target, Handle<Object> value) {
   ASSERT(target.is_valid());
   ASSERT(value->IsSmi());
   int bits = reinterpret_cast<int>(*value);
   __ Set(target, Immediate(bits & 0x0000FFFF));
-  __ xor_(target, bits & 0xFFFF0000);
+  __ or_(target, bits & 0xFFFF0000);
 }
 
 

src/ia32/codegen-ia32.h

@@ -468,9 +468,11 @@ class CodeGenerator: public AstVisitor {
   // than 16 bits.
   static const int kMaxSmiInlinedBits = 16;
   bool IsUnsafeSmi(Handle<Object> value);
-  // Load an integer constant x into a register target using
+  // Load an integer constant x into a register target or into the stack using
   // at most 16 bits of user-controlled data per assembly operation.
-  void LoadUnsafeSmi(Register target, Handle<Object> value);
+  void MoveUnsafeSmi(Register target, Handle<Object> value);
+  void StoreUnsafeSmiToLocal(int offset, Handle<Object> value);
+  void PushUnsafeSmi(Handle<Object> value);
 
   void CallWithArguments(ZoneList<Expression*>* arguments, int position);
 

src/ia32/register-allocator-ia32.cc

@@ -42,7 +42,7 @@ void Result::ToRegister() {
     Result fresh = CodeGeneratorScope::Current()->allocator()->Allocate();
     ASSERT(fresh.is_valid());
     if (CodeGeneratorScope::Current()->IsUnsafeSmi(handle())) {
-      CodeGeneratorScope::Current()->LoadUnsafeSmi(fresh.reg(), handle());
+      CodeGeneratorScope::Current()->MoveUnsafeSmi(fresh.reg(), handle());
     } else {
       CodeGeneratorScope::Current()->masm()->Set(fresh.reg(),
                                                  Immediate(handle()));
@@ -64,7 +64,7 @@ void Result::ToRegister(Register target) {
     } else {
       ASSERT(is_constant());
       if (CodeGeneratorScope::Current()->IsUnsafeSmi(handle())) {
-        CodeGeneratorScope::Current()->LoadUnsafeSmi(fresh.reg(), handle());
+        CodeGeneratorScope::Current()->MoveUnsafeSmi(fresh.reg(), handle());
       } else {
         CodeGeneratorScope::Current()->masm()->Set(fresh.reg(),
                                                    Immediate(handle()));

src/ia32/virtual-frame-ia32.cc

@@ -75,10 +75,7 @@ void VirtualFrame::SyncElementBelowStackPointer(int index) {
 
     case FrameElement::CONSTANT:
       if (cgen()->IsUnsafeSmi(element.handle())) {
-        Result temp = cgen()->allocator()->Allocate();
-        ASSERT(temp.is_valid());
-        cgen()->LoadUnsafeSmi(temp.reg(), element.handle());
-        __ mov(Operand(ebp, fp_relative(index)), temp.reg());
+        cgen()->StoreUnsafeSmiToLocal(fp_relative(index), element.handle());
       } else {
         __ Set(Operand(ebp, fp_relative(index)),
                Immediate(element.handle()));
@@ -127,10 +124,7 @@ void VirtualFrame::SyncElementByPushing(int index) {
 
     case FrameElement::CONSTANT:
       if (cgen()->IsUnsafeSmi(element.handle())) {
-        Result temp = cgen()->allocator()->Allocate();
-        ASSERT(temp.is_valid());
-        cgen()->LoadUnsafeSmi(temp.reg(), element.handle());
-        __ push(temp.reg());
+       cgen()->PushUnsafeSmi(element.handle());
       } else {
         __ push(Immediate(element.handle()));
       }
@@ -161,15 +155,16 @@ void VirtualFrame::SyncRange(int begin, int end) {
   // on the stack.
   int start = Min(begin, stack_pointer_ + 1);
 
-  // If positive we have to adjust the stack pointer.
-  int delta = end - stack_pointer_;
-  if (delta > 0) {
-    stack_pointer_ = end;
-    __ sub(Operand(esp), Immediate(delta * kPointerSize));
-  }
-
+  // Emit normal push instructions for elements above stack pointer
+  // and use mov instructions if we are below stack pointer.
   for (int i = start; i <= end; i++) {
-    if (!elements_[i].is_synced()) SyncElementBelowStackPointer(i);
+    if (!elements_[i].is_synced()) {
+      if (i <= stack_pointer_) {
+        SyncElementBelowStackPointer(i);
+      } else {
+        SyncElementByPushing(i);
+      }
+    }
   }
 }
 
@@ -198,7 +193,7 @@ void VirtualFrame::MakeMergable() {
         // Emit a move.
         if (element.is_constant()) {
           if (cgen()->IsUnsafeSmi(element.handle())) {
-            cgen()->LoadUnsafeSmi(fresh.reg(), element.handle());
+            cgen()->MoveUnsafeSmi(fresh.reg(), element.handle());
           } else {
             __ Set(fresh.reg(), Immediate(element.handle()));
           }
@@ -299,7 +294,7 @@ void VirtualFrame::MergeMoveRegistersToMemory(VirtualFrame* expected) {
           if (!source.is_synced()) {
             if (cgen()->IsUnsafeSmi(source.handle())) {
               esi_caches = i;
-              cgen()->LoadUnsafeSmi(esi, source.handle());
+              cgen()->MoveUnsafeSmi(esi, source.handle());
               __ mov(Operand(ebp, fp_relative(i)), esi);
             } else {
               __ Set(Operand(ebp, fp_relative(i)), Immediate(source.handle()));
@@ -407,7 +402,7 @@ void VirtualFrame::MergeMoveMemoryToRegisters(VirtualFrame* expected) {
 
         case FrameElement::CONSTANT:
           if (cgen()->IsUnsafeSmi(source.handle())) {
-            cgen()->LoadUnsafeSmi(target_reg, source.handle());
+            cgen()->MoveUnsafeSmi(target_reg, source.handle());
           } else {
            __ Set(target_reg, Immediate(source.handle()));
           }