[PATCH] D125316: [AMDGPU] gfx11 Decode wider instructions. NFC

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

Overall looks fine, but I do not understand the context in which DecoderUInt128 will be used.



================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:425
       if (STI.getFeatureBits()[AMDGPU::FeatureGFX10_BEncoding]) {
-        Res = tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address);
+        Res = tryDecodeInst<uint64_t>(DecoderTableGFX10_B64, MI, QW, Address);
         if (Res) {
----------------
Compiler should be able to deduce template type by QW type, why did you specify the type explicitly?


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h:43
+  operator bool() const { return Lo || Hi; }
+  void insertBits(uint64_t SubBits, unsigned bitPosition, unsigned numBits) {
+    assert(numBits && numBits <= 64);
----------------
Should be camel case.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h:54-55
+  }
+  uint64_t extractBitsAsZExtValue(unsigned numBits,
+                                  unsigned bitPosition) const {
+    assert(numBits && numBits <= 64);
----------------
Ditto.
Also it looks a bit illogical that the arguments are specified in the reversed order compared with insertBits. I assume this is required by autogenerated code?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125316/new/

https://reviews.llvm.org/D125316