Change keyed store IC interface on x64 to take value, key, and receiver in...

Change keyed store IC interface on x64 to take value, key, and receiver in registers rather than on the stack.
Review URL: http://codereview.chromium.org/2111011

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

src/ia32/virtual-frame-ia32.h

@@ -352,7 +352,7 @@ class VirtualFrame: public ZoneObject {
   Result CallStoreIC(Handle<String> name, bool is_contextual);
 
   // Call keyed store IC.  Value, key, and receiver are found on top
-  // of the frame.  Key and receiver are not dropped.
+  // of the frame.  All three are dropped.
   Result CallKeyedStoreIC();
 
   // Call call IC.  Function name, arguments, and receiver are found on top

src/x64/codegen-x64.cc

@@ -680,11 +680,51 @@ class DeferredReferenceSetKeyedValue: public DeferredCode {
 
 void DeferredReferenceSetKeyedValue::Generate() {
   __ IncrementCounter(&Counters::keyed_store_inline_miss, 1);
-  // Push receiver and key arguments on the stack.
-  __ push(receiver_);
-  __ push(key_);
-  // Move value argument to eax as expected by the IC stub.
-  if (!value_.is(rax)) __ movq(rax, value_);
+  // Move value, receiver, and key to registers rax, rdx, and rcx, as
+  // the IC stub expects.
+  // Move value to rax, using xchg if the receiver or key is in rax.
+  if (!value_.is(rax)) {
+    if (!receiver_.is(rax) && !key_.is(rax)) {
+      __ movq(rax, value_);
+    } else {
+      __ xchg(rax, value_);
+      // Update receiver_ and key_ if they are affected by the swap.
+      if (receiver_.is(rax)) {
+        receiver_ = value_;
+      } else if (receiver_.is(value_)) {
+        receiver_ = rax;
+      }
+      if (key_.is(rax)) {
+        key_ = value_;
+      } else if (key_.is(value_)) {
+        key_ = rax;
+      }
+    }
+  }
+  // Value is now in rax. Its original location is remembered in value_,
+  // and the value is restored to value_ before returning.
+  // The variables receiver_ and key_ are not preserved.
+  // Move receiver and key to rdx and rcx, swapping if necessary.
+  if (receiver_.is(rdx)) {
+    if (!key_.is(rcx)) {
+      __ movq(rcx, key_);
+    }  // Else everything is already in the right place.
+  } else if (receiver_.is(rcx)) {
+    if (key_.is(rdx)) {
+      __ xchg(rcx, rdx);
+    } else if (key_.is(rcx)) {
+      __ movq(rdx, receiver_);
+    } else {
+      __ movq(rdx, receiver_);
+      __ movq(rcx, key_);
+    }
+  } else if (key_.is(rcx)) {
+    __ movq(rdx, receiver_);
+  } else {
+    __ movq(rcx, key_);
+    __ movq(rdx, receiver_);
+  }
+
   // Call the IC stub.
   Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
   __ Call(ic, RelocInfo::CODE_TARGET);
@@ -697,11 +737,8 @@ void DeferredReferenceSetKeyedValue::Generate() {
   // Here we use masm_-> instead of the __ macro because this is the
   // instruction that gets patched and coverage code gets in the way.
   masm_->testl(rax, Immediate(-delta_to_patch_site));
-  // Restore value (returned from store IC), key and receiver
-  // registers.
+  // Restore value (returned from store IC).
   if (!value_.is(rax)) __ movq(value_, rax);
-  __ pop(key_);
-  __ pop(receiver_);
 }
 
 
@@ -7533,8 +7570,6 @@ void Reference::SetValue(InitState init_state) {
 
         deferred->BindExit();
 
-        cgen_->frame()->Push(&receiver);
-        cgen_->frame()->Push(&key);
         cgen_->frame()->Push(&value);
       } else {
         Result answer = cgen_->frame()->CallKeyedStoreIC();
@@ -7545,7 +7580,7 @@ void Reference::SetValue(InitState init_state) {
         masm->nop();
         cgen_->frame()->Push(&answer);
       }
-      cgen_->UnloadReference(this);
+      set_unloaded();
       break;
     }
 

src/x64/debug-x64.cc

@@ -134,10 +134,10 @@ void Debug::GenerateKeyedStoreICDebugBreak(MacroAssembler* masm) {
   // Register state for keyed IC load call (from ic-x64.cc).
   // ----------- S t a t e -------------
   //  -- rax    : value
+  //  -- rcx    : key
+  //  -- rdx    : receiver
   // -----------------------------------
-  // Register rax contains an object that needs to be pushed on the
-  // expression stack of the fake JS frame.
-  Generate_DebugBreakCallHelper(masm, rax.bit(), false);
+  Generate_DebugBreakCallHelper(masm, rax.bit() | rcx.bit() | rdx.bit(), false);
 }
 
 

src/x64/full-codegen-x64.cc

@@ -851,23 +851,22 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable,
       // We are declaring a function or constant that rewrites to a
       // property.  Use (keyed) IC to set the initial value.
       VisitForValue(prop->obj(), kStack);
-      VisitForValue(prop->key(), kStack);
-
       if (function != NULL) {
+        VisitForValue(prop->key(), kStack);
         VisitForValue(function, kAccumulator);
+        __ pop(rcx);
       } else {
+        VisitForValue(prop->key(), kAccumulator);
+        __ movq(rcx, result_register());
         __ LoadRoot(result_register(), Heap::kTheHoleValueRootIndex);
       }
+      __ pop(rdx);
 
       Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
       __ call(ic, RelocInfo::CODE_TARGET);
       // Absence of a test rax instruction following the call
       // indicates that none of the load was inlined.
       __ nop();
-
-      // Value in rax is ignored (declarations are statements).  Receiver
-      // and key on stack are discarded.
-      __ Drop(2);
     }
   }
 }
