[PATCH] D156939: [AMDGPU][True16] Support disassembling .h registers.

Thu Sep 21 03:00:33 PDT 2023

kosarev added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:506

-  unsigned Reg = TRI->getEncodingValue(AMDGPU::getMCReg(Op.getReg(), *ST));
+  unsigned Reg = TRI->getEncodingValue(AMDGPU::getMCReg(Op.getReg(), *ST)) &
+                 AMDGPU::EncValues::REG_IDX_MASK;
----------------
rampitec wrote:
> Why not to call `TRI->getHWRegIndex(AMDGPU::getMCReg(Op.getReg(), *ST))` here and remove any dependency on the encoding?
For these initial patches I would like to keep changes minimal and avoid any immediate reworks and refinements. We can always do that later (it's in my TODO list).

================
Comment at: llvm/test/MC/Disassembler/AMDGPU/gfx11_dasm_vop2.txt:70
+# GFX11-REAL16: v_add_f16_e32 v5.l, v1.h, v2.l   ; encoding: [0x81,0x05,0x0a,0x64]
+# GFX11-FAKE16: v_add_f16_e32 v5, v129/*Invalid register, operand has 'VS_32_Lo128' register class*/, v2 ; encoding: [0x81,0x05,0x0a,0x64]
+0x81,0x05,0x0a,0x64
----------------
rampitec wrote:
> Does it just break fake16 and these can be removed completely at this point?
Well, it just exposes the current level of support for these instructions, and gives them some coverage. I believe we didn't break anything here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156939