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

Tue Jun 23 04:12:42 PDT 2020

rochauha marked 13 inline comments as done.
rochauha added a comment.

In D80713#2106033 <https://reviews.llvm.org/D80713#2106033>, @jhenderson wrote:

> No idea about the functional logic of this code, but I do wonder whether you'd be better off dividing it into smaller patches for testability.

This patch is for entirely disassembling the kernel descriptor symbol. It happens to be so that many directives affect the kernel descriptor. I will also be adding a new test in this patch.

================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1303
+
+  case 4: // 0 + 4
+    FourByteBuffer = DE.getU32(&CurrentIndex, Err);
----------------
jhenderson wrote:
> I don't understand what this comment is trying to tell me, especially since the `Size` computation below is `4 + 4`...
When we fail, we set:
     Size = CurrentIndex (i.e starting point of the chunk of bytes) + length of the chunk.
The failed region is from 0 to this new value of Size. We do this because most directives in the kernel descriptor affect single or very few bits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80713