@@ -1665,6 +1664,12 @@ void FullCodeGenerator::EmitKeyedPropertyAssignment(Assignment* expr) {
     __ pop(result_register());
   }
 
+  __ pop(rcx);
+  if (expr->ends_initialization_block()) {
+    __ movq(rdx, Operand(rsp, 0));  // Leave receiver on the stack for later.
+  } else {
+    __ pop(rdx);
+  }
   // Record source code position before IC call.
   SetSourcePosition(expr->position());
   Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
@@ -1675,15 +1680,14 @@ void FullCodeGenerator::EmitKeyedPropertyAssignment(Assignment* expr) {
 
   // If the assignment ends an initialization block, revert to fast case.
   if (expr->ends_initialization_block()) {
+    __ pop(rdx);
     __ push(rax);  // Result of assignment, saved even if not needed.
-    // Receiver is under the key and value.
-    __ push(Operand(rsp, 2 * kPointerSize));
+    __ push(rdx);
     __ CallRuntime(Runtime::kToFastProperties, 1);
     __ pop(rax);
   }
 
-  // Receiver and key are still on stack.
-  DropAndApply(2, context_, rax);
+  Apply(context_, rax);
 }
 
 
@@ -2969,18 +2973,19 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
       break;
     }
     case KEYED_PROPERTY: {
+      __ pop(rcx);
+      __ pop(rdx);
       Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
       __ call(ic, RelocInfo::CODE_TARGET);
       // This nop signals to the IC that there is no inlined code at the call
       // site for it to patch.
       __ nop();
       if (expr->is_postfix()) {
-        __ Drop(2);  // Result is on the stack under the key and the receiver.
         if (context_ != Expression::kEffect) {
           ApplyTOS(context_);
         }
       } else {
-        DropAndApply(2, context_, rax);
+        Apply(context_, rax);
       }
       break;
     }

src/x64/ic-x64.cc

@@ -780,16 +780,16 @@ void KeyedLoadIC::GenerateIndexedInterceptor(MacroAssembler* masm) {
 void KeyedStoreIC::GenerateMiss(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- rax     : value
+  //  -- rcx     : key
+  //  -- rdx     : receiver
   //  -- rsp[0]  : return address
-  //  -- rsp[8]  : key
-  //  -- rsp[16] : receiver
   // -----------------------------------
 
-  __ pop(rcx);
-  __ push(Operand(rsp, 1 * kPointerSize));  // receiver
-  __ push(Operand(rsp, 1 * kPointerSize));  // key
+  __ pop(rbx);
+  __ push(rdx);  // receiver
+  __ push(rcx);  // key
   __ push(rax);  // value
-  __ push(rcx);  // return address
+  __ push(rbx);  // return address
 
   // Do tail-call to runtime routine.
   ExternalReference ref = ExternalReference(IC_Utility(kKeyedStoreIC_Miss));
@@ -800,16 +800,16 @@ void KeyedStoreIC::GenerateMiss(MacroAssembler* masm) {
 void KeyedStoreIC::GenerateRuntimeSetProperty(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- rax     : value
+  //  -- rcx     : key
+  //  -- rdx     : receiver
   //  -- rsp[0]  : return address
-  //  -- rsp[8]  : key
-  //  -- rsp[16] : receiver
   // -----------------------------------
 
-  __ pop(rcx);
-  __ push(Operand(rsp, 1 * kPointerSize));  // receiver
-  __ push(Operand(rsp, 1 * kPointerSize));  // key
+  __ pop(rbx);
+  __ push(rdx);  // receiver
+  __ push(rcx);  // key
   __ push(rax);  // value
-  __ push(rcx);  // return address
+  __ push(rbx);  // return address
 
   // Do tail-call to runtime routine.
   __ TailCallRuntime(Runtime::kSetProperty, 3, 1);
@@ -818,50 +818,46 @@ void KeyedStoreIC::GenerateRuntimeSetProperty(MacroAssembler* masm) {
 
 void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
   // ----------- S t a t e -------------
-  //  -- rax    : value
-  //  -- rsp[0] : return address
-  //  -- rsp[8] : key
-  //  -- rsp[16] : receiver
+  //  -- rax     : value
+  //  -- rcx     : key
+  //  -- rdx     : receiver
+  //  -- rsp[0]  : return address
   // -----------------------------------
   Label slow, fast, array, extra, check_pixel_array;
 
-  // Get the receiver from the stack.
-  __ movq(rdx, Operand(rsp, 2 * kPointerSize));  // 2 ~ return address, key
   // Check that the object isn't a smi.
   __ JumpIfSmi(rdx, &slow);
   // Get the map from the receiver.
-  __ movq(rcx, FieldOperand(rdx, HeapObject::kMapOffset));
+  __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset));
   // Check that the receiver does not require access checks.  We need
   // to do this because this generic stub does not perform map checks.
-  __ testb(FieldOperand(rcx, Map::kBitFieldOffset),
+  __ testb(FieldOperand(rbx, Map::kBitFieldOffset),
            Immediate(1 << Map::kIsAccessCheckNeeded));
   __ j(not_zero, &slow);
-  // Get the key from the stack.
-  __ movq(rbx, Operand(rsp, 1 * kPointerSize));  // 1 ~ return address
   // Check that the key is a smi.
-  __ JumpIfNotSmi(rbx, &slow);
+  __ JumpIfNotSmi(rcx, &slow);
 
-  __ CmpInstanceType(rcx, JS_ARRAY_TYPE);
+  __ CmpInstanceType(rbx, JS_ARRAY_TYPE);
   __ j(equal, &array);
   // Check that the object is some kind of JS object.
-  __ CmpInstanceType(rcx, FIRST_JS_OBJECT_TYPE);
+  __ CmpInstanceType(rbx, FIRST_JS_OBJECT_TYPE);
   __ j(below, &slow);
 
   // Object case: Check key against length in the elements array.
   // rax: value
   // rdx: JSObject
-  // rbx: index (as a smi)
-  __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset));
+  // rcx: index (as a smi)
+  __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset));
   // Check that the object is in fast mode (not dictionary).
