Bug 1958280 part 3 - Tidy up the code for adding GC allocations to safepoints. r=iain

Differential Revision: https://phabricator.services.mozilla.com/D246440

js/src/jit/BacktrackingAllocator.cpp

@@ -4612,43 +4612,6 @@ size_t BacktrackingAllocator::findFirstSafepoint(CodePosition pos,
   return i;
 }
 
-// Helper for ::populateSafepoints
-static inline bool IsNunbox(VirtualRegister& reg) {
-#ifdef JS_NUNBOX32
-  return reg.type() == LDefinition::TYPE || reg.type() == LDefinition::PAYLOAD;
-#else
-  return false;
-#endif
-}
-
-// Helper for ::populateSafepoints
-static inline bool IsSlotsOrElements(VirtualRegister& reg) {
-  return reg.type() == LDefinition::SLOTS;
-}
-
-// Helper for ::populateSafepoints
-static inline bool IsTraceable(VirtualRegister& reg) {
-  if (reg.type() == LDefinition::OBJECT ||
-      reg.type() == LDefinition::WASM_ANYREF) {
-    return true;
-  }
-#ifdef JS_PUNBOX64
-  if (reg.type() == LDefinition::BOX) {
-    return true;
-  }
-#endif
-  if (reg.type() == LDefinition::STACKRESULTS) {
-    MOZ_ASSERT(reg.def());
-    const LStackArea* alloc = reg.def()->output()->toStackArea();
-    for (auto iter = alloc->results(); iter; iter.next()) {
-      if (iter.isWasmAnyRef()) {
-        return true;
-      }
-    }
-  }
-  return false;
-}
-
 bool BacktrackingAllocator::populateSafepoints() {
   JitSpew(JitSpew_RegAlloc, "Populating Safepoints");
 
@@ -4661,8 +4624,7 @@ bool BacktrackingAllocator::populateSafepoints() {
   for (uint32_t i = 1; i < graph.numVirtualRegisters(); i++) {
     VirtualRegister& reg = vregs[i];
 
-    if (!reg.def() ||
+    if (!reg.def() || !reg.def()->isSafepointGCType(reg.ins())) {
-        (!IsTraceable(reg) && !IsSlotsOrElements(reg) && !IsNunbox(reg))) {
       continue;
     }
 
@@ -4711,54 +4673,9 @@ bool BacktrackingAllocator::populateSafepoints() {
           continue;
         }
 
-        switch (reg.type()) {
+        if (!safepoint->addGCAllocation(i, reg.def(), a)) {
-          case LDefinition::OBJECT:
-            if (!safepoint->addGcPointer(a)) {
           return false;
         }
-            break;
-          case LDefinition::SLOTS:
-            if (!safepoint->addSlotsOrElementsPointer(a)) {
-              return false;
-            }
-            break;
-          case LDefinition::WASM_ANYREF:
-            if (!safepoint->addWasmAnyRef(a)) {
-              return false;
-            }
-            break;
-          case LDefinition::STACKRESULTS: {
-            MOZ_ASSERT(a.isStackArea());
-            for (auto iter = a.toStackArea()->results(); iter; iter.next()) {
-              if (iter.isWasmAnyRef()) {
-                if (!safepoint->addWasmAnyRef(iter.alloc())) {
-                  return false;
-                }
-              }
-            }
-            break;
-          }
-#ifdef JS_NUNBOX32
-          case LDefinition::TYPE:
-            if (!safepoint->addNunboxType(i, a)) {
-              return false;
-            }
-            break;
-          case LDefinition::PAYLOAD:
-            if (!safepoint->addNunboxPayload(i, a)) {
-              return false;
-            }
-            break;
-#else
-          case LDefinition::BOX:
-            if (!safepoint->addBoxedValue(a)) {
-              return false;
-            }
-            break;
-#endif
-          default:
-            MOZ_CRASH("Bad register type");
-        }
       }
     }
   }

js/src/jit/LIR.cpp

@@ -681,6 +681,51 @@ void LInstruction::initSafepoint(TempAllocator& alloc) {
   MOZ_ASSERT(safepoint_);
 }
 
+bool LSafepoint::addGCAllocation(uint32_t vregId, LDefinition* def,
+                                 LAllocation a) {
+  switch (def->type()) {
+    case LDefinition::OBJECT:
+      return addGcPointer(a);
+
+    case LDefinition::SLOTS:
+      return addSlotsOrElementsPointer(a);
+
+    case LDefinition::WASM_ANYREF:
+      return addWasmAnyRef(a);
+
+#ifdef JS_NUNBOX32
+    case LDefinition::TYPE:
+      return addNunboxType(vregId, a);
+
+    case LDefinition::PAYLOAD:
+      return addNunboxPayload(vregId, a);
+#else
+    case LDefinition::BOX:
+      return addBoxedValue(a);
+#endif
+
+    case LDefinition::STACKRESULTS: {
+      MOZ_ASSERT(a.isStackArea());
+      for (auto iter = a.toStackArea()->results(); iter; iter.next()) {
+        if (iter.isWasmAnyRef()) {
+          if (!addWasmAnyRef(iter.alloc())) {
+            return false;
+          }
+        }
+      }
+      return true;
+    }
+
+    case LDefinition::GENERAL:
+    case LDefinition::INT32:
+    case LDefinition::FLOAT32:
+    case LDefinition::DOUBLE:
+    case LDefinition::SIMD128:
+      break;
+  }
+  MOZ_CRASH("Bad register type");
+}
+
 bool LMoveGroup::add(LAllocation from, LAllocation to, LDefinition::Type type) {
 #ifdef DEBUG
   MOZ_ASSERT(from != to);

js/src/jit/LIR.h

@@ -36,6 +36,7 @@ class LStackArea;
 class LArgument;
 class LConstantIndex;
 class LInstruction;
+class LNode;
 class LDefinition;
 class MBasicBlock;
 class MIRGenerator;
@@ -660,6 +661,11 @@ class LDefinition {
     return output_.toConstantIndex()->index();
   }
 
+  // Returns true if this definition should be added to safepoints for GC
+  // tracing. This includes Value type tags on 32-bit and slots/elements
+  // pointers.
+  inline bool isSafepointGCType(LNode* ins) const;
+
   static inline Type TypeFrom(MIRType type) {
     switch (type) {
       case MIRType::Boolean:
@@ -1899,6 +1905,9 @@ class LSafepoint : public TempObject {
 
 #endif  // JS_PUNBOX64
 
+  [[nodiscard]] bool addGCAllocation(uint32_t vregId, LDefinition* def,
+                                     LAllocation a);
+
   bool encoded() const { return safepointOffset_ != INVALID_SAFEPOINT_OFFSET; }
   uint32_t offset() const {
     MOZ_ASSERT(encoded());
@@ -1997,6 +2006,37 @@ class LInstruction::InputIterator {
   LAllocation* operator->() const { return **this; }
 };
 
+bool LDefinition::isSafepointGCType(LNode* ins) const {
+  switch (type()) {
+    case LDefinition::OBJECT:
+    case LDefinition::SLOTS:
+    case LDefinition::WASM_ANYREF:
+#ifdef JS_NUNBOX32
+    case LDefinition::TYPE:
+    case LDefinition::PAYLOAD:
+#else
+    case LDefinition::BOX:
+#endif
+      return true;
+    case LDefinition::STACKRESULTS: {
+      LStackArea alloc(ins->toInstruction());
+      for (auto iter = alloc.results(); iter; iter.next()) {
+        if (iter.isWasmAnyRef()) {
+          return true;
+        }
+      }
+      return false;
+    }
+    case LDefinition::GENERAL:
+    case LDefinition::INT32:
+    case LDefinition::FLOAT32:
+    case LDefinition::DOUBLE:
+    case LDefinition::SIMD128:
+      return false;
+  }
+  MOZ_CRASH("invalid type");
+}
+
 class LIRGraph {
   struct ValueHasher {
     using Lookup = Value;