[PATCH] D52824: [AMDGPU] Implemented MCELFNoteDisassembler for PAL metadata note

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:986-988
+  // PAL metadata is in 32-bit words and only exists on AMDGPU, which is
+  // always little endian, so the following does not work if running
+  // llvm-readobj on a big-endian host.
----------------
We can't just not work on big endian hosts. Can't you just use the utilities in Endian.h to support this?


================
Comment at: lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:989-990
+  // llvm-readobj on a big-endian host.
+#if BYTE_ORDER == LITTLE_ENDIAN
+  case ELF::NT_AMD_AMDGPU_PAL_METADATA: {
+    ArrayRef<uint32_t> DescAsU32(
----------------
I'd prefer to just do if (BYTE_ORDER == LITTLE_ENDIAN) inside


================
Comment at: lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h:184-185
+  // Returns false if the note type is not recognized or disassembly fails.
+  virtual bool disassembleNote(unsigned Type, StringRef Name, StringRef Desc,
+        support::endianness TargetEndianness, MCTargetStreamer *TS) override;
+};
----------------
No virtual


Repository:
  rL LLVM

https://reviews.llvm.org/D52824