[llvm-libtool-darwin] Add constant CPU_TYPE_ARM64_V8

[sameerarora101]

Summary

Add support for constant MachO::CPU_SUBTYPE_ARM64_V8. This constant is
needed so as to match llvm-libtool-darwin's behavior to that of
cctools' libtool when -arch_only flag is passed in on command line.
Here is the diff of llvm-libtool-darwin adding arch_only option for
reference: https://reviews.llvm.org/D84770.

[alexander-shaposhnikov]

I'd probably add a simple test for llvm-readobj as well.

[MaskRay]

llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test

2

Usually RUN and CHECK lines should use the same comment marker. Either you include the comment marker or leave the comment marker for both RUN and CHECK

[jhenderson]

In D85041#2188548, @alexshap wrote:

I'd probably add a simple test for llvm-readobj as well.

+1. Whilst you're at it. You might as well test the other types too in that enum (assuming they aren't already tested). You can use yaml2obj's -D to reuse the YAML for each input file.

llvm/test/tools/llvm-objdump/MachO/AArch64/Inputs/arm64_v8.macho.yaml

1 ↗

(On Diff #282295)

As this input is only used in one place, you can put it inline with the test that uses it. You'll need to use '#' for the RUN/CHECKs in the test then.

llvm/tools/llvm-objdump/MachODump.cpp

2108

Are there tests for these other cases too? If not, perhaps we should make the new test more generic, and test them all in one test?

[sameerarora101]

llvm/tools/llvm-objdump/MachODump.cpp

2108

Under llvm-objdump, for CPU-TYPE CPU_TYPE_ARM64,

• there is one test case for subtype CPU_SUBTYPE_ARM64E (macho-arm64e.test)
• one test case (which I added above) for CPU_SUBTYPE_ARM64_V8 (macho-arm64-v8.test)
• all remaining tests are for subtype CPU_SUBTYPE_ARM64_ALL

[sameerarora101]

Add test for llvm-readobj

[jhenderson]

llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test

1

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)

llvm/test/tools/llvm-readobj/MachO/file-headers-arm64.test

28–43

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)
...

llvm/tools/llvm-objdump/MachODump.cpp

2112–2115

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.

[sameerarora101]

llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test

1

Yeah, I agree. Updated it now, thanks!

llvm/tools/llvm-objdump/MachODump.cpp

2112–2115

True. Added a test for them while printing universal headers now (similar to universal-x86_64.i386.test) Thanks.

[sameerarora101]

Update tests.

[smeenai]

LGTM, but please wait for @jhenderson too.

[jhenderson]

LGTM, with optional nit.

llvm/test/tools/llvm-objdump/MachO/universal-arm64.test

6 ↗