-  __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset),
+  __ CompareRoot(FieldOperand(rbx, HeapObject::kMapOffset),
                  Heap::kFixedArrayMapRootIndex);
   __ j(not_equal, &check_pixel_array);
   // Untag the key (for checking against untagged length in the fixed array).
-  __ SmiToInteger32(rdx, rbx);
-  __ cmpl(rdx, FieldOperand(rcx, Array::kLengthOffset));
+  __ SmiToInteger32(rdi, rcx);
+  __ cmpl(rdi, FieldOperand(rbx, Array::kLengthOffset));
   // rax: value
-  // rcx: FixedArray
-  // rbx: index (as a smi)
+  // rbx: FixedArray
+  // rcx: index (as a smi)
   __ j(below, &fast);
 
   // Slow case: call runtime.
@@ -870,31 +866,31 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
 
   // Check whether the elements is a pixel array.
   // rax: value
-  // rcx: elements array
-  // rbx: index (as a smi), zero-extended.
+  // rdx: receiver
+  // rbx: receiver's elements array
+  // rcx: index (as a smi), zero-extended.
   __ bind(&check_pixel_array);
-  __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset),
+  __ CompareRoot(FieldOperand(rbx, HeapObject::kMapOffset),
                  Heap::kPixelArrayMapRootIndex);
   __ j(not_equal, &slow);
   // Check that the value is a smi. If a conversion is needed call into the
   // runtime to convert and clamp.
   __ JumpIfNotSmi(rax, &slow);
-  __ SmiToInteger32(rbx, rbx);
-  __ cmpl(rbx, FieldOperand(rcx, PixelArray::kLengthOffset));
+  __ SmiToInteger32(rdi, rcx);
+  __ cmpl(rdi, FieldOperand(rbx, PixelArray::kLengthOffset));
   __ j(above_equal, &slow);
-  __ movq(rdx, rax);  // Save the value.
-  __ SmiToInteger32(rax, rax);
+  // No more bailouts to slow case on this path, so key not needed.
+  __ SmiToInteger32(rcx, rax);
   {  // Clamp the value to [0..255].
     Label done;
-    __ testl(rax, Immediate(0xFFFFFF00));
+    __ testl(rcx, Immediate(0xFFFFFF00));
     __ j(zero, &done);
-    __ setcc(negative, rax);  // 1 if negative, 0 if positive.
-    __ decb(rax);  // 0 if negative, 255 if positive.
+    __ setcc(negative, rcx);  // 1 if negative, 0 if positive.
+    __ decb(rcx);  // 0 if negative, 255 if positive.
     __ bind(&done);
   }
-  __ movq(rcx, FieldOperand(rcx, PixelArray::kExternalPointerOffset));
-  __ movb(Operand(rcx, rbx, times_1, 0), rax);
-  __ movq(rax, rdx);  // Return the original value.
+  __ movq(rbx, FieldOperand(rbx, PixelArray::kExternalPointerOffset));
+  __ movb(Operand(rbx, rdi, times_1, 0), rcx);
   __ ret(0);
 
   // Extra capacity case: Check if there is extra capacity to
