[PATCH] D64827: [Xtensa 2/10] Add Xtensa ELF definitions.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/xtensa-elf-flags.test:1
+## Check that we are able to dump EF_XTENSA_XT_* flags correctly
+
----------------
Rename this to `xtensa-header-flags.test`. This should hopefully make it clear that these are flags for the ELF header, without the redundant `elf` in the filename.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/xtensa-elf-flags.test:3-7
+# RUN: yaml2obj %s -o %t -DFLAG=INSN
+# RUN: llvm-readobj -S --file-headers %t | FileCheck --check-prefixes=ELF-ALL,ELF-INSN %s
+
+# RUN: yaml2obj %s -o %t -DFLAG=LIT
+# RUN: llvm-readobj -S --file-headers %t | FileCheck --check-prefixes=ELF-ALL,ELF-LIT %s
----------------
Use a different filename for the files. That way, if the test breaks, you can easily compare the two files. This is generally very useful for debugging.

I'd also drop `ELF-` from the prefixes, since they don't add any value.

I've suggested a possible name inline.


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

https://reviews.llvm.org/D64827