Bug 1728549 - Make pointer values RegPtr. r=jseward

Traditionally pointer values (tls, heap pointer) were represented as
RegI32 because we "know" that that is really a GPR, even if the GPR is
64-bit on 64-bit systems.

Later, we introduced RegPtr to hold pointer values that were not
reference values, but used only in some cases.  Let's use RegPtr to
hold the tls and heapreg pointer values now, introducing better type
safety that will aid us in the memory64 work.

There's a wrinkle because RegPtr can't be stored on the value stack.
Recently a "RegIntptr" abstraction was introduced as part of the
cleanup of WasmBCMemory, but this is not necessary at this time: all
we need is a local conversion to RegI32 or RegI64 as appropriate for
the platform, in the one place where this is needed, to push a RegPtr
value.  So simplify this code by removing RegIntptr again and adding
some local conversion functions.

Drive-by fix: Remove some redundant annotations on method definitons.

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

js/src/wasm/WasmBCClass.h

@@ -436,7 +436,13 @@ struct BaseCompiler final {
   // Free r if it is not invalid.
   inline void maybeFree(RegI32 r);
   inline void maybeFree(RegI64 r);
+  inline void maybeFree(RegF32 r);
   inline void maybeFree(RegF64 r);
+  inline void maybeFree(RegRef r);
+  inline void maybeFree(RegPtr r);
+#ifdef ENABLE_WASM_SIMD
+  inline void maybeFree(RegV128 r);
+#endif
 
   // On 64-bit systems, `except` must equal r and this is a no-op.  On 32-bit
   // systems, `except` must equal the high or low part of a pair and the other
@@ -934,6 +940,7 @@ struct BaseCompiler final {
   inline void moveI32(RegI32 src, RegI32 dest);
   inline void moveI64(RegI64 src, RegI64 dest);
   inline void moveRef(RegRef src, RegRef dest);
+  inline void movePtr(RegPtr src, RegPtr dest);
   inline void moveF64(RegF64 src, RegF64 dest);
   inline void moveF32(RegF32 src, RegF32 dest);
 #ifdef ENABLE_WASM_SIMD
@@ -1049,17 +1056,17 @@ struct BaseCompiler final {
                      uint32_t local);
   void bceLocalIsUpdated(uint32_t local);
   void prepareMemoryAccess(MemoryAccessDesc* access, AccessCheck* check,
-                           RegI32 tls, RegI32 ptr);
+                           RegPtr tls, RegI32 ptr);
 
 #if defined(RABALDR_HAS_HEAPREG)
   BaseIndex prepareAtomicMemoryAccess(MemoryAccessDesc* access,
-                                      AccessCheck* check, RegI32 tls,
+                                      AccessCheck* check, RegPtr tls,
                                       RegI32 ptr);
 #else
   // Some consumers depend on the returned Address not incorporating tls, as tls
   // may be the scratch register.
   Address prepareAtomicMemoryAccess(MemoryAccessDesc* access,
-                                    AccessCheck* check, RegI32 tls, RegI32 ptr);
+                                    AccessCheck* check, RegPtr tls, RegI32 ptr);
 #endif
   void computeEffectiveAddress(MemoryAccessDesc* access);
   [[nodiscard]] bool needTlsForAccess(const AccessCheck& check);
@@ -1067,12 +1074,12 @@ struct BaseCompiler final {
   // ptr and dest may be the same iff dest is I32.
   // This may destroy ptr even if ptr and dest are not the same.
   [[nodiscard]] bool load(MemoryAccessDesc* access, AccessCheck* check,
-                          RegI32 tls, RegI32 ptr, AnyReg dest, RegI32 temp);
+                          RegPtr tls, RegI32 ptr, AnyReg dest, RegI32 temp);
 
   // ptr and src must not be the same register.
   // This may destroy ptr and src.
   [[nodiscard]] bool store(MemoryAccessDesc* access, AccessCheck* check,
-                           RegI32 tls, RegI32 ptr, AnyReg src, RegI32 temp);
+                           RegPtr tls, RegI32 ptr, AnyReg src, RegI32 temp);
 
   bool atomicLoad(MemoryAccessDesc* access, ValType type);
   bool atomicStore(MemoryAccessDesc* access, ValType type);
