From ffac9f36fb045dd2c6a81e1b5b9ccc115e5ef924 Mon Sep 17 00:00:00 2001 From: kobalicek Date: Fri, 28 Jun 2024 21:41:39 +0200 Subject: [PATCH] [bug] Deprecate BaseMem::setSize() BaseMem::setSize() should not be used anymore as the only memory operand that understands size is x86::Mem, which makes it x86 specific. The reason is that other architectures require more bits, so for example arm::Mem uses the storage used by x86 size for storing other information such as offset mode, and possibly more information will be needed in the future to support AArch64 SVE or SME, etc... At the moment BaseMem::setSize() has been deprecated, so code using it would still compile, but with a warning. It will be removed in the future though. --- .github/workflows/build.yml | 2 +- src/asmjit/core/operand.h | 42 ++++++++++++++++++------------------- src/asmjit/core/ralocal.cpp | 5 ++++- src/asmjit/x86/x86operand.h | 3 +++ 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6e56927..d92f8cf 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -40,7 +40,7 @@ jobs: - { title: "diag-msan" , host: "ubuntu-latest" , arch: "x64" , cc: "clang-18", conf: "Release", diagnostics: "msan", defs: "ASMJIT_TEST=1" } - { title: "diag-ubsan" , host: "ubuntu-latest" , arch: "x64" , cc: "clang-18", conf: "Release", diagnostics: "ubsan", defs: "ASMJIT_TEST=1" } - { title: "diag-hardened" , host: "ubuntu-latest" , arch: "x64" , cc: "clang-18", conf: "Release", diagnostics: "hardened", defs: "ASMJIT_TEST=1" } - - { title: "diag-valgrind" , host: "ubuntu-latest" , arch: "x64" , cc: "clang-18", conf: "Release", diagnostics: "valgrind", defs: "ASMJIT_TEST=1" } + - { title: "diag-valgrind" , host: "ubuntu-24.04" , arch: "x64" , cc: "clang-18", conf: "Release", diagnostics: "valgrind", defs: "ASMJIT_TEST=1" } - { title: "no-deprecated" , host: "ubuntu-latest" , arch: "x64" , cc: "clang-18", conf: "Release", defs: "ASMJIT_TEST=1,ASMJIT_NO_DEPRECATED=1" } - { title: "no-intrinsics" , host: "ubuntu-latest" , arch: "x64" , cc: "clang-18", conf: "Release", defs: "ASMJIT_TEST=1,ASMJIT_NO_INTRINSICS=1" } diff --git a/src/asmjit/core/operand.h b/src/asmjit/core/operand.h index 8a1eee7..3626779 100644 --- a/src/asmjit/core/operand.h +++ b/src/asmjit/core/operand.h @@ -738,26 +738,24 @@ struct Operand_ { //! Returns a size of a register or an X86 memory operand. //! - //! At the moment only X86 and X86_64 memory operands have a size - other memory operands can use bits that represent - //! size as an additional payload. This means that memory size is architecture specific and should be accessed via - //! \ref x86::Mem::size(). Sometimes when the user knows that the operand is either a register or memory operand this - //! function can be helpful as it avoids casting. - ASMJIT_INLINE_NODEBUG constexpr uint32_t x86RmSize() const noexcept { - return _signature.size(); - } - -#if !defined(ASMJIT_NO_DEPRECATED) - ASMJIT_DEPRECATED("hasSize() is no longer portable - use x86RmSize() instead, if your target is X86/X86_64") - ASMJIT_INLINE_NODEBUG constexpr bool hasSize() const noexcept { return x86RmSize() != 0u; } - - ASMJIT_DEPRECATED("hasSize() is no longer portable - use x86RmSize() instead, if your target is X86/X86_64") - ASMJIT_INLINE_NODEBUG constexpr bool hasSize(uint32_t s) const noexcept { return x86RmSize() == s; } - - ASMJIT_DEPRECATED("size() is no longer portable - use x86RmSize() instead, if your target is X86/X86_64") - ASMJIT_INLINE_NODEBUG constexpr uint32_t size() const noexcept { return _signature.getField(); } -#endif + //! \remarks At the moment only X86 and X86_64 memory operands have a size - other memory operands can use bits + //! that represent size as an additional payload. This means that memory size is architecture specific and should + //! be accessed via \ref x86::Mem::size(). Sometimes when the user knows that the operand is either a register or + //! memory operand this function can be helpful as it avoids casting, but it only works when it targets X86 and X86_64. + ASMJIT_INLINE_NODEBUG constexpr uint32_t x86RmSize() const noexcept { return _signature.size(); } //! \} + +#if !defined(ASMJIT_NO_DEPRECATED) + ASMJIT_DEPRECATED("hasSize() is no longer portable - use x86RmSize() or x86::Mem::hasSize() instead, if your target is X86/X86_64") + ASMJIT_INLINE_NODEBUG constexpr bool hasSize() const noexcept { return x86RmSize() != 0u; } + + ASMJIT_DEPRECATED("hasSize() is no longer portable - use x86RmSize() or x86::Mem::hasSize() instead, if your target is X86/X86_64") + ASMJIT_INLINE_NODEBUG constexpr bool hasSize(uint32_t s) const noexcept { return x86RmSize() == s; } + + ASMJIT_DEPRECATED("size() is no longer portable - use x86RmSize() or x86::Mem::size() instead, if your target is X86/X86_64") + ASMJIT_INLINE_NODEBUG constexpr uint32_t size() const noexcept { return _signature.getField(); } +#endif }; //! Base class representing an operand in AsmJit (default constructed version). @@ -1600,9 +1598,6 @@ public: //! Resets the memory operand's INDEX register. ASMJIT_INLINE_NODEBUG void resetIndex() noexcept { _setIndex(RegType::kNone, 0); } - //! Sets the memory operand size (in bytes). - ASMJIT_INLINE_NODEBUG void setSize(uint32_t size) noexcept { _signature.setField(size); } - //! Tests whether the memory operand has a 64-bit offset or absolute address. //! //! If this is true then `hasBase()` must always report false. @@ -1670,6 +1665,11 @@ public: ASMJIT_INLINE_NODEBUG void resetOffsetLo32() noexcept { setOffsetLo32(0); } //! \} + +#if !defined(ASMJIT_NO_DEPRECATED) + ASMJIT_DEPRECATED("setSize() is no longer portable - use setX86RmSize() or x86::Mem::setSize() instead, if your target is X86/X86_64") + ASMJIT_INLINE_NODEBUG void setSize(uint32_t size) noexcept { _signature.setField(size); } +#endif }; //! Type of the an immediate value. diff --git a/src/asmjit/core/ralocal.cpp b/src/asmjit/core/ralocal.cpp index 194b5a8..358fbf5 100644 --- a/src/asmjit/core/ralocal.cpp +++ b/src/asmjit/core/ralocal.cpp @@ -594,7 +594,10 @@ Error RALocalAllocator::allocInst(InstNode* node) noexcept { if (rmSize <= workReg->virtReg()->virtSize()) { Operand& op = node->operands()[opIndex]; op = _pass->workRegAsMem(workReg); - op.as().setSize(rmSize); + + // NOTE: We cannot use `x86::Mem::setSize()` from here, so let's manipulate the signature directly. + op._signature.setSize(rmSize); + tiedReg->_useRewriteMask = 0; tiedReg->markUseDone(); diff --git a/src/asmjit/x86/x86operand.h b/src/asmjit/x86/x86operand.h index ac56731..8510a93 100644 --- a/src/asmjit/x86/x86operand.h +++ b/src/asmjit/x86/x86operand.h @@ -871,6 +871,9 @@ public: //! distinguish between 8-bit, 16-bit, 32-bit, and 64-bit increments. ASMJIT_INLINE_NODEBUG constexpr uint32_t size() const noexcept { return _signature.getField(); } + //! Sets the memory operand size (in bytes). + ASMJIT_INLINE_NODEBUG void setSize(uint32_t size) noexcept { _signature.setField(size); } + //! \} //! \name Address Type