@@ -902,18 +898,17 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
   // element to the array by writing to array[array.length].
   __ bind(&extra);
   // rax: value
-  // rdx: JSArray
-  // rcx: FixedArray
-  // rbx: index (as a smi)
+  // rdx: receiver (a JSArray)
+  // rbx: receiver's elements array (a FixedArray)
+  // rcx: index (as a smi)
   // flags: smicompare (rdx.length(), rbx)
   __ j(not_equal, &slow);  // do not leave holes in the array
-  __ SmiToInteger64(rbx, rbx);
-  __ cmpl(rbx, FieldOperand(rcx, FixedArray::kLengthOffset));
+  __ SmiToInteger64(rdi, rcx);
+  __ cmpl(rdi, FieldOperand(rbx, FixedArray::kLengthOffset));
   __ j(above_equal, &slow);
   // Increment and restore smi-tag.
-  __ Integer64PlusConstantToSmi(rbx, rbx, 1);
-  __ movq(FieldOperand(rdx, JSArray::kLengthOffset), rbx);
-  __ SmiSubConstant(rbx, rbx, Smi::FromInt(1));
+  __ Integer64PlusConstantToSmi(rdi, rdi, 1);
+  __ movq(FieldOperand(rdx, JSArray::kLengthOffset), rdi);
   __ jmp(&fast);
 
   // Array case: Get the length and the elements array from the JS
@@ -921,39 +916,39 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
   // length is always a smi.
   __ bind(&array);
   // rax: value
-  // rdx: JSArray
-  // rbx: index (as a smi)
-  __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset));
-  __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset),
+  // rdx: receiver (a JSArray)
+  // rcx: index (as a smi)
+  __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset));
+  __ CompareRoot(FieldOperand(rbx, HeapObject::kMapOffset),
                  Heap::kFixedArrayMapRootIndex);
   __ j(not_equal, &slow);
 
   // Check the key against the length in the array, compute the
   // address to store into and fall through to fast case.
-  __ SmiCompare(FieldOperand(rdx, JSArray::kLengthOffset), rbx);
+  __ SmiCompare(FieldOperand(rdx, JSArray::kLengthOffset), rcx);
   __ j(below_equal, &extra);
 
   // Fast case: Do the store.
   __ bind(&fast);
   // rax: value
-  // rcx: FixedArray
-  // rbx: index (as a smi)
+  // rbx: receiver's elements array (a FixedArray)
+  // rcx: index (as a smi)
   Label non_smi_value;
   __ JumpIfNotSmi(rax, &non_smi_value);
-  SmiIndex index = masm->SmiToIndex(rbx, rbx, kPointerSizeLog2);
-  __ movq(Operand(rcx, index.reg, index.scale,
+  SmiIndex index = masm->SmiToIndex(rcx, rcx, kPointerSizeLog2);
+  __ movq(Operand(rbx, index.reg, index.scale,
                   FixedArray::kHeaderSize - kHeapObjectTag),
           rax);
   __ ret(0);
   __ bind(&non_smi_value);
-  // Slow case that needs to retain rbx for use by RecordWrite.
+  // Slow case that needs to retain rcx for use by RecordWrite.
   // Update write barrier for the elements array address.
-  SmiIndex index2 = masm->SmiToIndex(kScratchRegister, rbx, kPointerSizeLog2);
-  __ movq(Operand(rcx, index2.reg, index2.scale,
+  SmiIndex index2 = masm->SmiToIndex(kScratchRegister, rcx, kPointerSizeLog2);
+  __ movq(Operand(rbx, index2.reg, index2.scale,
                   FixedArray::kHeaderSize - kHeapObjectTag),
           rax);
   __ movq(rdx, rax);
-  __ RecordWriteNonSmi(rcx, 0, rdx, rbx);
+  __ RecordWriteNonSmi(rbx, 0, rdx, rcx);
   __ ret(0);
 }
 
@@ -961,102 +956,103 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
 void KeyedStoreIC::GenerateExternalArray(MacroAssembler* masm,
                                          ExternalArrayType array_type) {
   // ----------- S t a t e -------------
-  //  -- rax    : value
-  //  -- rsp[0] : return address
-  //  -- rsp[8] : key
-  //  -- rsp[16] : receiver
+  //  -- rax     : value
+  //  -- rcx     : key
+  //  -- rdx     : receiver
+  //  -- rsp[0]  : return address
   // -----------------------------------
   Label slow, check_heap_number;
 
-  // Get the receiver from the stack.
-  __ movq(rdx, Operand(rsp, 2 * kPointerSize));
   // Check that the object isn't a smi.
   __ JumpIfSmi(rdx, &slow);
   // Get the map from the receiver.
-  __ movq(rcx, FieldOperand(rdx, HeapObject::kMapOffset));
+  __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset));
   // Check that the receiver does not require access checks.  We need
   // to do this because this generic stub does not perform map checks.
-  __ testb(FieldOperand(rcx, Map::kBitFieldOffset),
+  __ testb(FieldOperand(rbx, Map::kBitFieldOffset),
            Immediate(1 << Map::kIsAccessCheckNeeded));
   __ j(not_zero, &slow);
-  // Get the key from the stack.
-  __ movq(rbx, Operand(rsp, 1 * kPointerSize));  // 1 ~ return address
   // Check that the key is a smi.
-  __ JumpIfNotSmi(rbx, &slow);
+  __ JumpIfNotSmi(rcx, &slow);
 
   // Check that the object is a JS object.
-  __ CmpInstanceType(rcx, JS_OBJECT_TYPE);
+  __ CmpInstanceType(rbx, JS_OBJECT_TYPE);
   __ j(not_equal, &slow);
 
   // Check that the elements array is the appropriate type of
   // ExternalArray.
   // rax: value
-  // rdx: JSObject
-  // rbx: index (as a smi)
-  __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset));
-  __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset),
+  // rcx: key (a smi)
+  // rdx: receiver (a JSObject)
+  __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset));
+  __ CompareRoot(FieldOperand(rbx, HeapObject::kMapOffset),
                  Heap::RootIndexForExternalArrayType(array_type));
   __ j(not_equal, &slow);
 
   // Check that the index is in range.
