[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
----------------
rochauha wrote:
> 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.
Went this route to check whether re-assembled binaries match or not. Turns out that both binaries match, in size (overall size as well as size of sections)  and also in terms of all the disassembled content. But a `diff object1 object2` says that binary files differ.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80713