Revert "Remove builtin-function-id in SFI"

This reverts commit f8a67670.

Reason for revert: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/25576

I don't think I've seen MSAN being flaky. Chances are that the change to SFI's object layout indeed left some field uninitialized.

Original change's description:
> Remove builtin-function-id in SFI
> 
> 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: Iee9b539475bc6531c9aa65b1904d1402a9ef30db
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1495898
> Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#60017}

TBR=ulan@chromium.org,jgruber@chromium.org,leszeks@chromium.org,bmeurer@chromium.org,duongn@microsoft.com

Change-Id: Ic3964ce182ddbd7ef529ddb8b78b9bdfb1be7887
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6993
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1499500Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60018}

BUILD.gn

@@ -2264,6 +2264,7 @@ 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

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

src/builtins/builtins-collections-gen.cc

@@ -2611,7 +2611,7 @@ TF_BUILTIN(WeakMapGet, WeakCollectionsBuiltinsAssembler) {
   Return(UndefinedConstant());
 }
 
-TF_BUILTIN(WeakMapPrototypeHas, WeakCollectionsBuiltinsAssembler) {
+TF_BUILTIN(WeakMapHas, 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(WeakSetPrototypeHas, WeakCollectionsBuiltinsAssembler) {
+TF_BUILTIN(WeakSetHas, WeakCollectionsBuiltinsAssembler) {
   Node* const receiver = Parameter(Descriptor::kReceiver);
   Node* const key = Parameter(Descriptor::kKey);
   Node* const context = Parameter(Descriptor::kContext);

src/builtins/builtins-definitions.h

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

src/compiler/js-heap-broker.h

@@ -13,6 +13,7 @@
 #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"
@@ -522,7 +523,9 @@ 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::kWeakMapPrototypeHas:
+    case Builtins::kWeakMapHas:
     // 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::kWeakSetPrototypeHas:
+    case Builtins::kWeakSetHas:
     // String builtins. Strings are immutable.
     case Builtins::kStringFromCharCode:
     case Builtins::kStringFromCodePoint:

src/heap/factory.cc

@@ -3664,6 +3664,8 @@ 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 (HasBuiltinId()) {
+  if (HasBuiltinFunctionId()) {
     TraceInlining(*this, "false (is a builtin)");
     return false;
   }

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

@@ -133,6 +133,8 @@ 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, raw_function_token_offset,
                  kFunctionTokenOffsetOffset)
 RELAXED_INT32_ACCESSORS(SharedFunctionInfo, flags, kFlagsOffset)
@@ -706,6 +708,18 @@ 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,6 +8,7 @@
 #include "src/bailout-reason.h"
 #include "src/function-kind.h"
 #include "src/objects.h"
+#include "src/objects/builtin-function-id.h"
 #include "src/objects/script.h"
 #include "src/objects/smi.h"
 #include "src/objects/struct.h"
@@ -353,7 +354,11 @@ class SharedFunctionInfo : public HeapObject {
   inline AsmWasmData asm_wasm_data() const;
   inline void set_asm_wasm_data(AsmWasmData data);
 
-  // builtin_id corresponds to the auto-generated Builtins::Name id.
+  // 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).
   inline bool HasBuiltinId() const;
   inline int builtin_id() const;
   inline void set_builtin_id(int builtin_id);
@@ -373,6 +378,18 @@ 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