[PATCH] D128014: [AMDGPU] Improve assembler + disassembler handling of kernel descriptors

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 14 10:22:55 PDT 2022


MaskRay added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:2031
+    uint16_t KernelCodeProperties = support::endian::read16(
+        (const void *)&Bytes[amdhsa::KERNEL_CODE_PROPERTIES_OFFSET],
+        support::endianness::little);
----------------
the cast is redundant.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/AMDGPU/kd-gfx10.s:3
+
+; RUN: split-file %s %t.dir
+
----------------
When there are multiple files, consider:

`; RUN: rm -rf %t && split-file %s %t && cd %t`

then just use relative filenames below (remove all `%t`)


================
Comment at: llvm/test/tools/llvm-objdump/ELF/AMDGPU/kd-gfx10.s:6
+; RUN: llvm-mc %t.dir/1.s --triple=amdgcn-amd-amdhsa -mcpu=gfx1010 -mattr=-xnack,+wavefrontsize32,-wavefrontsize64 -filetype=obj -o %t1
+; RUN: llvm-objdump --disassemble-symbols=my_kernel_1.kd %t1 | tail -n +7 \
+; RUN: | llvm-mc --triple=amdgcn-amd-amdhsa -mcpu=gfx1010 -mattr=-xnack,+wavefrontsize32,-wavefrontsize64 -filetype=obj -o %t1-re-assemble
----------------
In several test/tools, the convention is to place `|` at the end of the previous line and indent the continuation line(s) by 2 spaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128014



More information about the llvm-commits mailing list