@@ -1231,9 +1238,9 @@ struct BaseCompiler final {
   [[nodiscard]] bool emitTeeLocal();
   [[nodiscard]] bool emitGetGlobal();
   [[nodiscard]] bool emitSetGlobal();
-  [[nodiscard]] RegI32 maybeLoadTlsForAccess(const AccessCheck& check);
-  [[nodiscard]] RegI32 maybeLoadTlsForAccess(const AccessCheck& check,
-                                             RegI32 specific);
+  [[nodiscard]] RegPtr maybeLoadTlsForAccess(const AccessCheck& check);
+  [[nodiscard]] RegPtr maybeLoadTlsForAccess(const AccessCheck& check,
+                                             RegPtr specific);
   [[nodiscard]] bool emitLoad(ValType type, Scalar::Type viewType);
   [[nodiscard]] bool loadCommon(MemoryAccessDesc* access, AccessCheck check,
                                 ValType type);

js/src/wasm/WasmBCCodegen-inl.h

@@ -51,6 +51,12 @@ void BaseCompiler::moveRef(RegRef src, RegRef dest) {
   }
 }
 
+void BaseCompiler::movePtr(RegPtr src, RegPtr dest) {
+  if (src != dest) {
+    masm.movePtr(src, dest);
+  }
+}
+
 void BaseCompiler::moveF64(RegF64 src, RegF64 dest) {
   if (src != dest) {
     masm.moveDouble(src, dest);
@@ -96,6 +102,11 @@ inline void BaseCompiler::move<RegRef>(RegRef src, RegRef dest) {
   moveRef(src, dest);
 }
 
+template <>
+inline void BaseCompiler::move<RegPtr>(RegPtr src, RegPtr dest) {
+  movePtr(src, dest);
+}
+
 #ifdef ENABLE_WASM_SIMD
 template <>
 inline void BaseCompiler::move<RegV128>(RegV128 src, RegV128 dest) {

js/src/wasm/WasmBCMemory.cpp

@@ -162,23 +162,37 @@ RegI32 BaseCompiler::popMemory32Access(MemoryAccessDesc* access,
   return popI32();
 }
 
+#ifdef JS_64BIT
+static inline RegI64 RegPtrToRegIntptr(RegPtr r) {
+  return RegI64(Register64(Register(r)));
+}
+
+static inline RegPtr RegIntptrToRegPptr(RegI64 r) {
+  return RegPtr(Register64(r).reg);
+}
+#else
+static inline RegI32 RegPtrToRegIntptr(RegPtr r) { return RegI32(Register(r)); }
+
+static inline RegPtr RegIntptrToRegPtr(RegI32 r) { return RegPtr(Register(r)); }
+#endif
+
 #ifdef RABALDR_HAS_HEAPREG
 void BaseCompiler::pushHeapBase() {
-  RegIntptr heapBase = need<RegIntptr>();
-  move(RegIntptr(RegIntptrRegister(HeapReg)), heapBase);
-  push(heapBase);
+  RegPtr heapBase = need<RegPtr>();
+  move(RegPtr(HeapReg), heapBase);
+  push(RegPtrToRegIntptr(heapBase));
 }
 #else
 void BaseCompiler::pushHeapBase() {
-  RegIntptr heapBase = need<RegIntptr>();
+  RegPtr heapBase = need<RegPtr>();
   fr.loadTlsPtr(heapBase);
   masm.loadPtr(Address(heapBase, offsetof(TlsData, memoryBase)), heapBase);
-  push(heapBase);
+  push(RegPtrToRegIntptr(heapBase));
 }
 #endif
 
 void BaseCompiler::prepareMemoryAccess(MemoryAccessDesc* access,
-                                       AccessCheck* check, RegI32 tls,
+                                       AccessCheck* check, RegPtr tls,
                                        RegI32 ptr) {
   uint32_t offsetGuardLimit =
       GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled());
@@ -283,7 +297,7 @@ void BaseCompiler::computeEffectiveAddress(MemoryAccessDesc* access) {
   }
 }
 
-[[nodiscard]] bool BaseCompiler::needTlsForAccess(const AccessCheck& check) {
+bool BaseCompiler::needTlsForAccess(const AccessCheck& check) {
 #ifndef RABALDR_HAS_HEAPREG
   // Platform requires Tls for memory base.
   return true;
@@ -292,22 +306,22 @@ void BaseCompiler::computeEffectiveAddress(MemoryAccessDesc* access) {
 #endif
 }
 
-RegI32 BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check) {
+RegPtr BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check) {
   if (needTlsForAccess(check)) {
-    RegI32 tls = needI32();
+    RegPtr tls = need<RegPtr>();
     fr.loadTlsPtr(tls);
     return tls;
   }
-  return RegI32::Invalid();
+  return RegPtr::Invalid();
 }
 
-RegI32 BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check,
-                                           RegI32 specific) {
+RegPtr BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check,
+                                           RegPtr specific) {
   if (needTlsForAccess(check)) {
     fr.loadTlsPtr(specific);
     return specific;
   }
-  return RegI32::Invalid();
+  return RegPtr::Invalid();
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -316,9 +330,8 @@ RegI32 BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check,
 
 // ptr and dest may be the same iff dest is I32.
 // This may destroy ptr even if ptr and dest are not the same.
-[[nodiscard]] bool BaseCompiler::load(MemoryAccessDesc* access,
-                                      AccessCheck* check, RegI32 tls,
-                                      RegI32 ptr, AnyReg dest, RegI32 temp) {
+bool BaseCompiler::load(MemoryAccessDesc* access, AccessCheck* check,
+                        RegPtr tls, RegI32 ptr, AnyReg dest, RegI32 temp) {
   prepareMemoryAccess(access, check, tls, ptr);
 
 #if defined(JS_CODEGEN_X64)
@@ -391,9 +404,8 @@ RegI32 BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check,
 
 // ptr and src must not be the same register.
 // This may destroy ptr and src.
-[[nodiscard]] bool BaseCompiler::store(MemoryAccessDesc* access,
-                                       AccessCheck* check, RegI32 tls,
-                                       RegI32 ptr, AnyReg src, RegI32 temp) {
+bool BaseCompiler::store(MemoryAccessDesc* access, AccessCheck* check,
+                         RegPtr tls, RegI32 ptr, AnyReg src, RegI32 temp) {
   prepareMemoryAccess(access, check, tls, ptr);
 
   // Emit the store
@@ -478,7 +490,8 @@ RegI32 BaseCompiler::maybeLoadTlsForAccess(const AccessCheck& check,
 
 bool BaseCompiler::loadCommon(MemoryAccessDesc* access, AccessCheck check,
                               ValType type) {
-  RegI32 tls, temp;
+  RegPtr tls;
+  RegI32 temp;
 #if defined(JS_CODEGEN_MIPS64)
   temp = needI32();
 #endif
@@ -560,7 +573,8 @@ bool BaseCompiler::loadCommon(MemoryAccessDesc* access, AccessCheck check,
 
 bool BaseCompiler::storeCommon(MemoryAccessDesc* access, AccessCheck check,
                                ValType resultType) {
-  RegI32 tls, temp;
+  RegPtr tls;
+  RegI32 temp;
 #if defined(JS_CODEGEN_MIPS64)
   temp = needI32();
 #endif
@@ -650,7 +664,7 @@ bool BaseCompiler::storeCommon(MemoryAccessDesc* access, AccessCheck check,
 
 BaseIndex BaseCompiler::prepareAtomicMemoryAccess(MemoryAccessDesc* access,
                                                   AccessCheck* check,
-                                                  RegI32 tls, RegI32 ptr) {
+                                                  RegPtr tls, RegI32 ptr) {
   MOZ_ASSERT(needTlsForAccess(*check) == tls.isValid());
   prepareMemoryAccess(access, check, tls, ptr);
   return BaseIndex(HeapReg, ptr, TimesOne, access->offset());
@@ -661,7 +675,7 @@ BaseIndex BaseCompiler::prepareAtomicMemoryAccess(MemoryAccessDesc* access,
 // Some consumers depend on the returned Address not incorporating tls, as tls
 // may be the scratch register.
 Address BaseCompiler::prepareAtomicMemoryAccess(MemoryAccessDesc* access,
-                                                AccessCheck* check, RegI32 tls,
+                                                AccessCheck* check, RegPtr tls,
                                                 RegI32 ptr) {
   MOZ_ASSERT(needTlsForAccess(*check) == tls.isValid());
   prepareMemoryAccess(access, check, tls, ptr);
@@ -735,13 +749,13 @@ bool BaseCompiler::atomicLoad(MemoryAccessDesc* access, ValType type) {
   RegI32 rp = popMemory32Access(access, &check);
 
 #  ifdef RABALDR_HAS_HEAPREG
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   masm.wasmAtomicLoad64(*access, memaddr, temp, rd);
   maybeFree(tls);
 #  else
   ScratchAtomicNoHeapReg scratch(*this);
-  RegI32 tls = maybeLoadTlsForAccess(check, scratch);
+  RegPtr tls = maybeLoadTlsForAccess(check, RegIntptrToRegPtr(scratch));
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   masm.wasmAtomicLoad64(*access, memaddr, temp, rd);
   MOZ_ASSERT(tls == scratch);
@@ -937,7 +951,7 @@ void BaseCompiler::atomicRMW32(MemoryAccessDesc* access, ValType type,
 
   AccessCheck check;
   RegI32 rp = popMemory32Access(access, &check);
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
 
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_rmw32::Perform(this, *access, memaddr, op, rv, rd, temps);
@@ -1088,13 +1102,13 @@ void BaseCompiler::atomicRMW64(MemoryAccessDesc* access, ValType type,
   RegI32 rp = popMemory32Access(access, &check);
 
 #if defined(RABALDR_HAS_HEAPREG)
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_rmw64::Perform(this, *access, memaddr, op, rv, temp, rd);
   maybeFree(tls);
 #else
   ScratchAtomicNoHeapReg scratch(*this);
-  RegI32 tls = maybeLoadTlsForAccess(check, scratch);
+  RegPtr tls = maybeLoadTlsForAccess(check, RegIntptrToRegPtr(scratch));
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_rmw64::Perform(this, *access, memaddr, op, rv, temp, rd, scratch);
   MOZ_ASSERT(tls == scratch);
@@ -1243,7 +1257,7 @@ void BaseCompiler::atomicXchg32(MemoryAccessDesc* access, ValType type) {
   AccessCheck check;
 
   RegI32 rp = popMemory32Access(access, &check);
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
 
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_xchg32::Perform(this, *access, memaddr, rv, rd, temps);
@@ -1342,13 +1356,13 @@ void BaseCompiler::atomicXchg64(MemoryAccessDesc* access,
   RegI32 rp = popMemory32Access(access, &check);
 
 #ifdef RABALDR_HAS_HEAPREG
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   masm.wasmAtomicExchange64(*access, memaddr, rv, rd);
   maybeFree(tls);
 #else
   ScratchAtomicNoHeapReg scratch(*this);
-  RegI32 tls = maybeLoadTlsForAccess(check, scratch);
+  RegPtr tls = maybeLoadTlsForAccess(check, RegIntptrToRegPtr(scratch));
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   RegI64 rvNew = atomic_xchg64::Setup(this, rv, scratch);
   masm.wasmAtomicExchange64(*access, memaddr, rvNew, rd);
@@ -1513,7 +1527,7 @@ void BaseCompiler::atomicCmpXchg32(MemoryAccessDesc* access, ValType type) {
 
   AccessCheck check;
   RegI32 rp = popMemory32Access(access, &check);
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
 
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_cmpxchg32::Perform(this, *access, memaddr, rexpect, rnew, rd, temps);
@@ -1642,13 +1656,13 @@ void BaseCompiler::atomicCmpXchg64(MemoryAccessDesc* access, ValType type) {
   RegI32 rp = popMemory32Access(access, &check);
 
 #ifdef RABALDR_HAS_HEAPREG
-  RegI32 tls = maybeLoadTlsForAccess(check);
+  RegPtr tls = maybeLoadTlsForAccess(check);
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_cmpxchg64::Perform(this, *access, memaddr, rexpect, rnew, rd);
   maybeFree(tls);
 #else
   ScratchAtomicNoHeapReg scratch(*this);
-  RegI32 tls = maybeLoadTlsForAccess(check, scratch);
+  RegPtr tls = maybeLoadTlsForAccess(check, RegIntptrToRegPtr(scratch));
   auto memaddr = prepareAtomicMemoryAccess(access, &check, tls, rp);
   atomic_cmpxchg64::Perform(this, *access, memaddr, rexpect, rnew, rd, scratch);
   MOZ_ASSERT(tls == scratch);

js/src/wasm/WasmBCRegDefs.h

@@ -330,14 +330,6 @@ struct AnyReg {
   }
 };
 
-#ifdef JS_64BIT
-using RegIntptr = RegI64;
-using RegIntptrRegister = Register64;
-#else
-using RegIntptr = RegI32;
-using RegIntptrRegister = Register;
-#endif
-
 //////////////////////////////////////////////////////////////////////////////
 //
 // Platform-specific registers.

js/src/wasm/WasmBCRegMgmt-inl.h

@@ -160,12 +160,38 @@ void BaseCompiler::maybeFree(RegI64 r) {
   }
 }
 
+void BaseCompiler::maybeFree(RegF32 r) {
+  if (r.isValid()) {
+    freeF32(r);
+  }
+}
+
 void BaseCompiler::maybeFree(RegF64 r) {
   if (r.isValid()) {
     freeF64(r);
   }
 }
 
+void BaseCompiler::maybeFree(RegRef r) {
+  if (r.isValid()) {
+    freeRef(r);
+  }
+}
+
+void BaseCompiler::maybeFree(RegPtr r) {
+  if (r.isValid()) {
+    freePtr(r);
+  }
+}
+
+#ifdef ENABLE_WASM_SIMD128
+void BaseCompiler::maybeFree(RegV128 r) {
+  if (r.isValid()) {
+    freeV128(r);
+  }
+}
+#endif
+
 void BaseCompiler::needI32NoSync(RegI32 r) {
   MOZ_ASSERT(isAvailableI32(r));
   needI32(r);
@@ -280,6 +306,12 @@ inline RegV128 BaseCompiler::pop<RegV128>() {
 }
 #endif
 
+// RegPtr values can't be pushed, hence can't be popped.
+template <>
+inline RegPtr BaseCompiler::need<RegPtr>() {
+  return needPtr();
+}
+
 void BaseCompiler::needResultRegisters(ResultType type, ResultRegKind which) {
   if (type.empty()) {
     return;