Remove spills from most inlined runtime calls.

src/codegen-ia32.cc

 // Copyright 2006-2008 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 3411 matching lines @@
   frame_->CallStub(&call_function, arg_count + 1);
 
   // Restore context and pop function from the stack.
   frame_->RestoreContextRegister();
   __ mov(frame_->Top(), eax);
 }
 
 
 void CodeGenerator::GenerateIsSmi(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 1);
-  LoadAndSpill(args->at(0));
-  frame_->EmitPop(eax);
-  __ test(eax, Immediate(kSmiTagMask));
+  Load(args->at(0));
+  Result value = frame_->Pop();
+  value.ToRegister();
+  ASSERT(value.is_valid());
+  __ test(value.reg(), Immediate(kSmiTagMask));
+  value.Unuse();
   true_target()->Branch(zero);
   false_target()->Jump();
 }
 
 
 void CodeGenerator::GenerateLog(ZoneList<Expression*>* args) {
   // Conditionally generate a log call.
   // Args:
   //   0 (literal string): The type of logging (corresponds to the flags).
   //     This is used to determine whether or not to generate the log call.
   //   1 (string): Format string.  Access the string at argument index 2
   //     with '%2s' (see Logger::LogRuntime for all the formats).
   //   2 (array): Arguments to the format string.
   ASSERT_EQ(args->length(), 3);
 #ifdef ENABLE_LOGGING_AND_PROFILING
   if (ShouldGenerateLog(args->at(0))) {
-    LoadAndSpill(args->at(1));
-    LoadAndSpill(args->at(2));
+    Load(args->at(1));
+    Load(args->at(2));
     frame_->CallRuntime(Runtime::kLog, 2);
   }
 #endif
   // Finally, we're expected to leave a value on the top of the stack.
-  frame_->EmitPush(Immediate(Factory::undefined_value()));
+  frame_->Push(Factory::undefined_value());
 }
 
 
 void CodeGenerator::GenerateIsNonNegativeSmi(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 1);
-  LoadAndSpill(args->at(0));
-  frame_->EmitPop(eax);
-  __ test(eax, Immediate(kSmiTagMask | 0x80000000));
+  Load(args->at(0));
+  Result value = frame_->Pop();
+  value.ToRegister();
+  ASSERT(value.is_valid());
+  __ test(value.reg(), Immediate(kSmiTagMask | 0x80000000));
+  value.Unuse();
   true_target()->Branch(zero);
   false_target()->Jump();
 }
 
 
 // This generates code that performs a charCodeAt() call or returns
 // undefined in order to trigger the slow case, Runtime_StringCharCodeAt.
 // It can handle flat and sliced strings, 8 and 16 bit characters and
 // cons strings where the answer is found in the left hand branch of the
 // cons.  The slow case will flatten the string, which will ensure that
 // the answer is in the left hand side the next time around.
 void CodeGenerator::GenerateFastCharCodeAt(ZoneList<Expression*>* args) {
+  VirtualFrame::SpilledScope spilled_scope(this);
   ASSERT(args->length() == 2);
 
   JumpTarget slow_case(this);
   JumpTarget end(this);
   JumpTarget not_a_flat_string(this);
   JumpTarget not_a_cons_string_either(this);
   JumpTarget try_again_with_new_string(this, JumpTarget::BIDIRECTIONAL);
   JumpTarget ascii_string(this);
   JumpTarget got_char_code(this);
 
@@ skipping 92 matching lines @@
 
   slow_case.Bind();
   frame_->EmitPush(Immediate(Factory::undefined_value()));
 
   end.Bind();
 }
 
 
 void CodeGenerator::GenerateIsArray(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 1);
-  LoadAndSpill(args->at(0));
-  // We need the CC bits to come out as not_equal in the case where the

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

It's nice to get rid of this.

-  // object is a smi.  This can't be done with the usual test opcode so
-  // we copy the object to ecx and do some destructive ops on it that
-  // result in the right CC bits.
-  frame_->EmitPop(eax);
-  __ mov(ecx, Operand(eax));
-  __ and_(ecx, kSmiTagMask);
-  __ xor_(ecx, kSmiTagMask);
-  false_target()->Branch(not_equal);
+  Load(args->at(0));
+  Result value = frame_->Pop();
+  value.ToRegister();
+  ASSERT(value.is_valid());
+  __ test(value.reg(), Immediate(kSmiTagMask));
+  false_target()->Branch(equal);
   // It is a heap object - get map.
