Stop adapting the arguments passed to the builtin implementations

of Array.prototype.push and Array.prototype.pop. Avoid going 
through the arguments adaptor trampoline for call ICs that end
up calling a builtin that does not need arguments adaption.
Review URL: http://codereview.chromium.org/2884

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

src/bootstrapper.cc

@@ -291,8 +291,7 @@ class Genesis BASE_EMBEDDED {
 
   void AddSpecialFunction(Handle<JSObject> prototype,
                           const char* name,
-                          Handle<Code> code,
-                          int parameter_count);
+                          Handle<Code> code);
 
   void BuildSpecialFunctionTable();
 
@@ -1266,8 +1265,7 @@ void Genesis::MakeFunctionInstancePrototypeWritable() {
 
 void Genesis::AddSpecialFunction(Handle<JSObject> prototype,
                                  const char* name,
-                                 Handle<Code> code,
-                                 int parameter_count) {
+                                 Handle<Code> code) {
   Handle<String> key = Factory::LookupAsciiSymbol(name);
   Handle<Object> value = Handle<Object>(prototype->GetProperty(*key));
   if (value->IsJSFunction()) {
@@ -1276,7 +1274,7 @@ void Genesis::AddSpecialFunction(Handle<JSObject> prototype,
                                                         JSObject::kHeaderSize,
                                                         code,
                                                         false);
-    optimized->shared()->set_formal_parameter_count(parameter_count);
+    optimized->shared()->DontAdaptArguments();
     int len = global_context()->special_function_table()->length();
     Handle<FixedArray> new_array = Factory::NewFixedArray(len + 3);
     for (int index = 0; index < len; index++) {
@@ -1301,11 +1299,9 @@ void Genesis::BuildSpecialFunctionTable() {
   Handle<JSObject> prototype =
       Handle<JSObject>(JSObject::cast(function->prototype()));
   AddSpecialFunction(prototype, "pop",
-                     Handle<Code>(Builtins::builtin(Builtins::ArrayPop)),
-                     0);
+                     Handle<Code>(Builtins::builtin(Builtins::ArrayPop)));
   AddSpecialFunction(prototype, "push",
-                     Handle<Code>(Builtins::builtin(Builtins::ArrayPush)),
-                     1);
+                     Handle<Code>(Builtins::builtin(Builtins::ArrayPush)));
 }
 
 

src/builtins-arm.cc

@@ -40,12 +40,21 @@ namespace v8 { namespace internal {
 void Builtins::Generate_Adaptor(MacroAssembler* masm,
                                 int argc,
                                 CFunctionId id) {
-  // r0 contains the number of arguments excluding the receiver.
-  // JumpToBuiltin expects r0 to contains the number of arguments
-  // including the receiver.
-  __ mov(r0, Operand(argc + 1));
+  // TODO(1238487): Don't pass the function in a static variable.
   __ mov(ip, Operand(ExternalReference::builtin_passed_function()));
   __ str(r1, MemOperand(ip, 0));
+
+  if (argc == -1) {
+    // The actual argument count has already been loaded into register
+    // r0, but JumpToBuiltin expects r0 to contain the number of
+    // arguments including the receiver.
+    __ add(r0, r0, Operand(1));
+  } else {
+    // The number passed in argc excludes the receiver, but
+    // JumpToBuiltin expects r0 to contain the number of arguments
+    // including the receiver.
+    __ mov(r0, Operand(argc + 1));
+  }
   __ JumpToBuiltin(ExternalReference(id));
 }
 

src/builtins-ia32.cc

@@ -40,12 +40,21 @@ namespace v8 { namespace internal {
 void Builtins::Generate_Adaptor(MacroAssembler* masm,
                                 int argc,
                                 CFunctionId id) {
-  // argc is the number of arguments excluding the receiver.
-  // JumpToBuiltin expects eax to contain the number of arguments
-  // including the receiver.
-  __ mov(eax, argc + 1);
-  __ mov(Operand::StaticVariable(ExternalReference::builtin_passed_function()),
-         edi);
+  // TODO(1238487): Don't pass the function in a static variable.
+  ExternalReference passed = ExternalReference::builtin_passed_function();
+  __ mov(Operand::StaticVariable(passed), edi);
+
+  if (argc == -1) {
+    // The actual argument count has already been loaded into register
+    // eax, but JumpToBuiltin expects eax to contain the number of
+    // arguments including the receiver.
+    __ inc(eax);
+  } else {
+    // The number passed in argc excludes the receiver, but
+    // JumpToBuiltin expects eax to contain the number of arguments
+    // including the receiver.
+    __ mov(eax, argc + 1);
+  }
   __ JumpToBuiltin(ExternalReference(id));
 }
 

src/builtins.cc

@@ -67,7 +67,7 @@ namespace v8 { namespace internal {
 // and start calling the builtins in a more direct way. Looking at the
 // stack frames for all builtin invocations comes with a pretty
 // significant performance penalty.
-#define BUILTIN_0(name)                                                 \
+#define BUILTIN(name)                                                   \
   static Object* Builtin_##name(int __argc__,                           \
                                 Object** __argv__) {                    \
     Handle<Object> receiver(&__argv__[0]);                              \
@@ -95,19 +95,13 @@ namespace v8 { namespace internal {
     }
 
 
-#define BUILTIN_1(name, a0)     \
-  BUILTIN_0(name)               \
-  Object* a0 = BUILTIN_ARG(1);
-
-
-#define BUILTIN_2(name, a0, a1)  \
-  BUILTIN_1(name, a0)            \
-  Object* a1 = BUILTIN_ARG(2);
-
-
-#define BUILTIN_3(name, a0, a1, a2)  \
-  BUILTIN_2(name, a0, a1)            \
-  Object* a2 = BUILTIN_ARG(3);
+// We're transitioning to a much simpler builtins framework where all
+// builtins are called *without* arguments adaption. For now, only a
+// few of the builtins have been rewritten to support this and they
+// all use the NEW_BUILTIN macro instead of plain old BUILTIN.
+#define NEW_BUILTIN(name)                                               \
+  static Object* Builtin_##name(int __argc__, Object** __argv__) {      \
+    Handle<Object> receiver(&__argv__[0]);
 
 
 // Use an inline function to avoid evaluating the index (n) more than
@@ -172,18 +166,18 @@ Handle<Code> Builtins::GetCode(JavaScript id, bool* resolved) {
 }
 
 
-BUILTIN_0(Illegal) {
+BUILTIN(Illegal) {
   UNREACHABLE();
 }
 BUILTIN_END
 
 
-BUILTIN_0(EmptyFunction) {
+BUILTIN(EmptyFunction) {
 }
 BUILTIN_END
 
 
-BUILTIN_0(ArrayCode) {
+BUILTIN(ArrayCode) {
   JSArray* array;
   if (is_construct) {
     array = JSArray::cast(*receiver);
@@ -241,7 +235,7 @@ BUILTIN_0(ArrayCode) {
 BUILTIN_END
 
 
-BUILTIN_0(ArrayPush) {
+NEW_BUILTIN(ArrayPush) {
   JSArray* array = JSArray::cast(*receiver);
   ASSERT(array->HasFastElements());
 
@@ -280,7 +274,7 @@ BUILTIN_0(ArrayPush) {
 BUILTIN_END
 
 
-BUILTIN_0(ArrayPop) {
+NEW_BUILTIN(ArrayPop) {
   JSArray* array = JSArray::cast(*receiver);
   ASSERT(array->HasFastElements());
   Object* undefined = Heap::undefined_value();
@@ -365,7 +359,7 @@ static inline Object* TypeCheck(int argc,
 }
 
 
-BUILTIN_0(HandleApiCall) {
+BUILTIN(HandleApiCall) {
   HandleScope scope;
 
   // TODO(1238487): This is not nice. We need to get rid of this
@@ -445,7 +439,7 @@ BUILTIN_END
 
 // Handle calls to non-function objects created through the API that
 // support calls.
-BUILTIN_0(HandleApiCallAsFunction) {
+BUILTIN(HandleApiCallAsFunction) {
   // Non-functions are never called as constructors.
   ASSERT(!is_construct);
 

src/builtins.h

@@ -38,8 +38,8 @@ namespace v8 { namespace internal {
                                                    \
   V(ArrayCode, 0)                                  \
                                                    \
-  V(ArrayPush, 1)                                  \
-  V(ArrayPop, 0)                                   \
+  V(ArrayPush, -1)                                 \
+  V(ArrayPop, -1)                                  \
                                                    \
   V(HandleApiCall, 0)                              \
   V(HandleApiCallAsFunction, 0)

src/macro-assembler-arm.cc

@@ -299,7 +299,16 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       definitely_matches = true;
     } else {
       mov(r0, Operand(actual.immediate()));
-      mov(r2, Operand(expected.immediate()));
+      const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
+      if (expected.immediate() == sentinel) {
+        // Don't worry about adapting arguments for builtins that
+        // don't want that done. Skip adaption code by making it look
+        // like we have a match between expected and actual number of
+        // arguments.
+        definitely_matches = true;
+      } else {
+        mov(r2, Operand(expected.immediate()));
+      }
     }
   } else {
     if (actual.is_immediate()) {

src/macro-assembler-ia32.cc

@@ -572,7 +572,16 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       definitely_matches = true;
     } else {
       mov(eax, actual.immediate());
-      mov(ebx, expected.immediate());
+      const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
+      if (expected.immediate() == sentinel) {
+        // Don't worry about adapting arguments for builtins that
+        // don't want that done. Skip adaption code by making it look
+        // like we have a match between expected and actual number of
+        // arguments.
+        definitely_matches = true;
+      } else {
+        mov(ebx, expected.immediate());
+      }
     }
   } else {
     if (actual.is_immediate()) {

src/objects-inl.h

@@ -1783,6 +1783,7 @@ INT_ACCESSORS(SharedFunctionInfo, function_token_position,
 
 
 void SharedFunctionInfo::DontAdaptArguments() {
+  ASSERT(code()->kind() == Code::BUILTIN);
   set_formal_parameter_count(kDontAdaptArgumentsSentinel);
 }