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

Tue Oct 6 10:46:08 PDT 2020

scott.linder added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1395
+  assert(Bytes.size() == 64);
+  DataExtractor DE(Bytes, /*IsLittleEndian=*/true, /*AddressSize=*/8);
+
----------------
scott.linder wrote:
> scott.linder wrote:
> > rochauha wrote:
> > > I think this is where little endian is being 'hardcoded'. However, since AMDGPU relocatable objects are meant to be little endian, I don't understand why they are big endian on pp64.
> > I think this is actually a bug with the encoding code. Like you say, we should be host-endianness agnostic when encoding the kernel descriptor, but it seems we aren't.
> > 
> > I think I vaguely remember this coming up when we implemented it, but I don't remember why we didn't do this to start. I think it is just an oversight.
> I think https://reviews.llvm.org/D88858 should be the fix, need to confirm if big-endian testers will run it automatically.
I tried to determine if the pre-checkin builders include any big-endian archs, but gave up and just committed it. I'll keep an eye on the builders and see if it needs to be revered. After that you can proceed with this patch again.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80713