[NFC][TableGen] Change DecoderEmitter insertBits to use integer types only

[jurahul]

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.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-backend-amdgpu

Author: Rahul Joshi (jurahul)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/147613.diff

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h (+1-17)
• (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+11-14)

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;
 }
 )";
 }

[jurahul]

Thanks!