[cleanup] Various misc. cleanups

- Use kNoBuiltinId instead of literal -1.
- Remove support for non-embedded builtins.
- Update Code object layout comment.

Bug: v8:10933
Change-Id: Ie75c6ccc0a0f19348ae214249a8fc81f7e91df0c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474115
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70557}

src/builtins/setup-builtins-internal.cc

@@ -38,8 +38,7 @@ AssemblerOptions BuiltinAssemblerOptions(Isolate* isolate,
   CHECK(!options.use_pc_relative_calls_and_jumps);
   CHECK(!options.collect_win64_unwind_info);
 
-  if (!isolate->IsGeneratingEmbeddedBuiltins() ||
-      !Builtins::IsIsolateIndependent(builtin_index)) {
+  if (!isolate->IsGeneratingEmbeddedBuiltins()) {
     return options;
   }
 

src/codegen/arm/macro-assembler-arm.cc

@@ -163,12 +163,10 @@ void TurboAssembler::Jump(Handle<Code> code, RelocInfo::Mode rmode,
                  Builtins::IsIsolateIndependentBuiltin(*code));
 
   int builtin_index = Builtins::kNoBuiltinId;
-  bool target_is_isolate_independent_builtin =
-      isolate()->builtins()->IsBuiltinHandle(code, &builtin_index) &&
-      Builtins::IsIsolateIndependent(builtin_index);
+  bool target_is_builtin =
+      isolate()->builtins()->IsBuiltinHandle(code, &builtin_index);
 
-  if (options().use_pc_relative_calls_and_jumps &&
-      target_is_isolate_independent_builtin) {
+  if (options().use_pc_relative_calls_and_jumps && target_is_builtin) {
     int32_t code_target_index = AddCodeTarget(code);
     b(code_target_index * kInstrSize, cond, RelocInfo::RELATIVE_CODE_TARGET);
     return;
@@ -183,8 +181,7 @@ void TurboAssembler::Jump(Handle<Code> code, RelocInfo::Mode rmode,
     ldr(scratch, MemOperand(kRootRegister, offset));
     Jump(scratch, cond);
     return;
-  } else if (options().inline_offheap_trampolines &&
-             target_is_isolate_independent_builtin) {
+  } else if (options().inline_offheap_trampolines && target_is_builtin) {
     // Inline the trampoline.
     RecordCommentForOffHeapTrampoline(builtin_index);
     EmbeddedData d = EmbeddedData::FromBlob();
@@ -258,12 +255,10 @@ void TurboAssembler::Call(Handle<Code> code, RelocInfo::Mode rmode,
                  Builtins::IsIsolateIndependentBuiltin(*code));
 
   int builtin_index = Builtins::kNoBuiltinId;
-  bool target_is_isolate_independent_builtin =
-      isolate()->builtins()->IsBuiltinHandle(code, &builtin_index) &&
-      Builtins::IsIsolateIndependent(builtin_index);
+  bool target_is_builtin =
+      isolate()->builtins()->IsBuiltinHandle(code, &builtin_index);
 
-  if (target_is_isolate_independent_builtin &&
-      options().use_pc_relative_calls_and_jumps) {
+  if (target_is_builtin && options().use_pc_relative_calls_and_jumps) {
     int32_t code_target_index = AddCodeTarget(code);
     bl(code_target_index * kInstrSize, cond, RelocInfo::RELATIVE_CODE_TARGET);
     return;
@@ -276,8 +271,7 @@ void TurboAssembler::Call(Handle<Code> code, RelocInfo::Mode rmode,
     ldr(ip, MemOperand(kRootRegister, offset));
     Call(ip, cond);
     return;
-  } else if (target_is_isolate_independent_builtin &&
-             options().inline_offheap_trampolines) {
+  } else if (target_is_builtin && options().inline_offheap_trampolines) {
     // Inline the trampoline.
     CallBuiltin(builtin_index);
     return;

src/codegen/arm64/macro-assembler-arm64.cc

@@ -1778,8 +1778,7 @@ void TurboAssembler::Jump(Handle<Code> code, RelocInfo::Mode rmode,
 
   if (options().inline_offheap_trampolines) {
     int builtin_index = Builtins::kNoBuiltinId;
-    if (isolate()->builtins()->IsBuiltinHandle(code, &builtin_index) &&
-        Builtins::IsIsolateIndependent(builtin_index)) {
+    if (isolate()->builtins()->IsBuiltinHandle(code, &builtin_index)) {
       // Inline the trampoline.
       RecordCommentForOffHeapTrampoline(builtin_index);
       CHECK_NE(builtin_index, Builtins::kNoBuiltinId);
@@ -1833,8 +1832,7 @@ void TurboAssembler::Call(Handle<Code> code, RelocInfo::Mode rmode) {
 
   if (options().inline_offheap_trampolines) {
     int builtin_index = Builtins::kNoBuiltinId;
-    if (isolate()->builtins()->IsBuiltinHandle(code, &builtin_index) &&
-        Builtins::IsIsolateIndependent(builtin_index)) {
+    if (isolate()->builtins()->IsBuiltinHandle(code, &builtin_index)) {
       // Inline the trampoline.
       CallBuiltin(builtin_index);
       return;

src/codegen/ia32/macro-assembler-ia32.cc

@@ -1871,8 +1871,7 @@ void TurboAssembler::Call(Handle<Code> code_object, RelocInfo::Mode rmode) {
                  Builtins::IsIsolateIndependentBuiltin(*code_object));
   if (options().inline_offheap_trampolines) {
     int builtin_index = Builtins::kNoBuiltinId;
-    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index) &&
-        Builtins::IsIsolateIndependent(builtin_index)) {
+    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index)) {
       // Inline the trampoline.
       CallBuiltin(builtin_index);
       return;
@@ -1975,8 +1974,7 @@ void TurboAssembler::Jump(Handle<Code> code_object, RelocInfo::Mode rmode) {
                  Builtins::IsIsolateIndependentBuiltin(*code_object));
   if (options().inline_offheap_trampolines) {
     int builtin_index = Builtins::kNoBuiltinId;
-    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index) &&
-        Builtins::IsIsolateIndependent(builtin_index)) {
+    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index)) {
       // Inline the trampoline.
       RecordCommentForOffHeapTrampoline(builtin_index);
       CHECK_NE(builtin_index, Builtins::kNoBuiltinId);

src/codegen/x64/macro-assembler-x64.cc

@@ -1530,8 +1530,7 @@ void TurboAssembler::Jump(Handle<Code> code_object, RelocInfo::Mode rmode,
                  Builtins::IsIsolateIndependentBuiltin(*code_object));
   if (options().inline_offheap_trampolines) {
     int builtin_index = Builtins::kNoBuiltinId;
-    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index) &&
-        Builtins::IsIsolateIndependent(builtin_index)) {
+    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index)) {
       Label skip;
       if (cc != always) {
         if (cc == never) return;
@@ -1580,8 +1579,7 @@ void TurboAssembler::Call(Handle<Code> code_object, RelocInfo::Mode rmode) {
                  Builtins::IsIsolateIndependentBuiltin(*code_object));
   if (options().inline_offheap_trampolines) {
     int builtin_index = Builtins::kNoBuiltinId;
-    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index) &&
-        Builtins::IsIsolateIndependent(builtin_index)) {
+    if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index)) {
       // Inline the trampoline.
       CallBuiltin(builtin_index);
       return;

src/execution/isolate.cc

@@ -3295,9 +3295,8 @@ void CreateOffHeapTrampolines(Isolate* isolate) {
 
   EmbeddedData d = EmbeddedData::FromBlob();
 
+  STATIC_ASSERT(Builtins::kAllBuiltinsAreIsolateIndependent);
   for (int i = 0; i < Builtins::builtin_count; i++) {
-    if (!Builtins::IsIsolateIndependent(i)) continue;
-
     Address instruction_start = d.InstructionStartOfBuiltin(i);
     Handle<Code> trampoline = isolate->factory()->NewOffHeapTrampolineFor(
         builtins->builtin_handle(i), instruction_start);

src/objects/code-inl.h

@@ -440,16 +440,18 @@ inline HandlerTable::CatchPrediction Code::GetBuiltinCatchPrediction() {
 
 int Code::builtin_index() const {
   int index = ReadField<int>(kBuiltinIndexOffset);
-  DCHECK(index == -1 || Builtins::IsBuiltinId(index));
+  DCHECK(index == Builtins::kNoBuiltinId || Builtins::IsBuiltinId(index));
   return index;
 }
 
 void Code::set_builtin_index(int index) {
-  DCHECK(index == -1 || Builtins::IsBuiltinId(index));
+  DCHECK(index == Builtins::kNoBuiltinId || Builtins::IsBuiltinId(index));
   WriteField<int>(kBuiltinIndexOffset, index);
 }
 
-bool Code::is_builtin() const { return builtin_index() != -1; }
+bool Code::is_builtin() const {
+  return builtin_index() != Builtins::kNoBuiltinId;
+}
 
 unsigned Code::inlined_bytecode_size() const {
   DCHECK(CodeKindIsOptimizedJSFunction(kind()) ||

src/objects/code.h

@@ -153,8 +153,8 @@ class Code : public HeapObject {
   inline void set_can_have_weak_objects(bool value);
 
   // [builtin_index]: For builtins, tells which builtin index the code object
-  // has. The builtin index is a non-negative integer for builtins, and -1
-  // otherwise.
+  // has. The builtin index is a non-negative integer for builtins, and
+  // Builtins::kNoBuiltinId (-1) otherwise.
   inline int builtin_index() const;
   inline void set_builtin_index(int id);
   inline bool is_builtin() const;
@@ -303,9 +303,6 @@ class Code : public HeapObject {
   // Returns the address right after the relocation info (read backwards!).
   inline byte* relocation_end() const;
 
-  // [has_unwinding_info]: Whether this code object has unwinding information.
-  // If it doesn't, unwinding_information_start() will point to invalid data.
-  //
   // The body of all code objects has the following layout.
   //
   //  +--------------------------+  <-- raw_instruction_start()
@@ -336,7 +333,9 @@ class Code : public HeapObject {
   //
   // and unwinding_info_end() points to the first memory location after the end
   // of the code object.
-  //
+
+  // [has_unwinding_info]: Whether this code object has unwinding information.
+  // If it doesn't, unwinding_information_start() will point to invalid data.
   inline bool has_unwinding_info() const;
 
   // [unwinding_info_size]: Size of the unwinding information.

src/regexp/arm/regexp-macro-assembler-arm.cc

@@ -1084,7 +1084,6 @@ void RegExpMacroAssemblerARM::CallCheckStackGuardState() {
   __ mov(ip, Operand(stack_guard_check));
 
   EmbeddedData d = EmbeddedData::FromBlob();
-  CHECK(Builtins::IsIsolateIndependent(Builtins::kDirectCEntry));
   Address entry = d.InstructionStartOfBuiltin(Builtins::kDirectCEntry);
   __ mov(lr, Operand(entry, RelocInfo::OFF_HEAP_TARGET));
   __ Call(lr);

src/regexp/arm64/regexp-macro-assembler-arm64.cc

@@ -1411,7 +1411,6 @@ void RegExpMacroAssemblerARM64::CallCheckStackGuardState(Register scratch) {
     Register scratch = temps.AcquireX();
 
     EmbeddedData d = EmbeddedData::FromBlob();
-    CHECK(Builtins::IsIsolateIndependent(Builtins::kDirectCEntry));
     Address entry = d.InstructionStartOfBuiltin(Builtins::kDirectCEntry);
 
     __ Ldr(scratch, Operand(entry, RelocInfo::OFF_HEAP_TARGET));

src/snapshot/embedded/embedded-data.cc

@@ -157,9 +157,8 @@ void FinalizeEmbeddedCodeTargets(Isolate* isolate, EmbeddedData* blob) {
       RelocInfo::ModeMask(RelocInfo::CODE_TARGET) |
       RelocInfo::ModeMask(RelocInfo::RELATIVE_CODE_TARGET);
 
+  STATIC_ASSERT(Builtins::kAllBuiltinsAreIsolateIndependent);
   for (int i = 0; i < Builtins::builtin_count; i++) {
-    if (!Builtins::IsIsolateIndependent(i)) continue;
-
     Code code = isolate->builtins()->builtin(i);
     RelocIterator on_heap_it(code, kRelocMask);
     RelocIterator off_heap_it(blob, code, kRelocMask);
@@ -209,33 +208,30 @@ EmbeddedData EmbeddedData::FromIsolate(Isolate* isolate) {
 
   bool saw_unsafe_builtin = false;
   uint32_t raw_code_size = 0;
+  STATIC_ASSERT(Builtins::kAllBuiltinsAreIsolateIndependent);
   for (int i = 0; i < Builtins::builtin_count; i++) {
     Code code = builtins->builtin(i);
 
-    if (Builtins::IsIsolateIndependent(i)) {
-      // Sanity-check that the given builtin is isolate-independent and does not
-      // use the trampoline register in its calling convention.
-      if (!code.IsIsolateIndependent(isolate)) {
-        saw_unsafe_builtin = true;
-        fprintf(stderr, "%s is not isolate-independent.\n", Builtins::name(i));
-      }
-      if (BuiltinAliasesOffHeapTrampolineRegister(isolate, code)) {
-        saw_unsafe_builtin = true;
-        fprintf(stderr, "%s aliases the off-heap trampoline register.\n",
-                Builtins::name(i));
-      }
-
-      uint32_t length = static_cast<uint32_t>(code.raw_instruction_size());
-
-      DCHECK_EQ(0, raw_code_size % kCodeAlignment);
-      metadata[i].instructions_offset = raw_code_size;
-      metadata[i].instructions_length = length;
-
-      // Align the start of each instruction stream.
-      raw_code_size += PadAndAlign(length);
-    } else {
-      metadata[i].instructions_offset = raw_code_size;
+    // Sanity-check that the given builtin is isolate-independent and does not
+    // use the trampoline register in its calling convention.
+    if (!code.IsIsolateIndependent(isolate)) {
+      saw_unsafe_builtin = true;
+      fprintf(stderr, "%s is not isolate-independent.\n", Builtins::name(i));
+    }
+    if (BuiltinAliasesOffHeapTrampolineRegister(isolate, code)) {
+      saw_unsafe_builtin = true;
+      fprintf(stderr, "%s aliases the off-heap trampoline register.\n",
+              Builtins::name(i));
     }
+
+    uint32_t length = static_cast<uint32_t>(code.raw_instruction_size());
+
+    DCHECK_EQ(0, raw_code_size % kCodeAlignment);
+    metadata[i].instructions_offset = raw_code_size;
+    metadata[i].instructions_length = length;
+
+    // Align the start of each instruction stream.
+    raw_code_size += PadAndAlign(length);
   }
   CHECK_WITH_MSG(
       !saw_unsafe_builtin,
@@ -267,8 +263,8 @@ EmbeddedData EmbeddedData::FromIsolate(Isolate* isolate) {
               MetadataTableSize());
 
   // Write the raw data section.
+  STATIC_ASSERT(Builtins::kAllBuiltinsAreIsolateIndependent);
   for (int i = 0; i < Builtins::builtin_count; i++) {
-    if (!Builtins::IsIsolateIndependent(i)) continue;
     Code code = builtins->builtin(i);
     uint32_t offset = metadata[i].instructions_offset;
     uint8_t* dst = raw_code_start + offset;
@@ -346,8 +342,8 @@ void EmbeddedData::PrintStatistics() const {
   int embedded_count = 0;
   int instruction_size = 0;
   int sizes[kCount];
+  STATIC_ASSERT(Builtins::kAllBuiltinsAreIsolateIndependent);
   for (int i = 0; i < kCount; i++) {
-    if (!Builtins::IsIsolateIndependent(i)) continue;
     const int size = InstructionSizeOfBuiltin(i);
     instruction_size += size;
     sizes[embedded_count] = size;