-  __ SmiToInteger32(rbx, rbx);  // Untag the index.
-  __ cmpl(rbx, FieldOperand(rcx, ExternalArray::kLengthOffset));
+  __ SmiToInteger32(rdi, rcx);  // Untag the index.
+  __ cmpl(rdi, FieldOperand(rbx, ExternalArray::kLengthOffset));
   // Unsigned comparison catches both negative and too-large values.
   __ j(above_equal, &slow);
 
   // Handle both smis and HeapNumbers in the fast path. Go to the
   // runtime for all other kinds of values.
   // rax: value
-  // rcx: elements array
-  // rbx: untagged index
+  // rcx: key (a smi)
+  // rdx: receiver (a JSObject)
+  // rbx: elements array
+  // rdi: untagged key
   __ JumpIfNotSmi(rax, &check_heap_number);
-  __ movq(rdx, rax);  // Save the value.
-  __ SmiToInteger32(rax, rax);
-  __ movq(rcx, FieldOperand(rcx, ExternalArray::kExternalPointerOffset));
-  // rcx: base pointer of external storage
+  // No more branches to slow case on this path.  Key and receiver not needed.
+  __ SmiToInteger32(rdx, rax);
+  __ movq(rbx, FieldOperand(rbx, ExternalArray::kExternalPointerOffset));
+  // rbx: base pointer of external storage
   switch (array_type) {
     case kExternalByteArray:
     case kExternalUnsignedByteArray:
-      __ movb(Operand(rcx, rbx, times_1, 0), rax);
+      __ movb(Operand(rbx, rdi, times_1, 0), rdx);
       break;
     case kExternalShortArray:
     case kExternalUnsignedShortArray:
-      __ movw(Operand(rcx, rbx, times_2, 0), rax);
+      __ movw(Operand(rbx, rdi, times_2, 0), rdx);
       break;
     case kExternalIntArray:
     case kExternalUnsignedIntArray:
-      __ movl(Operand(rcx, rbx, times_4, 0), rax);
+      __ movl(Operand(rbx, rdi, times_4, 0), rdx);
       break;
     case kExternalFloatArray:
       // Need to perform int-to-float conversion.
-      __ push(rax);
+      __ push(rdx);
       __ fild_s(Operand(rsp, 0));
-      __ pop(rax);
-      __ fstp_s(Operand(rcx, rbx, times_4, 0));
+      __ pop(rdx);
+      __ fstp_s(Operand(rbx, rdi, times_4, 0));
       break;
     default:
       UNREACHABLE();
       break;
   }
-  __ movq(rax, rdx);  // Return the original value.
   __ ret(0);
 
   __ bind(&check_heap_number);
-  __ CmpObjectType(rax, HEAP_NUMBER_TYPE, rdx);
+  // rax: value
+  // rcx: key (a smi)
+  // rdx: receiver (a JSObject)
+  // rbx: elements array
+  // rdi: untagged key
+  __ CmpObjectType(rax, HEAP_NUMBER_TYPE, kScratchRegister);
   __ j(not_equal, &slow);
+  // No more branches to slow case on this path.
 
   // The WebGL specification leaves the behavior of storing NaN and
   // +/-Infinity into integer arrays basically undefined. For more
   // reproducible behavior, convert these to zero.
   __ fld_d(FieldOperand(rax, HeapNumber::kValueOffset));
