[PATCH] D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors

[Ronak Chauhan via Phabricator via llvm-commits]

rochauha marked an inline comment as done.
rochauha added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1245
+  //
+  // To to get the exact same bytes in re-assembled binary, we disassemble
+  // aamdhsa_next_free_sgpr as the amdgcn.next_free_sgpr assembler symbol and
----------------
scott.linder wrote:
> For this and the above case we should have tests to prove this out. I.e. assemble sources to a binary, disassemble and reassemble it, and then compare the two binaries. Ideally we would do this for some edge cases around VGPR/SGPR allocation granularity.
> 
> There may need to be some fixup between disassembly and reassembly to account for the remaining non-reassembleable bits produced by llvm-objdump, but they should be pretty minor for a trivial kernel, and I would expect you could handle them with just `sed` which seems to be available to LIT tests.
Right now we can't really re-assemble in the lit-test. This needs to be tested 'informally' by:

- Manually writing a small test case. Make a copy of it too.  
- Assembling it into the binary : Binary-1.
- Disassembling it.
- Replace the original kernel descriptor with the disassembled kernel descriptor in the copy.
- Assemble the copy : Binary-2.
- Compare Binary-1 and Binary-2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80713