-  __ mov(eax, FieldOperand(eax, HeapObject::kMapOffset));
-  __ movzx_b(eax, FieldOperand(eax, Map::kInstanceTypeOffset));
+  Result temp = allocator()->Allocate();

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

We will need to sort out the decision to spill an existing register (eg, value)
or allocate a fresh one.  A simple solution is if the existing register is
unaliased in the frame, reuse it, otherwise allocate.

A more sophisticated solution considers the cost of spilling vs. allocating. 
Allocating will never consider the actually unneeded off-frame register, because
it thinks it is needed.

+  ASSERT(temp.is_valid());
+  __ mov(temp.reg(), FieldOperand(value.reg(), HeapObject::kMapOffset));
+  __ movzx_b(temp.reg(), FieldOperand(temp.reg(), Map::kInstanceTypeOffset));
   // Check if the object is a JS array or not.
-  __ cmp(eax, JS_ARRAY_TYPE);
+  __ cmp(temp.reg(), JS_ARRAY_TYPE);
+  value.Unuse();
+  temp.Unuse();
   true_target()->Branch(equal);
   false_target()->Jump();
 }
 
 
 void CodeGenerator::GenerateArgumentsLength(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 0);
 
   // Seed the result with the formal parameters count, which will be

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

I would reword this comment to be more explicit.  The real point is that the
(read length version of the) arguments access stub takes the parameter count as
an argument passed in eax.

[William Hesse, 2012/05/07 09:36:09]

Done.

   // used in case no arguments adaptor frame is found below the
   // current frame.
-  __ Set(eax, Immediate(Smi::FromInt(scope_->num_parameters())));
+  Result initial_value = allocator()->Allocate(eax);
+  ASSERT(initial_value.is_valid());
+  __ Set(initial_value.reg(),
+         Immediate(Smi::FromInt(scope_->num_parameters())));
 
   // Call the shared stub to get to the arguments.length.
   ArgumentsAccessStub stub(ArgumentsAccessStub::READ_LENGTH);
-  frame_->CallStub(&stub, 0);
-  frame_->EmitPush(eax);
+  Result result = frame_->CallStub(&stub, &initial_value, 0);
+  frame_->Push(&result);
 }
 
 
 void CodeGenerator::GenerateValueOf(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 1);
   JumpTarget leave(this);
-  LoadAndSpill(args->at(0));  // Load the object.
-  __ mov(eax, frame_->Top());
+  Load(args->at(0));  // Load the object.
+  frame_->Dup();
+  Result object = frame_->Pop();
+  object.ToRegister();
+  ASSERT(object.is_valid());
   // if (object->IsSmi()) return object.
-  __ test(eax, Immediate(kSmiTagMask));
+  __ test(object.reg(), Immediate(kSmiTagMask));
   leave.Branch(zero, taken);
   // It is a heap object - get map.
-  __ mov(ecx, FieldOperand(eax, HeapObject::kMapOffset));
-  __ movzx_b(ecx, FieldOperand(ecx, Map::kInstanceTypeOffset));
+  Result temp = allocator()->Allocate();
+  ASSERT(temp.is_valid());
+  __ mov(temp.reg(), FieldOperand(object.reg(), HeapObject::kMapOffset));
+  __ movzx_b(temp.reg(), FieldOperand(temp.reg(), Map::kInstanceTypeOffset));
   // if (!object->IsJSValue()) return object.
-  __ cmp(ecx, JS_VALUE_TYPE);
+  __ cmp(temp.reg(), JS_VALUE_TYPE);
   leave.Branch(not_equal, not_taken);
-  __ mov(eax, FieldOperand(eax, JSValue::kValueOffset));
-  __ mov(frame_->Top(), eax);
+  __ mov(temp.reg(), FieldOperand(object.reg(), JSValue::kValueOffset));
+  object.Unuse();
+  frame_->Drop();
+  frame_->Push(&temp);

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

Can we avoid the drop/push sequence here by setting the top element?

[William Hesse, 2012/05/07 09:36:09]

