[PATCH] D107969: [llvm-readobj] Support dumping of MSP430 ELF attributes

Wed Aug 18 00:17:12 PDT 2021

jhenderson accepted this revision.
jhenderson added a comment.

Looks reasonable to me, barring some nits. It would be worth @asl or someone else with MSP430 knowledge to take a brief look at the test.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/MSP430/build-attributes.test:23
+
+# ENUM_HEADER:        BuildAttributes {
+# ENUM_HEADER-NEXT:   FormatVersion: 0x41
----------------
Small nit: use `-` rather than `_` in prefix names: it's more common in other tests, and is easier to type on a typical English keyboard.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/MSP430/build-attributes.test:84
+# RUN: llvm-readobj -A %t.o | FileCheck %s --check-prefixes HEADER,MSP430,SMALL_CODE,SMALL_DATA
+--- !ELF
+FileHeader:
----------------
Nit: add a blank line immediately before the YAML blocks. This just makes it a little easier to spot the end of the test directives and start of the YAML for a given test case.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107969