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

Ronak Chauhan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 2 05:20:40 PDT 2020


rochauha added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1345
+  return MCDisassembler::Success;
+} // decodeCOMPUTE_PGM_RSRC1()
+
----------------
scott.linder wrote:
> I don't know the general conventions here, but I don't think I have seen a comment for the end of a function elsewhere in LLVM. I do know that it is required for namespaces, so maybe it is permitted for long functions?
I'm not sure. I added those comments because these functions were getting quite long.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1630
+  // CP microcode requires the kernel descriptor to be 64 aligned.
+  if (Bytes.size() != 64 || KdAddress % 64 != 0)
+    return MCDisassembler::Fail;
----------------
scott.linder wrote:
> The `!= 0` here is redundant.
I know, but I thought that it is more readable this way. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80713





More information about the llvm-commits mailing list