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.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test | ||
---|---|---|
1 ↗ | (On Diff #282295) | 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 |
+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? |
llvm/tools/llvm-objdump/MachODump.cpp | ||
---|---|---|
2108 | Under llvm-objdump, for CPU-TYPE CPU_TYPE_ARM64,
|
llvm/test/tools/llvm-objdump/MachO/AArch64/macho-arm64-v8.test | ||
---|---|---|
1 ↗ | (On Diff #282692) | 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 | ||
29–44 | 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. |
LGTM, with optional nit.
llvm/test/tools/llvm-objdump/MachO/universal-arm64.test | ||
---|---|---|
6 | Nit: as you only have one check prefix in this file, you can get rid of --check-prefix and just use the default CHECK |
Nit: as you only have one check prefix in this file, you can get rid of --check-prefix and just use the default CHECK