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

[Andrei Safronov via Phabricator via llvm-commits]

andreisfr 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
+
----------------
jhenderson wrote:
> 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.
File name is renamed to xtensa-header-flags.test.


================
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
----------------
jhenderson wrote:
> 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.
Thank you for suggestions, I corrected this code in xtensa-header-flags.test


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

https://reviews.llvm.org/D64827