-  __ movq(rdx, rax);  // Save the value.
-  __ movq(rcx, FieldOperand(rcx, ExternalArray::kExternalPointerOffset));
-  // rbx: untagged index
-  // rcx: base pointer of external storage
+  __ movq(rbx, FieldOperand(rbx, ExternalArray::kExternalPointerOffset));
+  // rdi: untagged index
+  // rbx: base pointer of external storage
   // top of FPU stack: value
   if (array_type == kExternalFloatArray) {
-    __ fstp_s(Operand(rcx, rbx, times_4, 0));
-    __ movq(rax, rdx);  // Return the original value.
+    __ fstp_s(Operand(rbx, rdi, times_4, 0));
     __ ret(0);
   } else {
     // Need to perform float-to-int conversion.
@@ -1065,66 +1061,70 @@ void KeyedStoreIC::GenerateExternalArray(MacroAssembler* masm,
     __ fucomi(0);
     __ j(parity_even, &is_nan);
 
-    __ push(rax);  // Make room on stack
+    __ push(rdx);  // Make room on the stack.  Receiver is no longer needed.
     __ fistp_d(Operand(rsp, 0));
-    __ pop(rax);
-    // rax: untagged integer value
+    __ pop(rdx);
+    // rdx: value (converted to an untagged integer)
+    // rdi: untagged index
+    // rbx: base pointer of external storage
     switch (array_type) {
       case kExternalByteArray:
       case kExternalUnsignedByteArray:
-        __ movb(Operand(rcx, rbx, times_1, 0), rax);
+        __ movb(Operand(rbx, rdi, times_1, 0), rdx);
         break;
       case kExternalShortArray:
       case kExternalUnsignedShortArray:
-        __ movw(Operand(rcx, rbx, times_2, 0), rax);
+        __ movw(Operand(rbx, rdi, times_2, 0), rdx);
         break;
       case kExternalIntArray:
       case kExternalUnsignedIntArray: {
         // We also need to explicitly check for +/-Infinity. These are
         // converted to MIN_INT, but we need to be careful not to
-        // confuse with legal uses of MIN_INT.
+        // confuse with legal uses of MIN_INT.  Since MIN_INT truncated
+        // to 8 or 16 bits is zero, we only perform this test when storing
+        // 32-bit ints.
         Label not_infinity;
         // This test would apparently detect both NaN and Infinity,
         // but we've already checked for NaN using the FPU hardware
         // above.
-        __ movzxwq(rdi, FieldOperand(rdx, HeapNumber::kValueOffset + 6));
-        __ and_(rdi, Immediate(0x7FF0));
-        __ cmpw(rdi, Immediate(0x7FF0));
+        __ movzxwq(rcx, FieldOperand(rax, HeapNumber::kValueOffset + 6));
+        __ and_(rcx, Immediate(0x7FF0));
+        __ cmpw(rcx, Immediate(0x7FF0));
         __ j(not_equal, &not_infinity);
-        __ movq(rax, Immediate(0));
+        __ movq(rdx, Immediate(0));
         __ bind(&not_infinity);
-        __ movl(Operand(rcx, rbx, times_4, 0), rax);
+        __ movl(Operand(rbx, rdi, times_4, 0), rdx);
         break;
       }
       default:
         UNREACHABLE();
         break;
     }
-    __ movq(rax, rdx);  // Return the original value.
     __ ret(0);
 
     __ bind(&is_nan);
+    // rdi: untagged index
+    // rbx: base pointer of external storage
     __ ffree();
     __ fincstp();
-    __ movq(rax, Immediate(0));
+    __ movq(rdx, Immediate(0));
     switch (array_type) {
       case kExternalByteArray:
       case kExternalUnsignedByteArray:
-        __ movb(Operand(rcx, rbx, times_1, 0), rax);
+        __ movb(Operand(rbx, rdi, times_1, 0), rdx);
         break;
       case kExternalShortArray:
       case kExternalUnsignedShortArray:
-        __ movw(Operand(rcx, rbx, times_2, 0), rax);
+        __ movw(Operand(rbx, rdi, times_2, 0), rdx);
         break;
       case kExternalIntArray:
       case kExternalUnsignedIntArray:
-        __ movl(Operand(rcx, rbx, times_4, 0), rax);
+        __ movl(Operand(rbx, rdi, times_4, 0), rdx);
         break;
       default:
         UNREACHABLE();
         break;
     }
-    __ movq(rax, rdx);  // Return the original value.
     __ ret(0);
   }
 

src/x64/stub-cache-x64.cc

@@ -2029,23 +2029,18 @@ Object* KeyedStoreStubCompiler::CompileStoreField(JSObject* object,
                                                   String* name) {
   // ----------- S t a t e -------------
   //  -- rax     : value
+  //  -- rcx     : key
+  //  -- rdx     : receiver
   //  -- rsp[0]  : return address
-  //  -- rsp[8]  : key
-  //  -- rsp[16] : receiver
   // -----------------------------------
   Label miss;
 
   __ IncrementCounter(&Counters::keyed_store_field, 1);
 
-  // Get the name from the stack.
-  __ movq(rcx, Operand(rsp, 1 * kPointerSize));
   // Check that the name has not changed.
   __ Cmp(rcx, Handle<String>(name));
   __ j(not_equal, &miss);
 
-  // Get the receiver from the stack.
-  __ movq(rdx, Operand(rsp, 2 * kPointerSize));
-
   // Generate store field code.  Preserves receiver and name on jump to miss.
   GenerateStoreField(masm(),
                      object,

src/x64/virtual-frame-x64.cc

@@ -1031,6 +1031,46 @@ void VirtualFrame::DebugBreak() {
 #endif
 
 
+// This function assumes that the only results that could be in a_reg or b_reg
+// are a and b.  Other results can be live, but must not be in a_reg or b_reg.
+void VirtualFrame::MoveResultsToRegisters(Result* a,
+                                          Result* b,
+                                          Register a_reg,
+                                          Register b_reg) {
+  ASSERT(!a_reg.is(b_reg));
+  // Assert that cgen()->allocator()->count(a_reg) is accounted for by a and b.
+  ASSERT(cgen()->allocator()->count(a_reg) <= 2);
+  ASSERT(cgen()->allocator()->count(a_reg) != 2 || a->reg().is(a_reg));
+  ASSERT(cgen()->allocator()->count(a_reg) != 2 || b->reg().is(a_reg));
+  ASSERT(cgen()->allocator()->count(a_reg) != 1 ||
+         (a->is_register() && a->reg().is(a_reg)) ||
+         (b->is_register() && b->reg().is(a_reg)));
+  // Assert that cgen()->allocator()->count(b_reg) is accounted for by a and b.
+  ASSERT(cgen()->allocator()->count(b_reg) <= 2);
+  ASSERT(cgen()->allocator()->count(b_reg) != 2 || a->reg().is(b_reg));
+  ASSERT(cgen()->allocator()->count(b_reg) != 2 || b->reg().is(b_reg));
+  ASSERT(cgen()->allocator()->count(b_reg) != 1 ||
+         (a->is_register() && a->reg().is(b_reg)) ||
+         (b->is_register() && b->reg().is(b_reg)));
+
+  if (a->is_register() && a->reg().is(a_reg)) {
+    b->ToRegister(b_reg);
+  } else if (!cgen()->allocator()->is_used(a_reg)) {
+    a->ToRegister(a_reg);
+    b->ToRegister(b_reg);
+  } else if (cgen()->allocator()->is_used(b_reg)) {
+    // a must be in b_reg, b in a_reg.
+    __ xchg(a_reg, b_reg);
+    // Results a and b will be invalidated, so it is ok if they are switched.
+  } else {
+    b->ToRegister(b_reg);
+    a->ToRegister(a_reg);
+  }
+  a->Unuse();
+  b->Unuse();
+}
+
+
 Result VirtualFrame::CallLoadIC(RelocInfo::Mode mode) {
   // Name and receiver are on the top of the frame.  The IC expects
   // name in rcx and receiver on the stack.  It does not drop the
@@ -1053,15 +1093,52 @@ Result VirtualFrame::CallKeyedLoadIC(RelocInfo::Mode mode) {
 }
 
 
-Result VirtualFrame::CallKeyedStoreIC() {
-  // Value, key, and receiver are on the top of the frame.  The IC
-  // expects value in rax and key and receiver on the stack.  It does
-  // not drop the key and receiver.
-  Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
-  Result value = Pop();
-  PrepareForCall(2, 0);  // Two stack args, neither callee-dropped.
-  value.ToRegister(rax);
-  value.Unuse();
+Result VirtualFrame::CallCommonStoreIC(Handle<Code> ic,
+                                       Result* value,
+                                       Result* key,
+                                       Result* receiver) {
+  // The IC expects value in rax, key in rcx, and receiver in rdx.
+  PrepareForCall(0, 0);
+  // If one of the three registers is free, or a value is already
+  // in the correct register, move the remaining two values using
+  // MoveResultsToRegisters().
+  if (!cgen()->allocator()->is_used(rax) ||
+      (value->is_register() && value->reg().is(rax))) {
+    if (!cgen()->allocator()->is_used(rax)) {
+      value->ToRegister(rax);
+    }
+    MoveResultsToRegisters(key, receiver, rcx, rdx);
+    value->Unuse();
+  } else if (!cgen()->allocator()->is_used(rcx) ||
+             (key->is_register() && key->reg().is(rcx))) {
+    if (!cgen()->allocator()->is_used(rcx)) {
+      key->ToRegister(rcx);
+    }
+    MoveResultsToRegisters(value, receiver, rax, rdx);
+    key->Unuse();
+  } else if (!cgen()->allocator()->is_used(rdx) ||
+             (receiver->is_register() && receiver->reg().is(rdx))) {
+    if (!cgen()->allocator()->is_used(rdx)) {
+      receiver->ToRegister(rdx);
+    }
+    MoveResultsToRegisters(key, value, rcx, rax);
+    receiver->Unuse();
+  } else {
+    // Otherwise, no register is free, and no value is in the correct place.
+    // We have one of the two circular permutations of eax, ecx, edx.
+    ASSERT(value->is_register());
+    if (value->reg().is(rcx)) {
+      __ xchg(rax, rdx);
+      __ xchg(rax, rcx);
+    } else {
+      __ xchg(rax, rcx);
+      __ xchg(rax, rdx);
+    }
+    value->Unuse();
+    key->Unuse();
+    receiver->Unuse();
+  }
+
   return RawCallCodeObject(ic, RelocInfo::CODE_TARGET);
 }
 
@@ -1108,51 +1185,6 @@ Result VirtualFrame::CallConstructor(int arg_count) {
 }
 
 
-Result VirtualFrame::CallStoreIC() {
-  // Name, value, and receiver are on top of the frame.  The IC
-  // expects name in rcx, value in rax, and receiver in edx.
-  Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
-  Result name = Pop();
-  Result value = Pop();
-  Result receiver = Pop();
-  PrepareForCall(0, 0);
-
-  // Optimized for case in which name is a constant value.
-  if (name.is_register() && (name.reg().is(rdx) || name.reg().is(rax))) {
-    if (!is_used(rcx)) {
-      name.ToRegister(rcx);
-    } else if (!is_used(rbx)) {
-      name.ToRegister(rbx);
-    } else {
-      ASSERT(!is_used(rdi));  // Only three results are live, so rdi is free.
-      name.ToRegister(rdi);
-    }
-  }
-  // Now name is not in edx or eax, so we can fix them, then move name to ecx.
-  if (value.is_register() && value.reg().is(rdx)) {
-    if (receiver.is_register() && receiver.reg().is(rax)) {
-      // Wrong registers.
-      __ xchg(rax, rdx);
-    } else {
-      // Register rax is free for value, which frees rcx for receiver.
-      value.ToRegister(rax);
-      receiver.ToRegister(rdx);
-    }
-  } else {
-    // Register rcx is free for receiver, which guarantees rax is free for
-    // value.
-    receiver.ToRegister(rdx);
-    value.ToRegister(rax);
-  }
-  // Receiver and value are in the right place, so rcx is free for name.
-  name.ToRegister(rcx);
-  name.Unuse();
-  value.Unuse();
-  receiver.Unuse();
-  return RawCallCodeObject(ic, RelocInfo::CODE_TARGET);
-}
-
-
 void VirtualFrame::PushTryHandler(HandlerType type) {
   ASSERT(cgen()->HasValidEntryRegisters());
   // Grow the expression stack by handler size less one (the return

src/x64/virtual-frame-x64.h

@@ -336,13 +336,33 @@ class VirtualFrame : public ZoneObject {
   // frame.  They are not dropped.
   Result CallKeyedLoadIC(RelocInfo::Mode mode);
 
-  // Call store IC.  Name, value, and receiver are found on top of the
-  // frame.  Receiver is not dropped.
-  Result CallStoreIC();
+
+  // Calling a store IC and a keyed store IC differ only by which ic is called
+  // and by the order of the three arguments on the frame.
+  Result CallCommonStoreIC(Handle<Code> ic,
+                           Result* value,
+                           Result *key,
+                           Result* receiver);
+
+  // Call store IC.  Name, value, and receiver are found on top
+  // of the frame.  All are dropped.
+  Result CallStoreIC() {
+    Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+    Result name = Pop();
+    Result value = Pop();
+    Result receiver = Pop();
+    return CallCommonStoreIC(ic, &value, &name, &receiver);
+  }
 
   // Call keyed store IC.  Value, key, and receiver are found on top
-  // of the frame.  Key and receiver are not dropped.
-  Result CallKeyedStoreIC();
+  // of the frame.  All are dropped.
+  Result CallKeyedStoreIC() {
+    Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
+    Result value = Pop();
+    Result key = Pop();
+    Result receiver = Pop();
+    return CallCommonStoreIC(ic, &value, &key, &receiver);
+  }
 
   // Call call IC.  Function name, arguments, and receiver are found on top
   // of the frame and dropped by the call.
@@ -551,6 +571,14 @@ class VirtualFrame : public ZoneObject {
   // Register counts are correctly updated.
   int InvalidateFrameSlotAt(int index);
 
+  // This function assumes that a and b are the only results that could be in
+  // the registers a_reg or b_reg.  Other results can be live, but must not
+  //  be in the registers a_reg or b_reg.  The results a and b are invalidated.
+  void MoveResultsToRegisters(Result* a,
+                              Result* b,
+                              Register a_reg,
+                              Register b_reg);
+
   // Call a code stub that has already been prepared for calling (via
   // PrepareForCall).
   Result RawCallStub(CodeStub* stub);

test/mjsunit/compiler/assignment.js

@@ -262,3 +262,15 @@ function bar_loop() {
 }
 
 bar_loop();
+
+
+// Test for assignment using a keyed store ic:
+function store_i_in_element_i_of_object_i() {
+  var i = new Object();
+  i[i] = i;
+}
+
+// Run three times to exercise caches.
+store_i_in_element_i_of_object_i();
+store_i_in_element_i_of_object_i();
+store_i_in_element_i_of_object_i();