[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/tools/llvm-objdump/llvm-objdump.cpp:1399-1414
+      // if (Obj->isELF() && Obj->getArch() == Triple::amdgcn) {
+      //   if (Symbols[SI].Type == ELF::STT_AMDGPU_HSA_KERNEL) {
+      //     // skip amd_kernel_code_t at the begining of kernel symbol (256
+      //     bytes) Start += 256;
+      //   }
+      //   if (SI == SE - 1 ||
+      //       Symbols[SI + 1].Type == ELF::STT_AMDGPU_HSA_KERNEL) {
----------------
kzhuravl wrote:
> Why is this commented out?
`if (Symbols[SI].Type == ELF::STT_AMDGPU_HSA_KERNEL)` always evaluates to false. This is because there are two symbols for AMDGPU kernels:

`<kernel>` => the proper symbol (`STT_AMDGPU_HSA_KERNEL`)
`<kernel>$local` => the additional symbol of `STT_NOTYPE`

Both these symbols point to the same location. Now `<kernel>` is skipped early on in this loop because it is at the beginning of the next symbol in the list `<kernel>$local`

Additionally, this is symbol specific behavior and is being handled in onSymbolStart in the disassembler implementation.



`if (SI == SE - 1 ||  Symbols[SI + 1].Type == ELF::STT_AMDGPU_HSA_KERNEL)`

The first part of this condition evaluates to true for the last symbol in a section. Kernel descriptors are usually appear the .rodata section of the binary. Last 6 bytes of the kernel descriptor are reserved and must be zero. Due to this condition, we end up trimming last 4 bytes of the kernel descriptor, making it 'invalid'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80713