Reland Remove builtin-function-id in SFI

This is the reland of https://chromium-review.googlesource.com/c/v8/v8/+/1495898

builtin_function_id corresponded to BuiltinFunctionId (a manually maintained list of 'interesting' functionsmainly used during optimization). With this change, we nuke builtin-function-id in favor of builtin-id and 8 bits is freed up in SFI.

Bug: v8:6993
Change-Id: I7e1681cc2a95864c71ce8bdda075481310607166
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1506445Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#60131}

BUILD.gn

@@ -2278,7 +2278,6 @@ v8_source_set("v8_base") {
     "src/objects/arguments.h",
     "src/objects/bigint.cc",
     "src/objects/bigint.h",
-    "src/objects/builtin-function-id.h",
     "src/objects/cell-inl.h",
     "src/objects/cell.h",
     "src/objects/code-inl.h",

src/builtins/base.tq

@@ -184,8 +184,8 @@ extern class SharedFunctionInfo extends HeapObject {
   script_or_debug_info: Script | DebugInfo;
   length: int16;
   formal_parameter_count: uint16;
-  expected_nof_properties: int8;
-  builtin_function_id: int8;
+  // Currently set to uint16, can be set to uint8 to save space.
+  expected_nof_properties: uint16;
   function_token_offset: int16;
   flags: int32;
 }

src/builtins/builtins-collections-gen.cc

@@ -2611,7 +2611,7 @@ TF_BUILTIN(WeakMapGet, WeakCollectionsBuiltinsAssembler) {
   Return(UndefinedConstant());
 }
 
-TF_BUILTIN(WeakMapHas, WeakCollectionsBuiltinsAssembler) {
+TF_BUILTIN(WeakMapPrototypeHas, WeakCollectionsBuiltinsAssembler) {
   Node* const receiver = Parameter(Descriptor::kReceiver);
   Node* const key = Parameter(Descriptor::kKey);
   Node* const context = Parameter(Descriptor::kContext);
@@ -2775,7 +2775,7 @@ TF_BUILTIN(WeakSetPrototypeDelete, CodeStubAssembler) {
       CallBuiltin(Builtins::kWeakCollectionDelete, context, receiver, value));
 }
 
-TF_BUILTIN(WeakSetHas, WeakCollectionsBuiltinsAssembler) {
+TF_BUILTIN(WeakSetPrototypeHas, WeakCollectionsBuiltinsAssembler) {
   Node* const receiver = Parameter(Descriptor::kReceiver);
   Node* const key = Parameter(Descriptor::kKey);
   Node* const context = Parameter(Descriptor::kContext);

src/builtins/builtins-definitions.h

@@ -1171,13 +1171,13 @@ namespace internal {
   TFJ(WeakMapConstructor, SharedFunctionInfo::kDontAdaptArgumentsSentinel)     \
   TFS(WeakMapLookupHashIndex, kTable, kKey)                                    \
   TFJ(WeakMapGet, 1, kReceiver, kKey)                                          \
-  TFJ(WeakMapHas, 1, kReceiver, kKey)                                          \
+  TFJ(WeakMapPrototypeHas, 1, kReceiver, kKey)                                 \
   TFJ(WeakMapPrototypeSet, 2, kReceiver, kKey, kValue)                         \
   TFJ(WeakMapPrototypeDelete, 1, kReceiver, kKey)                              \
                                                                                \
   /* WeakSet */                                                                \
   TFJ(WeakSetConstructor, SharedFunctionInfo::kDontAdaptArgumentsSentinel)     \
-  TFJ(WeakSetHas, 1, kReceiver, kKey)                                          \
+  TFJ(WeakSetPrototypeHas, 1, kReceiver, kKey)                                 \
   TFJ(WeakSetPrototypeAdd, 1, kReceiver, kValue)                               \
   TFJ(WeakSetPrototypeDelete, 1, kReceiver, kValue)                            \
                                                                                \

src/compiler/js-heap-broker.h

@@ -13,7 +13,6 @@
 #include "src/globals.h"
 #include "src/handles.h"
 #include "src/objects.h"
-#include "src/objects/builtin-function-id.h"
 #include "src/objects/instance-type.h"
 #include "src/ostreams.h"
 #include "src/zone/zone-containers.h"
@@ -529,9 +528,7 @@ class ScopeInfoRef : public HeapObjectRef {
   V(LanguageMode, language_mode)            \
   V(bool, native)                           \
   V(bool, HasBreakInfo)                     \
-  V(bool, HasBuiltinFunctionId)             \
   V(bool, HasBuiltinId)                     \
-  V(BuiltinFunctionId, builtin_function_id) \
   V(bool, construct_as_builtin)             \
   V(bool, HasBytecodeArray)                 \
   V(bool, is_safe_to_skip_arguments_adaptor)

src/debug/debug-evaluate.cc

@@ -625,7 +625,7 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) {
     // WeakMap builtins.
     case Builtins::kWeakMapConstructor:
     case Builtins::kWeakMapGet:
-    case Builtins::kWeakMapHas:
+    case Builtins::kWeakMapPrototypeHas:
     // Math builtins.
     case Builtins::kMathAbs:
     case Builtins::kMathAcos:
@@ -690,7 +690,7 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) {
     case Builtins::kSetPrototypeValues:
     // WeakSet builtins.
     case Builtins::kWeakSetConstructor:
-    case Builtins::kWeakSetHas:
+    case Builtins::kWeakSetPrototypeHas:
     // String builtins. Strings are immutable.
     case Builtins::kStringFromCharCode:
     case Builtins::kStringFromCodePoint:

src/heap/factory.cc

@@ -3661,8 +3661,6 @@ Handle<SharedFunctionInfo> Factory::NewSharedFunctionInfo(
     share->set_length(0);
     share->set_internal_formal_parameter_count(0);
     share->set_expected_nof_properties(0);
-    share->set_builtin_function_id(
-        BuiltinFunctionId::kInvalidBuiltinFunctionId);
     share->set_raw_function_token_offset(0);
     // All flags default to false or 0.
     share->set_flags(0);

src/objects.cc

@@ -5228,7 +5228,7 @@ bool SharedFunctionInfo::IsInlineable() {
   }
 
   // Built-in functions are handled by the JSCallReducer.
-  if (HasBuiltinFunctionId()) {
+  if (HasBuiltinId()) {
     TraceInlining(*this, "false (is a builtin)");
     return false;
   }

src/objects/shared-function-info-inl.h

@@ -131,10 +131,8 @@ ACCESSORS(SharedFunctionInfo, script_or_debug_info, Object,
 UINT16_ACCESSORS(SharedFunctionInfo, length, kLengthOffset)
 UINT16_ACCESSORS(SharedFunctionInfo, internal_formal_parameter_count,
                  kFormalParameterCountOffset)
-UINT8_ACCESSORS(SharedFunctionInfo, expected_nof_properties,
-                kExpectedNofPropertiesOffset)
-UINT8_ACCESSORS(SharedFunctionInfo, raw_builtin_function_id,
-                kBuiltinFunctionIdOffset)
+UINT16_ACCESSORS(SharedFunctionInfo, expected_nof_properties,
+                 kExpectedNofPropertiesOffset)
 UINT16_ACCESSORS(SharedFunctionInfo, raw_function_token_offset,
                  kFunctionTokenOffsetOffset)
 RELAXED_INT32_ACCESSORS(SharedFunctionInfo, flags, kFlagsOffset)
@@ -708,18 +706,6 @@ void SharedFunctionInfo::SetDebugInfo(DebugInfo debug_info) {
   set_script_or_debug_info(debug_info);
 }
 
-bool SharedFunctionInfo::HasBuiltinFunctionId() {
-  return builtin_function_id() != BuiltinFunctionId::kInvalidBuiltinFunctionId;
-}
-
-BuiltinFunctionId SharedFunctionInfo::builtin_function_id() {
-  return static_cast<BuiltinFunctionId>(raw_builtin_function_id());
-}
-
-void SharedFunctionInfo::set_builtin_function_id(BuiltinFunctionId id) {
-  set_raw_builtin_function_id(static_cast<uint8_t>(id));
-}
-
 bool SharedFunctionInfo::HasInferredName() {
   Object scope_info = name_or_scope_info();
   if (scope_info->IsScopeInfo()) {

src/objects/shared-function-info.h

@@ -8,7 +8,6 @@
 #include "src/bailout-reason.h"
 #include "src/function-kind.h"
 #include "src/objects.h"
-#include "src/objects/builtin-function-id.h"
 #include "src/objects/compressed-slots.h"
 #include "src/objects/script.h"
 #include "src/objects/slots.h"
@@ -324,7 +323,7 @@ class SharedFunctionInfo : public HeapObject {
 
   // [expected_nof_properties]: Expected number of properties for the
   // function. The value is only reliable when the function has been compiled.
-  DECL_UINT8_ACCESSORS(expected_nof_properties)
+  DECL_UINT16_ACCESSORS(expected_nof_properties)
 
   // [function data]: This field holds some additional data for function.
   // Currently it has one of:
@@ -357,11 +356,7 @@ class SharedFunctionInfo : public HeapObject {
   inline AsmWasmData asm_wasm_data() const;
   inline void set_asm_wasm_data(AsmWasmData data);
 
-  // A brief note to clear up possible confusion:
-  // builtin_id corresponds to the auto-generated
-  // Builtins::Name id, while builtin_function_id corresponds to
-  // BuiltinFunctionId (a manually maintained list of 'interesting' functions
-  // mainly used during optimization).
+  // builtin_id corresponds to the auto-generated Builtins::Name id.
   inline bool HasBuiltinId() const;
   inline int builtin_id() const;
   inline void set_builtin_id(int builtin_id);
@@ -381,18 +376,6 @@ class SharedFunctionInfo : public HeapObject {
   // turning it into UncompiledDataWithoutPreparseData.
   inline void ClearPreparseData();
 
-  // [raw_builtin_function_id]: The id of the built-in function this function
-  // represents, used during optimization to improve code generation.
-  // TODO(leszeks): Once there are no more JS builtins, this can be replaced
-  // by BuiltinId.
-  DECL_UINT8_ACCESSORS(raw_builtin_function_id)
-  inline bool HasBuiltinFunctionId();
-  inline BuiltinFunctionId builtin_function_id();
-  inline void set_builtin_function_id(BuiltinFunctionId id);
-  // Make sure BuiltinFunctionIds fit in a uint8_t
-  STATIC_ASSERT((std::is_same<std::underlying_type<BuiltinFunctionId>::type,
-                              uint8_t>::value));
-
   // The inferred_name is inferred from variable or property assignment of this
   // function. It is used to facilitate debugging and profiling of JavaScript
   // code written in OO style, where almost all functions are anonymous but are