[builtins] Extract isolate-independent metadata

Instead of generating huge switch statements for builtin accessor functions,
simply store isolate-independent metadata in a struct indexed by builtin-id.

Drive-by-fix: Remove duplicate parameter-count lookup accessor.
Drive-by-fix: Print builtin kind with --print-builtin-size.

Bug: v8:6624
Change-Id: Ibe61eeee6b8849d5e1a2361ec8268b233be8bb13
Reviewed-on: https://chromium-review.googlesource.com/591847
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46994}

src/builtins/builtins.cc

@@ -20,6 +20,46 @@ namespace internal {
   Object* Builtin_##Name(int argc, Object** args, Isolate* isolate);
 BUILTIN_LIST_C(FORWARD_DECLARE)
 
+namespace {
+
+// TODO(jgruber): Pack in CallDescriptors::Key.
+struct BuiltinMetadata {
+  const char* name;
+  Builtins::Kind kind;
+  union {
+    Address cpp_entry;       // For CPP and API builtins.
+    int8_t parameter_count;  // For TFJ builtins.
+  } kind_specific_data;
+};
+
+// clang-format off
+#define DECL_CPP(Name, ...) { #Name, Builtins::CPP, \
+                              { FUNCTION_ADDR(Builtin_##Name) }},
+#define DECL_API(Name, ...) { #Name, Builtins::API, \
+                              { FUNCTION_ADDR(Builtin_##Name) }},
+#define DECL_TFJ(Name, Count, ...) { #Name, Builtins::TFJ, \
+                                     { reinterpret_cast<Address>(Count) }},
+#define DECL_TFC(Name, ...) { #Name, Builtins::TFC, {} },
+#define DECL_TFS(Name, ...) { #Name, Builtins::TFS, {} },
+#define DECL_TFH(Name, ...) { #Name, Builtins::TFH, {} },
+#define DECL_ASM(Name, ...) { #Name, Builtins::ASM, {} },
+#define DECL_DBG(Name, ...) { #Name, Builtins::DBG, {} },
+const BuiltinMetadata builtin_metadata[] = {
+  BUILTIN_LIST(DECL_CPP, DECL_API, DECL_TFJ, DECL_TFC, DECL_TFS, DECL_TFH,
+               DECL_ASM, DECL_DBG)
+};
+#undef DECL_CPP
+#undef DECL_API
+#undef DECL_TFJ
+#undef DECL_TFC
+#undef DECL_TFS
+#undef DECL_TFH
+#undef DECL_ASM
+#undef DECL_DBG
+// clang-format on
+
+}  // namespace
+
 Builtins::Builtins() : initialized_(false) {
   memset(builtins_, 0, sizeof(builtins_[0]) * builtin_count);
 }
@@ -27,29 +67,15 @@ Builtins::Builtins() : initialized_(false) {
 Builtins::~Builtins() {}
 
 BailoutId Builtins::GetContinuationBailoutId(Name name) {
-  switch (name) {
-#define BAILOUT_ID(NAME, ...) \
-  case k##NAME:               \
-    return BailoutId(BailoutId::kFirstBuiltinContinuationId + name);
-    BUILTIN_LIST_TFJ(BAILOUT_ID);
-    BUILTIN_LIST_TFC(BAILOUT_ID);
-#undef BAILOUT_ID
-    default:
-      UNREACHABLE();
-  }
+  DCHECK(Builtins::KindOf(name) == TFJ || Builtins::KindOf(name) == TFC);
+  return BailoutId(BailoutId::kFirstBuiltinContinuationId + name);
 }
 
 Builtins::Name Builtins::GetBuiltinFromBailoutId(BailoutId id) {
-  switch (id.ToInt()) {
-#define BAILOUT_ID(NAME, ...)                            \
-  case BailoutId::kFirstBuiltinContinuationId + k##NAME: \
-    return k##NAME;
-    BUILTIN_LIST_TFJ(BAILOUT_ID)
-    BUILTIN_LIST_TFC(BAILOUT_ID)
-#undef BAILOUT_ID
-    default:
-      UNREACHABLE();
-  }
+  int builtin_index = id.ToInt() - BailoutId::kFirstBuiltinContinuationId;
+  DCHECK(Builtins::KindOf(builtin_index) == TFJ ||
+         Builtins::KindOf(builtin_index) == TFC);
+  return static_cast<Name>(builtin_index);
 }
 
 void Builtins::TearDown() { initialized_ = false; }
@@ -122,18 +148,9 @@ Handle<Code> Builtins::builtin_handle(Name name) {
 }
 
 // static
-int Builtins::GetBuiltinParameterCount(Name name) {
-  switch (name) {
-#define TFJ_CASE(Name, ParamCount, ...) \
-  case k##Name: {                       \
-    return ParamCount;                  \
-  }
-    BUILTIN_LIST(IGNORE_BUILTIN, IGNORE_BUILTIN, TFJ_CASE, IGNORE_BUILTIN,
-                 IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN)
-#undef TFJ_CASE
-    default:
-      UNREACHABLE();
-  }
+int Builtins::GetStackParameterCount(Name name) {
+  DCHECK(Builtins::KindOf(name) == TFJ);
+  return builtin_metadata[name].kind_specific_data.parameter_count;
 }
 
 // static
@@ -186,104 +203,52 @@ Callable Builtins::CallableFor(Isolate* isolate, Name name) {
   return Callable(code, descriptor);
 }
 
-// static
-int Builtins::GetStackParameterCount(Isolate* isolate, Name name) {
-  switch (name) {
-#define CASE(Name, Count, ...) \
-  case k##Name: {              \
-    return Count;              \
-  }
-    BUILTIN_LIST_TFJ(CASE)
-#undef CASE
-    default:
-      UNREACHABLE();
-      return 0;
-  }
-}
-
 // static
 const char* Builtins::name(int index) {
-  switch (index) {
-#define CASE(Name, ...) \
-  case k##Name:         \
-    return #Name;
-    BUILTIN_LIST_ALL(CASE)
-#undef CASE
-    default:
-      UNREACHABLE();
-      break;
-  }
-  return "";
+  DCHECK(0 <= index && index < builtin_count);
+  return builtin_metadata[index].name;
 }
 
 // static
 Address Builtins::CppEntryOf(int index) {
-  DCHECK(0 <= index && index < builtin_count);
-  switch (index) {
-#define CASE(Name, ...) \
-  case k##Name:         \
-    return FUNCTION_ADDR(Builtin_##Name);
-    BUILTIN_LIST_C(CASE)
-#undef CASE
-    default:
-      return nullptr;
-  }
-  UNREACHABLE();
+  DCHECK(Builtins::HasCppImplementation(index));
+  return builtin_metadata[index].kind_specific_data.cpp_entry;
 }
 
 // static
-bool Builtins::IsCpp(int index) {
+Builtins::Kind Builtins::KindOf(int index) {
   DCHECK(0 <= index && index < builtin_count);
-  switch (index) {
-#define CASE(Name, ...) \
-  case k##Name:         \
-    return true;
-#define BUILTIN_LIST_CPP(V)                                       \
-  BUILTIN_LIST(V, IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN, \
-               IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN)
-    BUILTIN_LIST_CPP(CASE)
-#undef BUILTIN_LIST_CPP
-#undef CASE
-    default:
-      return false;
-  }
-  UNREACHABLE();
+  return builtin_metadata[index].kind;
 }
 
 // static
-bool Builtins::IsApi(int index) {
-  DCHECK(0 <= index && index < builtin_count);
-  switch (index) {
-#define CASE(Name, ...) \
-  case k##Name:         \
-    return true;
-#define BUILTIN_LIST_API(V)                                       \
-  BUILTIN_LIST(IGNORE_BUILTIN, V, IGNORE_BUILTIN, IGNORE_BUILTIN, \
-               IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN)
-    BUILTIN_LIST_API(CASE);
-#undef BUILTIN_LIST_API
-#undef CASE
-    default:
-      return false;
+const char* Builtins::KindNameOf(int index) {
+  Kind kind = Builtins::KindOf(index);
+  // clang-format off
+  switch (kind) {
+    case CPP: return "CPP";
+    case API: return "API";
+    case TFJ: return "TFJ";
+    case TFC: return "TFC";
+    case TFS: return "TFS";
+    case TFH: return "TFH";
+    case ASM: return "ASM";
+    case DBG: return "DBG";
   }
+  // clang-format on
   UNREACHABLE();
 }
 
+// static
+bool Builtins::IsCpp(int index) { return Builtins::KindOf(index) == CPP; }
+
 // static
 bool Builtins::HasCppImplementation(int index) {
-  DCHECK(0 <= index && index < builtin_count);
-  switch (index) {
-#define CASE(Name, ...) \
-  case k##Name:         \
-    return true;
-    BUILTIN_LIST_C(CASE)
-#undef CASE
-    default:
-      return false;
-  }
-  UNREACHABLE();
+  Kind kind = Builtins::KindOf(index);
+  return (kind == CPP || kind == API);
 }
 
+// TODO(jgruber): Replace with a CodeFor(name) accessor to emit less code.
 #define DEFINE_BUILTIN_ACCESSOR(Name, ...)                                    \
   Handle<Code> Builtins::Name() {                                             \
     Code** code_address = reinterpret_cast<Code**>(builtin_address(k##Name)); \

src/builtins/builtins.h

@@ -44,6 +44,9 @@ class Builtins {
         builtin_count
   };
 
+  // The different builtin kinds are documented in builtins-definitions.h.
+  enum Kind { CPP, API, TFJ, TFC, TFS, TFH, ASM, DBG };
+
   static BailoutId GetContinuationBailoutId(Name name);
   static Name GetBuiltinFromBailoutId(BailoutId);
 
@@ -77,11 +80,9 @@ class Builtins {
 
   Handle<Code> builtin_handle(Name name);
 
-  static int GetBuiltinParameterCount(Name name);
-
   V8_EXPORT_PRIVATE static Callable CallableFor(Isolate* isolate, Name name);
 
-  static int GetStackParameterCount(Isolate* isolate, Name name);
+  static int GetStackParameterCount(Name name);
 
   static const char* name(int index);
 
@@ -89,8 +90,10 @@ class Builtins {
   // Address otherwise.
   static Address CppEntryOf(int index);
 
+  static Kind KindOf(int index);
+  static const char* KindNameOf(int index);
+
   static bool IsCpp(int index);
-  static bool IsApi(int index);
   static bool HasCppImplementation(int index);
 
   bool is_initialized() const { return initialized_; }

src/compiler/frame-states.cc

@@ -166,10 +166,9 @@ Node* CreateJavaScriptBuiltinContinuationFrameState(
   // the deoptimizer and not explicitly specified in the frame state. Check that
   // there is not a mismatch between the number of frame state parameters and
   // the stack parameters required by the builtin taking this into account.
-  DCHECK_EQ(
-      Builtins::GetStackParameterCount(isolate, name) + 1,  // add receiver
-      stack_parameter_count +
-          (mode == ContinuationFrameStateMode::EAGER ? 0 : 1));
+  DCHECK_EQ(Builtins::GetStackParameterCount(name) + 1,  // add receiver
+            stack_parameter_count +
+                (mode == ContinuationFrameStateMode::EAGER ? 0 : 1));
 
   Node* argc =
       js_graph->Constant(stack_parameter_count -

src/isolate.cc

@@ -2667,10 +2667,11 @@ namespace {
 void PrintBuiltinSizes(Isolate* isolate) {
   Builtins* builtins = isolate->builtins();
   for (int i = 0; i < Builtins::builtin_count; i++) {
-    if (Builtins::IsCpp(i) || Builtins::IsApi(i)) continue;
     const char* name = builtins->name(i);
+    const char* kind = Builtins::KindNameOf(i);
     Code* code = builtins->builtin(static_cast<Builtins::Name>(i));
-    PrintF(stdout, "Builtin, %s, %d\n", name, code->instruction_size());
+    PrintF(stdout, "%s Builtin, %s, %d\n", kind, name,
+           code->instruction_size());
   }
 }
 }  // namespace