(On Diff #282926)

Nit: as you only have one check prefix in this file, you can get rid of --check-prefix and just use the default CHECK

[sameerarora101]

Remove --check-prefix=FAT.

llvm/include/llvm/BinaryFormat/MachO.h

enum CPUSubTypeARM {
   CPU_SUBTYPE_ARM_V7K = 12,
   CPU_SUBTYPE_ARM_V6M = 14,
   CPU_SUBTYPE_ARM_V7M = 15,
   CPU_SUBTYPE_ARM_V7EM = 16
 };
 
 enum CPUSubTypeARM64 {
   CPU_SUBTYPE_ARM64_ALL = 0,
+  CPU_SUBTYPE_ARM64_V8 = 1,
   CPU_SUBTYPE_ARM64E = 2,
 };
 
 enum CPUSubTypeARM64_32 { CPU_SUBTYPE_ARM64_32_V8 = 1 };
 
 enum CPUSubTypeSPARC { CPU_SUBTYPE_SPARC_ALL = 0 };
 
 enum CPUSubTypePowerPC {

llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test

+# RUN: yaml2obj %s -o %tarm-v8.o
+# RUN: llvm-objdump -p %tarm-v8.o | FileCheck --strict-whitespace %s
+
+# CHECK:      Mach header
+# CHECK-NEXT:       magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
+# CHECK-NEXT: MH_MAGIC_64   ARM64         V8  0x00      OBJECT     0          0 0x00000000
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x0100000C
+  cpusubtype:      0x00000001
+  filetype:        0x00000001
+  ncmds:           0
+  sizeofcmds:      0
+  flags:           0x00000000
+  reserved:        0x00000000
+...

llvm/test/tools/llvm-readobj/MachO/file-headers-arm64.test

+## Check subtype Arm64-ALL:
+# RUN: yaml2obj %s -o %t.arm -DSUBTYPE=0x0
+# RUN: llvm-readobj -h %t.arm \
+# RUN:  | FileCheck %s --strict-whitespace --match-full-lines -DFILE=%t.arm --check-prefix=ARM64-ALL
+
+#      ARM64-ALL:File: [[FILE]]
+# ARM64-ALL-NEXT:Format: Mach-O arm64
+# ARM64-ALL-NEXT:Arch: aarch64
+# ARM64-ALL-NEXT:AddressSize: 64bit
+# ARM64-ALL-NEXT:MachHeader {
+# ARM64-ALL-NEXT:  Magic: Magic64 (0xFEEDFACF)
+# ARM64-ALL-NEXT:  CpuType: Arm64 (0x100000C)
+# ARM64-ALL-NEXT:  CpuSubType: CPU_SUBTYPE_ARM64_ALL (0x0)
+# ARM64-ALL-NEXT:  FileType: Relocatable (0x1)
+# ARM64-ALL-NEXT:  NumOfLoadCommands: 0
+# ARM64-ALL-NEXT:  SizeOfLoadCommands: 0
+# ARM64-ALL-NEXT:  Flags [ (0x0)
+# ARM64-ALL-NEXT:  ]
+# ARM64-ALL-NEXT:  Reserved: 0x0
+# ARM64-ALL-NEXT:}
+# ARM64-ALL-NOT:{{.}}
+
+## Check subtype Arm64-V8:
+# RUN: yaml2obj %s -o %t.arm -DSUBTYPE=0x1
+# RUN: llvm-readobj -h %t.arm \
+# RUN:  | FileCheck %s --strict-whitespace --match-full-lines -DFILE=%t.arm --check-prefix=ARM64-V8
+
+#      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)
+# ARM64-V8-NEXT:  CpuSubType: CPU_SUBTYPE_ARM64_V8 (0x1)
+# ARM64-V8-NEXT:  FileType: Relocatable (0x1)
+# ARM64-V8-NEXT:  NumOfLoadCommands: 0
+# ARM64-V8-NEXT:  SizeOfLoadCommands: 0
+# ARM64-V8-NEXT:  Flags [ (0x0)
+# ARM64-V8-NEXT:  ]
+# ARM64-V8-NEXT:  Reserved: 0x0
+# ARM64-V8-NEXT:}
+# ARM64-V8-NOT:{{.}}
+
+## Check subtype Arm64E:
+# RUN: yaml2obj %s -o %t.arm -DSUBTYPE=0x2
+# RUN: llvm-readobj -h %t.arm \
+# RUN:  | FileCheck %s --strict-whitespace --match-full-lines -DFILE=%t.arm --check-prefix=ARM64E
+
+#      ARM64E:File: [[FILE]]
+# ARM64E-NEXT:Format: Mach-O arm64
+# ARM64E-NEXT:Arch: aarch64
+# ARM64E-NEXT:AddressSize: 64bit
+# ARM64E-NEXT:MachHeader {
+# ARM64E-NEXT:  Magic: Magic64 (0xFEEDFACF)
+# ARM64E-NEXT:  CpuType: Arm64 (0x100000C)
+# ARM64E-NEXT:  CpuSubType: CPU_SUBTYPE_ARM64E (0x2)
+# ARM64E-NEXT:  FileType: Relocatable (0x1)
+# ARM64E-NEXT:  NumOfLoadCommands: 0
+# ARM64E-NEXT:  SizeOfLoadCommands: 0
+# ARM64E-NEXT:  Flags [ (0x0)
+# ARM64E-NEXT:  ]
+# ARM64E-NEXT:  Reserved: 0x0
+# ARM64E-NEXT:}
+# ARM64E-NOT:{{.}}
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x0100000C
+  cpusubtype:      [[SUBTYPE]]
+  filetype:        0x00000001
+  ncmds:           0
+  sizeofcmds:      0
+  flags:           0x00000000
+  reserved:        0x00000000
+...

llvm/tools/llvm-objdump/MachODump.cpp

case MachO::CPU_SUBTYPE_ARM_V7S:
       break;
     default:
       printUnknownCPUType(cputype, cpusubtype);
       break;
     }
     break;
   case MachO::CPU_TYPE_ARM64:
     switch (cpusubtype & ~MachO::CPU_SUBTYPE_MASK) {
     case MachO::CPU_SUBTYPE_ARM64_ALL:
       outs() << "    cputype CPU_TYPE_ARM64\n";
       outs() << "    cpusubtype CPU_SUBTYPE_ARM64_ALL\n";
       break;
+    case MachO::CPU_SUBTYPE_ARM64_V8:
+      outs() << "    cputype CPU_TYPE_ARM64\n";
+      outs() << "    cpusubtype CPU_SUBTYPE_ARM64_V8\n";
+      break;
     case MachO::CPU_SUBTYPE_ARM64E:
       outs() << "    cputype CPU_TYPE_ARM64\n";
       outs() << "    cpusubtype CPU_SUBTYPE_ARM64E\n";
       break;
     default:
       printUnknownCPUType(cputype, cpusubtype);
       break;
     }
case MachO::CPU_TYPE_ARM:
       }
       break;
     case MachO::CPU_TYPE_ARM64:
       outs() << "   ARM64";
       switch (cpusubtype & ~MachO::CPU_SUBTYPE_MASK) {
       case MachO::CPU_SUBTYPE_ARM64_ALL:
         outs() << "        ALL";
         break;
+      case MachO::CPU_SUBTYPE_ARM64_V8:
+        outs() << "         V8";
+        break;
       case MachO::CPU_SUBTYPE_ARM64E:
         outs() << "          E";
         break;
       default:
         outs() << format(" %10d", cpusubtype & ~MachO::CPU_SUBTYPE_MASK);
         break;
       }
       break;

llvm/tools/llvm-readobj/MachODumper.cpp

static const EnumEntry<uint32_t> MachOHeaderCpuSubtypesARM[] = {
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM_V7S),
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM_V7K),
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM_V6M),
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM_V7M),
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM_V7EM),
 };
 
 static const EnumEntry<uint32_t> MachOHeaderCpuSubtypesARM64[] = {
     LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM64_ALL),
+    LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM64_V8),
     LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_ARM64E),
 };
 
 static const EnumEntry<uint32_t> MachOHeaderCpuSubtypesSPARC[] = {
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_SPARC_ALL),
 };
 
 static const EnumEntry<uint32_t> MachOHeaderCpuSubtypesPPC[] = {
   LLVM_READOBJ_ENUM_ENT(MachO, CPU_SUBTYPE_POWERPC_ALL),