[llvm] 23b4f4e - [NFC][TableGen] Change DecoderEmitter `insertBits` to use integer types only (#147613)

[via llvm-commits]

Author: Rahul Joshi
Date: 2025-07-09T08:56:07-07:00
New Revision: 23b4f4eb9b15e0c3a8d86d9c0857075afcfc7fe3

URL: https://github.com/llvm/llvm-project/commit/23b4f4eb9b15e0c3a8d86d9c0857075afcfc7fe3
DIFF: https://github.com/llvm/llvm-project/commit/23b4f4eb9b15e0c3a8d86d9c0857075afcfc7fe3.diff

LOG: [NFC][TableGen] Change DecoderEmitter `insertBits` to use integer types only (#147613)

The `insertBits` templated function generated by DecoderEmitter is
called with variable `tmp` of type `TmpType` which is:

```
using TmpType = std::conditional_t<std::is_integral<InsnType>::value, InsnType, uint64_t>;
```

That is, `TmpType` is always an integral type. Change the generated
`insertBits` to be valid only for integer types, and eliminate the
unused `insertBits` function from `DecoderUInt128` in
AMDGPUDisassembler.h

Additionally, drop some of the requirements `InsnType` must support as
they no longer seem to be required.

Added: 
    

Modified: 
    llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
    llvm/utils/TableGen/DecoderEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
index 67156b4a3a188..0fe487ff26fc1 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
@@ -43,17 +43,6 @@ class DecoderUInt128 {
   DecoderUInt128() = default;
   DecoderUInt128(uint64_t Lo, uint64_t Hi = 0) : Lo(Lo), Hi(Hi) {}
   operator bool() const { return Lo || Hi; }
-  void insertBits(uint64_t SubBits, unsigned BitPosition, unsigned NumBits) {
-    assert(NumBits && NumBits <= 64);
-    assert(SubBits >> 1 >> (NumBits - 1) == 0);
-    assert(BitPosition < 128);
-    if (BitPosition < 64) {
-      Lo |= SubBits << BitPosition;
-      Hi |= SubBits >> 1 >> (63 - BitPosition);
-    } else {
-      Hi |= SubBits << (BitPosition - 64);
-    }
-  }
   uint64_t extractBitsAsZExtValue(unsigned NumBits,
                                   unsigned BitPosition) const {
     assert(NumBits && NumBits <= 64);
@@ -78,12 +67,7 @@ class DecoderUInt128 {
   bool operator!=(const DecoderUInt128 &RHS) {
     return Lo != RHS.Lo || Hi != RHS.Hi;
   }
-  bool operator!=(const int &RHS) {
-    return *this != DecoderUInt128(RHS);
-  }
-  friend raw_ostream &operator<<(raw_ostream &OS, const DecoderUInt128 &RHS) {
-    return OS << APInt(128, {RHS.Lo, RHS.Hi});
-  }
+  bool operator!=(const int &RHS) { return *this != DecoderUInt128(RHS); }
 };
 
 //===----------------------------------------------------------------------===//

diff  --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index a50fd06435a10..f523fc26a6671 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -2184,12 +2184,9 @@ static void emitFieldFromInstruction(formatted_raw_ostream &OS) {
 // Helper functions for extracting fields from encoded instructions.
 // InsnType must either be integral or an APInt-like object that must:
 // * be default-constructible and copy-constructible
-// * be constructible from an APInt (this can be private)
-// * Support insertBits(bits, startBit, numBits)
 // * Support extractBitsAsZExtValue(numBits, startBit)
 // * Support the ~, &, ==, and != operators with other objects of the same type
 // * Support the != and bitwise & with uint64_t
-// * Support put (<<) to raw_ostream&
 template <typename InsnType>
 #if defined(_MSC_VER) && !defined(__clang__)
 __declspec(noinline)
@@ -2221,17 +2218,17 @@ fieldFromInstruction(const InsnType &insn, unsigned startBit,
 static void emitInsertBits(formatted_raw_ostream &OS) {
   OS << R"(
 // Helper function for inserting bits extracted from an encoded instruction into
-// a field.
-template <typename InsnType>
-static void insertBits(InsnType &field, InsnType bits, unsigned startBit,
-                       unsigned numBits) {
-  if constexpr (std::is_integral<InsnType>::value) {
-    assert(startBit + numBits <= sizeof field * 8);
-    (void)numBits;
-    field |= (InsnType)bits << startBit;
-  } else {
-    field.insertBits(bits, startBit, numBits);
-  }
+// an integer-typed field.
+template <typename IntType>
+static std::enable_if_t<std::is_integral_v<IntType>, void>
+insertBits(IntType &field, IntType bits, unsigned startBit, unsigned numBits) {
+  // Check that no bit beyond numBits is set, so that a simple bitwise |
+  // is sufficient.
+  assert((~(((IntType)1 << numBits) - 1) & bits) == 0 &&
+           "bits has more than numBits bits set");
+  assert(startBit + numBits <= sizeof(IntType) * 8);
+  (void)numBits;
+  field |= bits << startBit;
 }
 )";
 }