[builtins][masm] Move hot flags to the beginning of IsolateData

... so that the offset fits into the maximum offset for load byte
instruction for arm/arm64 (Ldrb) in order to produce smaller code.

Update code generation so that the loading of the flag value is
combined with the comparison operation where possible.

Additionally, this CL moves the Isolate::is_profiling flag to the
IsolateData so that it can be loaded directly via roots register which
removes one indirection.

The fields moved in the IsolateData:
 - is_marking_flag and is_minor_marking_flag (checked by write barriers)
 - is_profiling (checked on API callbacks/getter calls)
 - stack_is_iterable (not super hot, checked during deoptimization).

Drive-by: this CL defines the bool fields as uint8_t in order to make
the field size expectations clear.

Bug: v8:11880
Change-Id: I80c292c6ec919861684152b6062225aa0fda2d3e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3856580Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82771}

include/v8-internal.h

@@ -541,8 +541,10 @@ class Internals {
   static const int kIsolateCageBaseOffset = 0;
   static const int kIsolateStackGuardOffset =
       kIsolateCageBaseOffset + kApiSystemPointerSize;
-  static const int kBuiltinTier0EntryTableOffset =
+  static const int kVariousBooleanFlagsOffset =
       kIsolateStackGuardOffset + kStackGuardSize;
+  static const int kBuiltinTier0EntryTableOffset =
+      kVariousBooleanFlagsOffset + kApiSystemPointerSize;
   static const int kBuiltinTier0TableOffset =
       kBuiltinTier0EntryTableOffset + kBuiltinTier0EntryTableSize;
   static const int kIsolateEmbedderDataOffset =

src/base/macros.h

@@ -430,6 +430,15 @@ bool is_inbounds(float_t v) {
 #define IF_TSAN(V, ...)
 #endif  // V8_ENABLE_WEBASSEMBLY
 
+// Defines IF_TARGET_ARCH_64_BIT, to be used in macro lists for elements that
+// should only be there if the target architecture is a 64-bit one.
+#if V8_TARGET_ARCH_64_BIT
+// EXPAND is needed to work around MSVC's broken __VA_ARGS__ expansion.
+#define IF_TARGET_ARCH_64_BIT(V, ...) EXPAND(V(__VA_ARGS__))
+#else
+#define IF_TARGET_ARCH_64_BIT(V, ...)
+#endif
+
 #ifdef GOOGLE3
 // Disable FRIEND_TEST macro in Google3.
 #define FRIEND_TEST(test_case_name, test_name)

src/builtins/arm/builtins-arm.cc

@@ -2925,8 +2925,8 @@ void CallApiFunctionAndReturn(MacroAssembler* masm, Register function_address,
   __ str(r6, MemOperand(r9, kLevelOffset));
 
   Label profiler_enabled, done_api_call;
-  __ Move(r8, ExternalReference::is_profiling_address(isolate));
-  __ ldrb(r8, MemOperand(r8, 0));
+  __ ldrb(r8, __ ExternalReferenceAsOperand(
+                  ExternalReference::is_profiling_address(isolate), r8));
   __ cmp(r8, Operand(0));
   __ b(ne, &profiler_enabled);
 #ifdef V8_RUNTIME_CALL_STATS

src/builtins/arm64/builtins-arm64.cc

@@ -3376,8 +3376,8 @@ void CallApiFunctionAndReturn(MacroAssembler* masm, Register function_address,
   __ Str(level_reg, MemOperand(handle_scope_base, kLevelOffset));
 
   Label profiler_enabled, done_api_call;
-  __ Mov(x10, ExternalReference::is_profiling_address(isolate));
-  __ Ldrb(w10, MemOperand(x10));
+  __ Ldrb(w10, __ ExternalReferenceAsOperand(
+                   ExternalReference::is_profiling_address(isolate), x10));
   __ Cbnz(w10, &profiler_enabled);
 #ifdef V8_RUNTIME_CALL_STATS
   __ Mov(x10, ExternalReference::address_of_runtime_stats_flag());

src/builtins/ia32/builtins-ia32.cc

@@ -3164,8 +3164,9 @@ void CallApiFunctionAndReturn(MacroAssembler* masm, Register function_address,
   __ mov(edi, __ ExternalReferenceAsOperand(limit_address, edi));
 
   Label profiler_enabled, done_api_call;
-  __ Move(eax, Immediate(ExternalReference::is_profiling_address(isolate)));
-  __ cmpb(Operand(eax, 0), Immediate(0));
+  __ cmpb(__ ExternalReferenceAsOperand(
+              ExternalReference::is_profiling_address(isolate), eax),
+          Immediate(0));
   __ j(not_zero, &profiler_enabled);
 #ifdef V8_RUNTIME_CALL_STATS
   __ Move(eax, Immediate(ExternalReference::address_of_runtime_stats_flag()));

src/builtins/x64/builtins-x64.cc

@@ -4445,8 +4445,9 @@ void CallApiFunctionAndReturn(MacroAssembler* masm, Register function_address,
   __ addl(Operand(base_reg, kLevelOffset), Immediate(1));
 
   Label profiler_enabled, done_api_call;
-  __ Move(rax, ExternalReference::is_profiling_address(isolate));
-  __ cmpb(Operand(rax, 0), Immediate(0));
+  __ cmpb(__ ExternalReferenceAsOperand(
+              ExternalReference::is_profiling_address(isolate), rax),
+          Immediate(0));
   __ j(not_zero, &profiler_enabled);
 #ifdef V8_RUNTIME_CALL_STATS
   __ Move(rax, ExternalReference::address_of_runtime_stats_flag());

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

@@ -119,6 +119,35 @@ void TurboAssembler::LoadRootRegisterOffset(Register destination,
   }
 }
 
+MemOperand TurboAssembler::ExternalReferenceAsOperand(
+    ExternalReference reference, Register scratch) {
+  if (root_array_available_ && options().enable_root_relative_access) {
+    int64_t offset =
+        RootRegisterOffsetForExternalReference(isolate(), reference);
+    if (is_int32(offset)) {
+      return MemOperand(kRootRegister, static_cast<int32_t>(offset));
+    }
+  }
+  if (root_array_available_ && options().isolate_independent_code) {
+    if (IsAddressableThroughRootRegister(isolate(), reference)) {
+      // Some external references can be efficiently loaded as an offset from
+      // kRootRegister.
+      intptr_t offset =
+          RootRegisterOffsetForExternalReference(isolate(), reference);
+      CHECK(is_int32(offset));
+      return MemOperand(kRootRegister, static_cast<int32_t>(offset));
+    } else {
+      // Otherwise, do a memory load from the external reference table.
+      ldr(scratch, MemOperand(kRootRegister,
+                              RootRegisterOffsetForExternalReferenceTableEntry(
+                                  isolate(), reference)));
+      return MemOperand(scratch, 0);
+    }
+  }
+  Move(scratch, reference);
+  return MemOperand(scratch, 0);
+}
+
 void TurboAssembler::Jump(Register target, Condition cond) { bx(target, cond); }
 
 void TurboAssembler::Jump(intptr_t target, RelocInfo::Mode rmode,

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

@@ -294,6 +294,15 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
   void LoadRootRegisterOffset(Register destination, intptr_t offset) final;
   void LoadRootRelative(Register destination, int32_t offset) final;
 
+  // Operand pointing to an external reference.
+  // May emit code to set up the scratch register. The operand is
+  // only guaranteed to be correct as long as the scratch register
+  // isn't changed.
+  // If the operand is used more than once, use a scratch register
+  // that is guaranteed not to be clobbered.
+  MemOperand ExternalReferenceAsOperand(ExternalReference reference,
+                                        Register scratch);
+
   // Jump, Call, and Ret pseudo instructions implementing inter-working.
   void Call(Register target, Condition cond = al);
   void Call(Address target, RelocInfo::Mode rmode, Condition cond = al,

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

@@ -1976,6 +1976,35 @@ void TurboAssembler::LoadRootRegisterOffset(Register destination,
   }
 }
 
+MemOperand TurboAssembler::ExternalReferenceAsOperand(
+    ExternalReference reference, Register scratch) {
+  if (root_array_available_ && options().enable_root_relative_access) {
+    int64_t offset =
+        RootRegisterOffsetForExternalReference(isolate(), reference);
+    if (is_int32(offset)) {
+      return MemOperand(kRootRegister, static_cast<int32_t>(offset));
+    }
+  }
+  if (root_array_available_ && options().isolate_independent_code) {
+    if (IsAddressableThroughRootRegister(isolate(), reference)) {
+      // Some external references can be efficiently loaded as an offset from
+      // kRootRegister.
+      intptr_t offset =
+          RootRegisterOffsetForExternalReference(isolate(), reference);
+      CHECK(is_int32(offset));
+      return MemOperand(kRootRegister, static_cast<int32_t>(offset));
+    } else {
+      // Otherwise, do a memory load from the external reference table.
+      Ldr(scratch, MemOperand(kRootRegister,
+                              RootRegisterOffsetForExternalReferenceTableEntry(
+                                  isolate(), reference)));
+      return MemOperand(scratch, 0);
+    }
+  }
+  Mov(scratch, reference);
+  return MemOperand(scratch, 0);
+}
+
 void TurboAssembler::Jump(Register target, Condition cond) {
   if (cond == nv) return;
   Label done;

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

@@ -942,6 +942,15 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
   void LoadRootRegisterOffset(Register destination, intptr_t offset) final;
   void LoadRootRelative(Register destination, int32_t offset) final;
 
+  // Operand pointing to an external reference.
+  // May emit code to set up the scratch register. The operand is
+  // only guaranteed to be correct as long as the scratch register
+  // isn't changed.
+  // If the operand is used more than once, use a scratch register
+  // that is guaranteed not to be clobbered.
+  MemOperand ExternalReferenceAsOperand(ExternalReference reference,
+                                        Register scratch);
+
   void Jump(Register target, Condition cond = al);
   void Jump(Address target, RelocInfo::Mode rmode, Condition cond = al);
   void Jump(Handle<CodeT> code, RelocInfo::Mode rmode, Condition cond = al);

src/codegen/external-reference.cc

@@ -738,10 +738,6 @@ ExternalReference ExternalReference::thread_in_wasm_flag_address_address(
   return ExternalReference(isolate->thread_in_wasm_flag_address_address());
 }
 
-ExternalReference ExternalReference::is_profiling_address(Isolate* isolate) {
-  return ExternalReference(isolate->is_profiling_address());
-}
-
 ExternalReference ExternalReference::invoke_function_callback() {
   Address thunk_address = FUNCTION_ADDR(&InvokeFunctionCallback);
   ExternalReference::Type thunk_type = ExternalReference::PROFILING_API_CALL;
@@ -1186,6 +1182,10 @@ ExternalReference ExternalReference::stack_is_iterable_address(
       isolate->isolate_data()->stack_is_iterable_address());
 }
 
+ExternalReference ExternalReference::is_profiling_address(Isolate* isolate) {
+  return ExternalReference(isolate->isolate_data()->is_profiling_address());
+}
+
 FUNCTION_REFERENCE(call_enqueue_microtask_function,
                    MicrotaskQueue::CallEnqueueMicrotask)
 

src/codegen/external-reference.h

@@ -61,7 +61,7 @@ class StatsCounter;
     "Debug::hook_on_function_call_address()")                                  \
   V(runtime_function_table_address,                                            \
     "Runtime::runtime_function_table_address()")                               \
-  V(is_profiling_address, "Isolate::is_profiling")                             \
+  V(is_profiling_address, "IsolateData::is_profiling")                         \
   V(debug_suspended_generator_address,                                         \
     "Debug::step_suspended_generator_address()")                               \
   V(fast_c_call_caller_fp_address,                                             \

src/execution/isolate-data.h

@@ -26,6 +26,12 @@ class Isolate;
   /* Misc. fields. */                                                         \
   V(kCageBaseOffset, kSystemPointerSize, cage_base)                           \
   V(kStackGuardOffset, StackGuard::kSizeInBytes, stack_guard)                 \
+  V(kIsMarkingFlag, kUInt8Size, is_marking_flag)                              \
+  V(kIsMinorMarkingFlag, kUInt8Size, is_minor_marking_flag)                   \
+  V(kIsProfilingOffset, kUInt8Size, is_profiling)                             \
+  V(kStackIsIterableOffset, kUInt8Size, stack_is_iterable)                    \
+  IF_TARGET_ARCH_64_BIT(V, kTablesAlignmentPaddingOffset,                     \
+                        kSystemPointerSize - 4, tables_alignment_padding)     \
   /* Tier 0 tables (small but fast access). */                                \
   V(kBuiltinTier0EntryTableOffset,                                            \
     Builtins::kBuiltinTier0Count* kSystemPointerSize,                         \
@@ -52,10 +58,7 @@ class Isolate;
     builtin_table)                                                            \
   /* Linear allocation areas for the heap's new and old space */              \
   V(kNewAllocationInfo, LinearAllocationArea::kSize, new_allocation_info)     \
-  V(kOldAllocationInfo, LinearAllocationArea::kSize, old_allocation_info)     \
-  V(kStackIsIterableOffset, kUInt8Size, stack_is_iterable)                    \
-  V(kIsMarkingFlag, kUInt8Size, is_marking_flag)                              \
-  V(kIsMinorMarkingFlag, kUInt8Size, is_minor_marking_flag)
+  V(kOldAllocationInfo, LinearAllocationArea::kSize, old_allocation_info)
 
 #ifdef V8_COMPRESS_POINTERS
 #define ISOLATE_DATA_FIELDS_POINTER_COMPRESSION(V)            \
@@ -171,6 +174,37 @@ class IsolateData final {
   // the stack limit used by stack checks in generated code.
   StackGuard stack_guard_;
 
+  //
+  // Hot flags that are regularily checked.
+  //
+
+  // These flags are regularily checked by write barriers.
+  // Only valid values are 0 or 1.
+  uint8_t is_marking_flag_ = false;
+  uint8_t is_minor_marking_flag_ = false;
+
+  // true if the Isolate is being profiled. Causes collection of extra compile
+  // info.
+  // This flag is checked on every API callback/getter call.
+  // Only valid values are 0 or 1.
+  std::atomic<uint8_t> is_profiling_{false};
+
+  //
+  // Not super hot flags, which are put here because we have to align the
+  // builtin entry table to kSystemPointerSize anyway.
+  //
+
+  // Whether the SafeStackFrameIterator can successfully iterate the current
+  // stack. Only valid values are 0 or 1.
+  uint8_t stack_is_iterable_ = 1;
+
+#if V8_TARGET_ARCH_64_BIT
+  // Ensure the following tables are kSystemPointerSize-byte aligned.
+  // 32-bit architectures currently don't require the alignment.
+  static_assert(FIELD_SIZE(kTablesAlignmentPaddingOffset) > 0);
+  uint8_t tables_alignment_padding_[FIELD_SIZE(kTablesAlignmentPaddingOffset)];
+#endif  // V8_TARGET_ARCH_64_BIT
+
   // Tier 0 tables. See also builtin_entry_table_ and builtin_table_.
   Address builtin_tier0_entry_table_[Builtins::kBuiltinTier0Count] = {};
   Address builtin_tier0_table_[Builtins::kBuiltinTier0Count] = {};
@@ -219,13 +253,6 @@ class IsolateData final {
   LinearAllocationArea new_allocation_info_;
   LinearAllocationArea old_allocation_info_;
 
-  // Whether the SafeStackFrameIterator can successfully iterate the current
-  // stack. Only valid values are 0 or 1.
-  uint8_t stack_is_iterable_ = 1;
-
-  bool is_marking_flag_ = false;
-  bool is_minor_marking_flag_ = false;
-
   // Ensure the size is 8-byte aligned in order to make alignment of the field
   // following the IsolateData field predictable. This solves the issue with
   // C++ compilers for 32-bit platforms which are not consistent at aligning

src/execution/isolate.cc

@@ -3477,6 +3477,16 @@ void Isolate::CheckIsolateLayout() {
 #endif
 
   CHECK_EQ(OFFSET_OF(Isolate, isolate_data_), 0);
+  CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.stack_guard_)),
+           Internals::kIsolateStackGuardOffset);
+  CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.is_marking_flag_)),
+           Internals::kVariousBooleanFlagsOffset);
+  CHECK_EQ(static_cast<int>(
+               OFFSET_OF(Isolate, isolate_data_.builtin_tier0_entry_table_)),
+           Internals::kBuiltinTier0EntryTableOffset);
+  CHECK_EQ(
+      static_cast<int>(OFFSET_OF(Isolate, isolate_data_.builtin_tier0_table_)),
+      Internals::kBuiltinTier0TableOffset);
   CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.embedder_data_)),
            Internals::kIsolateEmbedderDataOffset);
   CHECK_EQ(static_cast<int>(

src/execution/isolate.h

@@ -1339,17 +1339,15 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   Debug* debug() const { return debug_; }
 
-  void* is_profiling_address() { return &is_profiling_; }
-
   bool is_profiling() const {
-    return is_profiling_.load(std::memory_order_relaxed);
+    return isolate_data_.is_profiling_.load(std::memory_order_relaxed);
   }
 
   void SetIsProfiling(bool enabled) {
     if (enabled) {
       CollectSourcePositionsForAllBytecodeArrays();
     }
-    is_profiling_.store(enabled, std::memory_order_relaxed);
+    isolate_data_.is_profiling_.store(enabled, std::memory_order_relaxed);
     UpdateLogObjectRelocation();
   }
 
@@ -2257,9 +2255,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   ICUObjectCacheEntry icu_object_cache_[kICUObjectCacheTypeCount];
 #endif  // V8_INTL_SUPPORT
 
-  // true if being profiled. Causes collection of extra compile info.
-  std::atomic<bool> is_profiling_{false};
-
   // Whether the isolate has been created for snapshotting.
   bool serializer_enabled_ = false;
 

src/heap/heap.cc

@@ -7150,7 +7150,7 @@ void Heap::SetIsMarkingFlag(bool value) {
   isolate()->isolate_data()->is_marking_flag_ = value;
 }
 
-bool* Heap::IsMarkingFlagAddress() {
+uint8_t* Heap::IsMarkingFlagAddress() {
   return &isolate()->isolate_data()->is_marking_flag_;
 }
 
@@ -7158,7 +7158,7 @@ void Heap::SetIsMinorMarkingFlag(bool value) {
   isolate()->isolate_data()->is_minor_marking_flag_ = value;
 }
 
-bool* Heap::IsMinorMarkingFlagAddress() {
+uint8_t* Heap::IsMinorMarkingFlagAddress() {
   return &isolate()->isolate_data()->is_minor_marking_flag_;
 }
 

src/heap/heap.h

@@ -1074,8 +1074,8 @@ class Heap {
   // ===========================================================================
 
   // Used for query incremental marking status in generated code.
-  bool* IsMarkingFlagAddress();
-  bool* IsMinorMarkingFlagAddress();
+  uint8_t* IsMarkingFlagAddress();
+  uint8_t* IsMinorMarkingFlagAddress();
 
   void ClearRecordedSlot(HeapObject object, ObjectSlot slot);
   void ClearRecordedSlotRange(Address start, Address end);