[codegen] Decouple stack slots from safepoint entry size

We plan to trim the size of safepoint entries, to avoid emitting
completely empty entries, and also saving some bytes by removing
trailing zeros.
This CL prepares that by removing the assumption that the safepoint
entry contains one bit per stack slot. Instead, we just use all bits
that are there in the safepoint entry when iterating compiled frames.

R=jkummerow@chromium.org

Bug: v8:11630
Change-Id: Ib335a34da92a08e28fe84fb74f50c8535c2f4c41
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2831484Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74031}

src/codegen/safepoint-table.cc

@@ -20,20 +20,18 @@ namespace internal {
 
 SafepointTable::SafepointTable(Isolate* isolate, Address pc, Code code)
     : SafepointTable(code.InstructionStart(isolate, pc),
-                     code.SafepointTableAddress(), code.stack_slots(), true) {}
+                     code.SafepointTableAddress(), true) {}
 
 #if V8_ENABLE_WEBASSEMBLY
 SafepointTable::SafepointTable(const wasm::WasmCode* code)
     : SafepointTable(code->instruction_start(),
                      code->instruction_start() + code->safepoint_table_offset(),
-                     code->stack_slots(), false) {}
+                     false) {}
 #endif  // V8_ENABLE_WEBASSEMBLY
 
 SafepointTable::SafepointTable(Address instruction_start,
-                               Address safepoint_table_address,
-                               uint32_t stack_slots, bool has_deopt)
+                               Address safepoint_table_address, bool has_deopt)
     : instruction_start_(instruction_start),
-      stack_slots_(stack_slots),
       has_deopt_(has_deopt),
       safepoint_table_address_(safepoint_table_address),
       length_(ReadLength(safepoint_table_address)),
@@ -69,27 +67,18 @@ SafepointEntry SafepointTable::FindEntry(Address pc) const {
   UNREACHABLE();
 }
 
