[PATCH] D126904: [llvm-objdump] Add support for dumping embedded offloading data

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/Offloading/content-failure.test:1
+# Test to check if we failto get the section contents.
+# RUN: yaml2obj %s -o %t
----------------



================
Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:20
+
+static constexpr const char *OffloadSectionString = ".llvm.offloading";
+
----------------
The variable is writable.

`static` is unneeded for const variables (internal linkage by default).


================
Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:23
+/// Get the printable name of the image kind.
+static StringRef getImageName(const OffloadBinary *OB) {
+  switch (OB->getImageKind()) {
----------------
Use const reference if non-null


================
Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:42
+  outs() << "\nOFFLOADING IMAGE "
+         << "[" << Index << "]:\n";
+  outs() << left_justify("kind", 16) << getImageName(OB) << "\n";
----------------
Append `[` to `OFFLOAD IMAGE`


================
Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:61
+    OffloadBinary &Binary = **BinaryOrErr;
+
+    printBinary(&Binary, Index++);
----------------
delete unneeded blank line


================
Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:70
+/// Print the embedded offloading contents of an ObjectFile \p O.
+void llvm::dumpOffloadBinary(const ObjectFile *O) {
+  for (SectionRef Sec : O->sections()) {
----------------
Use const reference if non-null


================
Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:98
+/// multiple binaries stored in the same buffer.
+void llvm::dumpOffloadSections(const OffloadBinary *OB) {
+  // Print out all the binaries that are contained at this buffer. If we fail to
----------------
Use const reference if non-null


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126904