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

Fri Feb 14 02:48:29 PST 2020

jhenderson added a comment.

There should be an llvm-readobj test for the xtensa file header (machine type + flags).

================
Comment at: llvm/include/llvm/BinaryFormat/ELFRelocs/Xtensa.def:1
+#ifndef ELF_RELOC
+#error "ELF_RELOC must be defined"
----------------
(Not specific to your change, but I don't understand why .def files don't seem to have any licence information)

================
Comment at: llvm/test/Object/obj2yaml.test:667
+# ELF-XTENSA-NEXT:   Machine:         EM_XTENSA
+# ELF-XTENSA-NEXT:   Flags:           [ EF_XTENSA_MACH_BASE, EF_XTENSA_XT_LIT ]
+
----------------
It looks weird to me seeing a difference in the inptut Flags and printed Flags. Is EF_XTENSA_MACH_BASE a marker (like SHT_LORESERVE etc) or an actual flag? If it's only a marker, we shouldn't dump it.

================
Comment at: llvm/test/Object/obj2yaml.test:675
+  Machine:         EM_XTENSA
+  Flags:           [ EF_XTENSA_XT_LIT ]
+
----------------
This should specify all EF_XTENSA flags, to show that they all work.

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

https://reviews.llvm.org/D64827