Remove the old builtins framework with the huge, nasty

macro.
Review URL: http://codereview.chromium.org/3079

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

src/bootstrapper.cc

@@ -486,14 +486,15 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
       Factory::NewFunction(symbol, Factory::null_value());
 
   {  // --- E m p t y ---
-    Handle<Code> call_code =
+    Handle<Code> code =
         Handle<Code>(Builtins::builtin(Builtins::EmptyFunction));
     Handle<String> source = Factory::NewStringFromAscii(CStrVector("() {}"));
 
-    empty_function->set_code(*call_code);
+    empty_function->set_code(*code);
     empty_function->shared()->set_script(*Factory::NewScript(source));
     empty_function->shared()->set_start_position(0);
     empty_function->shared()->set_end_position(source->length());
+    empty_function->shared()->DontAdaptArguments();
     global_context()->function_map()->set_prototype(*empty_function);
     global_context()->function_instance_map()->set_prototype(*empty_function);
 
@@ -580,6 +581,7 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
         InstallFunction(global, "Array", JS_ARRAY_TYPE, JSArray::kSize,
                         Top::initial_object_prototype(), Builtins::ArrayCode,
                         true);
+    array_function->shared()->DontAdaptArguments();
 
     // This seems a bit hackish, but we need to make sure Array.length
     // is 1.
@@ -709,6 +711,7 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
       Factory::NewFunction(Factory::empty_symbol(), JS_OBJECT_TYPE,
                            JSObject::kHeaderSize, code, true);
   global_context()->set_call_as_function_delegate(*delegate);
+  delegate->shared()->DontAdaptArguments();
 
   global_context()->set_special_function_table(Heap::empty_fixed_array());
 

src/builtins-arm.cc

@@ -37,24 +37,15 @@ namespace v8 { namespace internal {
 #define __ masm->
 
 
-void Builtins::Generate_Adaptor(MacroAssembler* masm,
-                                int argc,
-                                CFunctionId id) {
+void Builtins::Generate_Adaptor(MacroAssembler* masm, CFunctionId id) {
   // 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));
-  }
+  // 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));
   __ JumpToBuiltin(ExternalReference(id));
 }
 

src/builtins-ia32.cc

@@ -37,24 +37,15 @@ namespace v8 { namespace internal {
 #define __ masm->
 
 
-void Builtins::Generate_Adaptor(MacroAssembler* masm,
-                                int argc,
-                                CFunctionId id) {
+void Builtins::Generate_Adaptor(MacroAssembler* masm, CFunctionId id) {
   // 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);
-  }
+  // 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);
   __ JumpToBuiltin(ExternalReference(id));
 }
 

src/builtins.cc

@@ -40,16 +40,13 @@ namespace v8 { namespace internal {
 //
 // A builtin function is defined by writing:
 //
-//   BUILTIN_<n>(name, ...)
-//   {
+//   BUILTIN(name) {
 //     ...
 //   }
 //   BUILTIN_END
 //
-// where <n> is the number of arguments (not counting the receiver). The
-// names of the arguments must be listed after the name in the declaration.
-// In the body of the builtin function, the variables 'env' and 'receiver'
-// are visible. The arguments can be accessed through:
+// In the body of the builtin function, the variable 'receiver' is visible.
+// The arguments can be accessed through:
 //
 //   BUILTIN_ARG(0): Receiver (also available as 'receiver')
 //   BUILTIN_ARG(1): First argument
@@ -63,43 +60,10 @@ namespace v8 { namespace internal {
 // ----------------------------------------------------------------------------
 
 
-// TODO(1238487): This is not okay. We need to get rid of this macro
-// 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.
+// TODO(1238487): We should consider passing whether or not the
+// builtin was invoked as a constructor as part of the
+// arguments. Maybe we also want to pass the called function?
 #define BUILTIN(name)                                                   \
-  static Object* Builtin_##name(int __argc__,                           \
-                                Object** __argv__) {                    \
-    Handle<Object> receiver(&__argv__[0]);                              \
-    bool is_construct = false;                                          \
-    USE(__argc__);                                                      \
-    USE(__argv__);                                                      \
-    { StackFrameIterator it;                                            \
-      ASSERT(it.frame()->is_exit());                                    \
-      it.Advance();                                                     \
-      StackFrame::Type type = it.frame()->type();                       \
-      if (type == StackFrame::INTERNAL) {                               \
-        InternalFrame* frame = InternalFrame::cast(it.frame());         \
-        is_construct = frame->is_construct_trampoline();                \
-      } else if (type == StackFrame::ARGUMENTS_ADAPTOR) {               \
-        ArgumentsAdaptorFrame* frame =                                  \
-            ArgumentsAdaptorFrame::cast(it.frame());                    \
-        /* __argc__ includes the receiver. */                           \
-        __argc__ = frame->GetProvidedParametersCount() + 1;             \
-        __argv__ = reinterpret_cast<Object**>(frame->pp()) - 1;         \
-        it.Advance();                                                   \
-        is_construct =                                                  \
-            it.frame()->is_internal() &&                                \
-            InternalFrame::cast(it.frame())->is_construct_trampoline(); \
-      }                                                                 \
-    }
-
-
-// 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]);
 
@@ -122,6 +86,19 @@ static inline Object* __builtin_arg__(int n, int argc, Object** argv) {
 }
 
 
+// TODO(1238487): Get rid of this function that determines if the
+// builtin is called as a constructor. This may be a somewhat slow
+// operation due to the stack frame iteration.
+static inline bool CalledAsConstructor() {
+  StackFrameIterator it;
+  ASSERT(it.frame()->is_exit());
+  it.Advance();
+  StackFrame* frame = it.frame();
+  return frame->is_internal() &&
+      InternalFrame::cast(frame)->is_construct_trampoline();
+}
+
+
 // ----------------------------------------------------------------------------
 
 
@@ -179,7 +156,7 @@ BUILTIN_END
 
 BUILTIN(ArrayCode) {
   JSArray* array;
-  if (is_construct) {
+  if (CalledAsConstructor()) {
     array = JSArray::cast(*receiver);
   } else {
     // Allocate the JS Array
@@ -235,7 +212,7 @@ BUILTIN(ArrayCode) {
 BUILTIN_END
 
 
-NEW_BUILTIN(ArrayPush) {
+BUILTIN(ArrayPush) {
   JSArray* array = JSArray::cast(*receiver);
   ASSERT(array->HasFastElements());
 
@@ -274,7 +251,7 @@ NEW_BUILTIN(ArrayPush) {
 BUILTIN_END
 
 
-NEW_BUILTIN(ArrayPop) {
+BUILTIN(ArrayPop) {
   JSArray* array = JSArray::cast(*receiver);
   ASSERT(array->HasFastElements());
   Object* undefined = Heap::undefined_value();
@@ -361,6 +338,7 @@ static inline Object* TypeCheck(int argc,
 
 BUILTIN(HandleApiCall) {
   HandleScope scope;
+  bool is_construct = CalledAsConstructor();
 
   // TODO(1238487): This is not nice. We need to get rid of this
   // kludgy behavior and start handling API calls in a more direct
@@ -441,7 +419,7 @@ BUILTIN_END
 // support calls.
 BUILTIN(HandleApiCallAsFunction) {
   // Non-functions are never called as constructors.
-  ASSERT(!is_construct);
+  ASSERT(!CalledAsConstructor());
 
   // Get the object called.
   JSObject* obj = JSObject::cast(*receiver);
@@ -474,7 +452,7 @@ BUILTIN(HandleApiCallAsFunction) {
         data,
         self,
         callee,
-        is_construct,
+        false,
         reinterpret_cast<void**>(__argv__ - 1),
         __argc__ - 1);
     v8::Handle<v8::Value> value;
@@ -606,7 +584,7 @@ static void Generate_KeyedStoreIC_Initialize(MacroAssembler* masm) {
 Object* Builtins::builtins_[builtin_count] = { NULL, };
 const char* Builtins::names_[builtin_count] = { NULL, };
 
-#define DEF_ENUM_C(name, ignore) FUNCTION_ADDR(Builtin_##name),
+#define DEF_ENUM_C(name) FUNCTION_ADDR(Builtin_##name),
   Address Builtins::c_functions_[cfunction_count] = {
     BUILTIN_LIST_C(DEF_ENUM_C)
   };
@@ -637,16 +615,14 @@ void Builtins::Setup(bool create_heap_objects) {
     const char* s_name;  // name is only used for generating log information.
     int name;
     Code::Flags flags;
-    int argc;
   };
 
-#define DEF_FUNCTION_PTR_C(name, argc)   \
+#define DEF_FUNCTION_PTR_C(name)         \
     { FUNCTION_ADDR(Generate_Adaptor),   \
       FUNCTION_ADDR(Builtin_##name),     \
       #name,                             \
       c_##name,                          \
-      Code::ComputeFlags(Code::BUILTIN), \
-      argc                               \
+      Code::ComputeFlags(Code::BUILTIN)  \
     },
 
 #define DEF_FUNCTION_PTR_A(name, kind, state) \
@@ -654,8 +630,7 @@ void Builtins::Setup(bool create_heap_objects) {
       NULL,                                   \
       #name,                                  \
       name,                                   \
-      Code::ComputeFlags(Code::kind, state),  \
-      -1                                      \
+      Code::ComputeFlags(Code::kind, state)   \
     },
 
   // Define array of pointers to generators and C builtin functions.
@@ -663,7 +638,7 @@ void Builtins::Setup(bool create_heap_objects) {
       BUILTIN_LIST_C(DEF_FUNCTION_PTR_C)
       BUILTIN_LIST_A(DEF_FUNCTION_PTR_A)
       // Terminator:
-      { NULL, NULL, NULL, builtin_count, static_cast<Code::Flags>(0), -1}
+      { NULL, NULL, NULL, builtin_count, static_cast<Code::Flags>(0) }
   };
 
 #undef DEF_FUNCTION_PTR_C
@@ -679,12 +654,12 @@ void Builtins::Setup(bool create_heap_objects) {
     if (create_heap_objects) {
       MacroAssembler masm(buffer, sizeof buffer);
       // Generate the code/adaptor.
-      typedef void (*Generator)(MacroAssembler*, int, int);
+      typedef void (*Generator)(MacroAssembler*, int);
       Generator g = FUNCTION_CAST<Generator>(functions[i].generator);
       // We pass all arguments to the generator, but it may not use all of
       // them.  This works because the first arguments are on top of the
       // stack.
-      g(&masm, functions[i].argc, functions[i].name);
+      g(&masm, functions[i].name);
       // Move the code into the object heap.
       CodeDesc desc;
       masm.GetCode(&desc);

src/builtins.h

@@ -32,17 +32,17 @@ namespace v8 { namespace internal {
 
 // Define list of builtins implemented in C.
 #define BUILTIN_LIST_C(V)                          \
-  V(Illegal, 0)                                    \
+  V(Illegal)                                       \
                                                    \
-  V(EmptyFunction, 0)                              \
+  V(EmptyFunction)                                 \
                                                    \
-  V(ArrayCode, 0)                                  \
+  V(ArrayCode)                                     \
                                                    \
-  V(ArrayPush, -1)                                 \
-  V(ArrayPop, -1)                                  \
+  V(ArrayPush)                                     \
+  V(ArrayPop)                                      \
                                                    \
-  V(HandleApiCall, 0)                              \
-  V(HandleApiCallAsFunction, 0)
+  V(HandleApiCall)                                 \
+  V(HandleApiCallAsFunction)
 
 
 // Define list of builtins implemented in assembly.
@@ -141,7 +141,7 @@ class Builtins : public AllStatic {
   static const char* Lookup(byte* pc);
 
   enum Name {
-#define DEF_ENUM_C(name, ignore) name,
+#define DEF_ENUM_C(name) name,
 #define DEF_ENUM_A(name, kind, state) name,
     BUILTIN_LIST_C(DEF_ENUM_C)
     BUILTIN_LIST_A(DEF_ENUM_A)
@@ -151,7 +151,7 @@ class Builtins : public AllStatic {
   };
 
   enum CFunctionId {
-#define DEF_ENUM_C(name, ignore) c_##name,
+#define DEF_ENUM_C(name) c_##name,
     BUILTIN_LIST_C(DEF_ENUM_C)
 #undef DEF_ENUM_C
     cfunction_count
@@ -200,7 +200,6 @@ class Builtins : public AllStatic {
   // function f, we use an Object* array here.
   static Object* builtins_[builtin_count];
   static const char* names_[builtin_count];
-
   static const char* javascript_names_[id_count];
   static int javascript_argc_[id_count];
 
@@ -210,9 +209,7 @@ class Builtins : public AllStatic {
   static int construct_call_pc_offset_;
   static int arguments_adaptor_call_pc_offset_;
 
-  static void Generate_Adaptor(MacroAssembler* masm,
-                               int argc,
-                               CFunctionId id);
+  static void Generate_Adaptor(MacroAssembler* masm, CFunctionId id);
   static void Generate_JSConstructCall(MacroAssembler* masm);
   static void Generate_JSEntryTrampoline(MacroAssembler* masm);
   static void Generate_JSConstructEntryTrampoline(MacroAssembler* masm);

src/factory.cc

@@ -727,6 +727,7 @@ Handle<JSFunction> Factory::CreateApiFunction(
   }
 
   result->shared()->set_function_data(*obj);
+  result->shared()->DontAdaptArguments();
 
   // Recursively copy parent templates' accessors, 'data' may be modified.
   Handle<DescriptorArray> array =

src/parser.cc

@@ -1138,6 +1138,8 @@ Statement* Parser::ParseNativeDeclaration(bool* ok) {
   // builtins.cc:HandleApiCall to perform argument type checks and to
   // find the right native code to call.
   boilerplate->shared()->set_function_data(fun->shared()->function_data());
+  int parameters = fun->shared()->formal_parameter_count();
+  boilerplate->shared()->set_formal_parameter_count(parameters);
 
   // TODO(1240846): It's weird that native function declarations are
   // introduced dynamically when we meet their declarations, whereas

src/serialize.cc

@@ -452,7 +452,7 @@ ExternalReferenceTable::ExternalReferenceTable() : refs_(64) {
   // Define all entries in the table.
 
   // Builtins
-#define DEF_ENTRY_C(name, ignore) \
+#define DEF_ENTRY_C(name) \
   Add(Builtins::c_function_address(Builtins::c_##name), \
       C_BUILTIN, \
       Builtins::c_##name, \
@@ -461,12 +461,12 @@ ExternalReferenceTable::ExternalReferenceTable() : refs_(64) {
   BUILTIN_LIST_C(DEF_ENTRY_C)
 #undef DEF_ENTRY_C
 
-#define DEF_ENTRY_C(name, ignore) \
+#define DEF_ENTRY_C(name) \
   Add(Builtins::builtin_address(Builtins::name), \
       BUILTIN, \
       Builtins::name, \
       "Builtins::" #name);
-#define DEF_ENTRY_A(name, kind, state) DEF_ENTRY_C(name, _)
+#define DEF_ENTRY_A(name, kind, state) DEF_ENTRY_C(name)
 
   BUILTIN_LIST_C(DEF_ENTRY_C)
   BUILTIN_LIST_A(DEF_ENTRY_A)