Done.

   leave.Bind();
 }
 
 
 void CodeGenerator::GenerateSetValueOf(ZoneList<Expression*>* args) {
+  VirtualFrame::SpilledScope spilled_scope(this);
   ASSERT(args->length() == 2);
   JumpTarget leave(this);
   LoadAndSpill(args->at(0));  // Load the object.
   LoadAndSpill(args->at(1));  // Load the value.
   __ mov(eax, frame_->ElementAt(1));
   __ mov(ecx, frame_->Top());
-  // if (object->IsSmi()) return object.
+  // if (object->IsSmi()) return value.
   __ test(eax, Immediate(kSmiTagMask));
   leave.Branch(zero, taken);
   // It is a heap object - get map.
   __ mov(ebx, FieldOperand(eax, HeapObject::kMapOffset));
   __ movzx_b(ebx, FieldOperand(ebx, Map::kInstanceTypeOffset));
-  // if (!object->IsJSValue()) return object.
+  // if (!object->IsJSValue()) return value.
   __ cmp(ebx, JS_VALUE_TYPE);
   leave.Branch(not_equal, not_taken);
   // Store the value.
   __ mov(FieldOperand(eax, JSValue::kValueOffset), ecx);
   // Update the write barrier.
   __ RecordWrite(eax, JSValue::kValueOffset, ecx, ebx);
   // Leave.
   leave.Bind();
   __ mov(ecx, frame_->Top());
   frame_->Drop();
   __ mov(frame_->Top(), ecx);
 }
 
 
 void CodeGenerator::GenerateArgumentsAccess(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 1);
 
   // Load the key onto the stack and set register eax to the formal
   // parameters count for the currently executing function.
-  LoadAndSpill(args->at(0));
-  __ Set(eax, Immediate(Smi::FromInt(scope_->num_parameters())));
+  Load(args->at(0));
+  Result parameters_count = allocator()->Allocate(eax);
+  ASSERT(parameters_count.is_valid());
+  __ Set(parameters_count.reg(),
+         Immediate(Smi::FromInt(scope_->num_parameters())));
 
   // Call the shared stub to get to arguments[key].
   ArgumentsAccessStub stub(ArgumentsAccessStub::READ_ELEMENT);
-  frame_->CallStub(&stub, 0);
-  __ mov(frame_->Top(), eax);
+  Result result = frame_->CallStub(&stub, &parameters_count, 0);
+  frame_->Drop();

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

Can we avoid the drop/push sequence here by setting the top element?

[William Hesse, 2012/05/07 09:36:09]

Done.

+  frame_->Push(&result);
 }
 
 
 void CodeGenerator::GenerateObjectEquals(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 2);
 
   // Load the two objects into registers and perform the comparison.
-  LoadAndSpill(args->at(0));
-  LoadAndSpill(args->at(1));
-  frame_->EmitPop(eax);
-  frame_->EmitPop(ecx);
-  __ cmp(eax, Operand(ecx));
+  Load(args->at(0));
+  Load(args->at(1));
+  Result right = frame_->Pop();
+  Result left = frame_->Pop();
+  right.ToRegister();
+  left.ToRegister();

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

I would rather not move left to a register, but generate a difference cmp
instruction in the case it's a constant.

[William Hesse, 2012/05/07 09:36:09]

What if right is a constant?  Let's do this when we start handling constants
differently.

+  __ cmp(right.reg(), Operand(left.reg()));
+  right.Unuse();
+  left.Unuse();
   true_target()->Branch(equal);
   false_target()->Jump();
 }
 
 
 void CodeGenerator::VisitCallRuntime(CallRuntime* node) {
-  VirtualFrame::SpilledScope spilled_scope(this);
   if (CheckForInlineRuntimeCall(node)) {
     return;
   }
+  VirtualFrame::SpilledScope spilled_scope(this);

[Kevin Millikin (Chromium), 2009/01/29 09:42:36]

Double check that this is scope is gone once you update to include your
immediately previous change list.

 
   ZoneList<Expression*>* args = node->arguments();
   Comment cmnt(masm_, "[ CallRuntime");
   Runtime::Function* function = node->function();
 
   if (function == NULL) {
     // Prepare stack for calling JS runtime function.
     frame_->EmitPush(Immediate(node->name()));
     // Push the builtins object found in the current global object.
     __ mov(edx, GlobalObject());
@@ skipping 2509 matching lines @@
 
   // Slow-case: Go through the JavaScript implementation.
   __ bind(&slow);
   __ InvokeBuiltin(Builtins::INSTANCE_OF, JUMP_FUNCTION);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal