[PATCH] D85041: [llvm-libtool-darwin] Add constant CPU_TYPE_ARM64_V8

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test:1
+# RUN: yaml2obj %s -o %tarm-v8.o
+# RUN: llvm-objdump -p %tarm-v8.o | FileCheck --strict-whitespace %s
----------------
Rather than add a test nearly identical to `macho-arm64e.test`, what do you think about renaming that test to `macho-arm64-subtypes.test` (or something similar) and merging the two (optionally with an explicit testing of the `ALL` subtype, but that's not so important since there's coverage elsewhere)


================
Comment at: llvm/test/tools/llvm-readobj/MachO/file-headers-arm64.test:28-43
+#      ARM64-V8:File: [[FILE]]
+# ARM64-V8-NEXT:Format: Mach-O arm64
+# ARM64-V8-NEXT:Arch: aarch64
+# ARM64-V8-NEXT:AddressSize: 64bit
+# ARM64-V8-NEXT:MachHeader {
+# ARM64-V8-NEXT:  Magic: Magic64 (0xFEEDFACF)
+# ARM64-V8-NEXT:  CpuType: Arm64 (0x100000C)
----------------
Rather than duplicating all these CHECKs just for the sake of one different one, you could use `FileCheck`'s -D option or --check-prefixes option to share them.

Example using -D:

```
# RUN:   | FileCheck %s -DSUBTYPE="CPU_SUBTYPE_ARM64_ALL (0x0)" ...

#      CHECK:File: [[FILE]]
...
# CHECK-NEXT:  CpuSubType: [[SUBTYPE]]
...

# RUN:   | FileCheck %s -DSUBTYPE="CPU_SUBTYPE_ARM64_V8 (0x1)" ...
# RUN:   | FileCheck %s -DSUBTYPE="CPU_SUBTYPE_ARM64E (0x2)" ...
```

Using --check-prefixes:

```
# RUN:   | FileCheck %s --check-prefixes="COMMON,ALL" ...
...
# RUN:   | FileCheck %s --check-prefixes="COMMON,V8" ...
...
# RUN:   | FileCheck %s --check-prefixes="COMMON,E" ...

#      COMMON:File: [[FILE]]
...
# COMMON-NEXT:  CpuType: Arm64 (0x100000C)
#    ALL-NEXT:  CpuSubType: CPU_SUBTYPE_ARM64_ALL (0x0)
#     V8-NEXT:  CpuSubType: CPU_SUBTYPE_ARM64_V8 (0x1)
#      E-NEXT:  CpuSubType: CPU_SUBTYPE_ARM64E (0x2)
# COMMON-NEXT:  FileType: Relocatable (0x1)
...
```


================
Comment at: llvm/tools/llvm-objdump/MachODump.cpp:2112-2115
+    case MachO::CPU_SUBTYPE_ARM64_V8:
+      outs() << "    cputype CPU_TYPE_ARM64\n";
+      outs() << "    cpusubtype CPU_SUBTYPE_ARM64_V8\n";
+      break;
----------------
This code-path doesn't seem to have any testing? The new objdump test only covers the other change in this file by my reading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85041