-void SafepointTable::PrintEntry(unsigned index,
-                                std::ostream& os) const {  // NOLINT
+void SafepointTable::PrintEntry(unsigned index, std::ostream& os) const {
   disasm::NameConverter converter;
   SafepointEntry entry = GetEntry(index);
   uint8_t* bits = entry.bits();
 
   // Print the stack slot bits.
   if (entry_size_ > 0) {
-    const int first = 0;
-    int last = entry_size_ - 1;
-    for (int i = first; i < last; i++) PrintBits(os, bits[i], kBitsPerByte);
-    int last_bits = stack_slots_ - ((last - first) * kBitsPerByte);
-    PrintBits(os, bits[last], last_bits);
-  }
-}
-
-void SafepointTable::PrintBits(std::ostream& os,  // NOLINT
-                               uint8_t byte, int digits) {
-  DCHECK(digits >= 0 && digits <= kBitsPerByte);
-  for (int i = 0; i < digits; i++) {
-    os << (((byte & (1 << i)) == 0) ? "0" : "1");
+    for (uint32_t i = 0; i < entry_size_; ++i) {
+      for (int bit = 0; bit < kBitsPerByte; ++bit) {
+        os << ((bits[i] & (1 << bit)) ? "1" : "0");
+      }
+    }
   }
 }
 

src/codegen/safepoint-table.h

@@ -5,6 +5,7 @@
 #ifndef V8_CODEGEN_SAFEPOINT_TABLE_H_
 #define V8_CODEGEN_SAFEPOINT_TABLE_H_
 
+#include "src/base/iterator.h"
 #include "src/base/memory.h"
 #include "src/common/assert-scope.h"
 #include "src/utils/allocation.h"
@@ -21,11 +22,14 @@ class WasmCode;
 
 class SafepointEntry {
  public:
-  SafepointEntry()
-      : deopt_index_(0), bits_(nullptr), trampoline_pc_(kNoTrampolinePC) {}
-
-  SafepointEntry(unsigned deopt_index, uint8_t* bits, int trampoline_pc)
-      : deopt_index_(deopt_index), bits_(bits), trampoline_pc_(trampoline_pc) {
+  SafepointEntry() = default;
+
+  SafepointEntry(unsigned deopt_index, uint8_t* bits, uint8_t* bits_end,
+                 int trampoline_pc)
+      : deopt_index_(deopt_index),
+        bits_(bits),
+        bits_end_(bits_end),
+        trampoline_pc_(trampoline_pc) {
     DCHECK(is_valid());
   }
 
@@ -38,6 +42,7 @@ class SafepointEntry {
   void Reset() {
     deopt_index_ = 0;
     bits_ = nullptr;
+    bits_end_ = nullptr;
   }
 
   int trampoline_pc() { return trampoline_pc_; }
@@ -67,16 +72,23 @@ class SafepointEntry {
     return deopt_index_ != kNoDeoptIndex;
   }
 
-  uint8_t* bits() {
+  uint8_t* bits() const {
     DCHECK(is_valid());
     return bits_;
   }
 
+  base::iterator_range<uint8_t*> iterate_bits() const {
+    return base::make_iterator_range(bits_, bits_end_);
+  }
+
+  size_t entry_size() const { return bits_end_ - bits_; }
+
  private:
-  uint32_t deopt_index_;
-  uint8_t* bits_;
+  uint32_t deopt_index_ = 0;
+  uint8_t* bits_ = nullptr;
+  uint8_t* bits_end_ = nullptr;
   // It needs to be an integer as it is -1 for eager deoptimizations.
-  int trampoline_pc_;
+  int trampoline_pc_ = kNoTrampolinePC;
 };
 
 class SafepointTable {
@@ -117,7 +129,7 @@ class SafepointTable {
     int trampoline_pc = has_deopt_
                             ? base::Memory<int>(GetTrampolineLocation(index))
                             : SafepointEntry::kNoTrampolinePC;
-    return SafepointEntry(deopt_index, bits, trampoline_pc);
+    return SafepointEntry(deopt_index, bits, bits + entry_size_, trampoline_pc);
   }
 
   // Returns the entry for the given pc.
@@ -127,7 +139,7 @@ class SafepointTable {
 
  private:
   SafepointTable(Address instruction_start, Address safepoint_table_address,
-                 uint32_t stack_slots, bool has_deopt);
+                 bool has_deopt);
 
   static const uint8_t kNoRegisters = 0xFF;
 
@@ -165,12 +177,9 @@ class SafepointTable {
     return GetPcOffsetLocation(index) + kTrampolinePcOffset;
   }
 
-  static void PrintBits(std::ostream& os, uint8_t byte, int digits);
-
   DISALLOW_GARBAGE_COLLECTION(no_gc_)
 
   const Address instruction_start_;
-  const uint32_t stack_slots_;
   const bool has_deopt_;
 
   // Safepoint table layout.

src/execution/frames.cc

@@ -1059,12 +1059,14 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
   }
 
   // Visit pointer spill slots and locals.
-  uint8_t* safepoint_bits = safepoint_entry.bits();
-  for (unsigned index = 0; index < stack_slots; index++) {
-    int byte_index = index >> kBitsPerByteLog2;
-    int bit_index = index & (kBitsPerByte - 1);
-    if ((safepoint_bits[byte_index] & (1U << bit_index)) != 0) {
-      FullObjectSlot spill_slot = parameters_limit + index;
+  DCHECK_GE((stack_slots + kBitsPerByte) / kBitsPerByte,
+            safepoint_entry.entry_size());
+  int slot_offset = 0;
+  for (uint8_t bits : safepoint_entry.iterate_bits()) {
+    while (bits) {
+      int bit = base::bits::CountTrailingZeros(bits);
+      bits &= ~(1 << bit);
+      FullObjectSlot spill_slot = parameters_limit + slot_offset + bit;
 #ifdef V8_COMPRESS_POINTERS
       // Spill slots may contain compressed values in which case the upper
       // 32-bits will contain zeros. In order to simplify handling of such
@@ -1082,6 +1084,7 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
 #endif
       v->VisitRootPointer(Root::kStackRoots, nullptr, spill_slot);
     }
+    slot_offset += kBitsPerByte;
   }
 
   // Visit tagged parameters that have been passed to the function of this