[PATCH] D149091: [Object] Recognize ARM64EC binaries in COFFObjectFile::getMachine.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/COFF/arm64ec.yaml:2
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump -d %t | FileCheck -check-prefix=DISAS %s
+# RUN: llvm-readobj --coff-load-config %t | FileCheck -check-prefix=CODEMAP %s
----------------
Nits:
1) Prefer double-dash options for FileCheck (`--check-prefix`), for consistency with the llvm-readobj command you've added.
2) `DISASM` is a more common abbreviation than `DISAS`, so I suggest using that.


================
Comment at: llvm/test/tools/llvm-objdump/COFF/arm64ec.yaml:28
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
----------------
Rather than having two identical(?)-barring-one-field input YAMLs between the two tests, can you use the yaml2obj -D feature to enable sharing of the two? I don't know if that feature is specific to the ELF side or not, but if it is usable, you could have both tests in the same file, and just create two input files from the same input block, which differ only by the Machine type. This would also allow you to minimise the duplication in the FileCheck CHECK patterns.

A good example of what I mean can be seen in this test: https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/file-header-machine-types.test.


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

https://reviews.llvm.org/D149091