[llvm-objdump] Default to the "apple-latest" mcpu when disassembling arm64 Mach-O binaries

[mtrent]

Summary

This changes the Mach-O dumper to use the "apple-latest" Mcpu value
if one isn't specified by the command-line arguments or by the arch
triple.

[MaskRay]

Can you clarify whether the intention is:

• For MachOObjectFile::getArchTriple, we want to use a conservative arch flag, because it may be used by both producers and consumers. We don't want to impose an arch feature restriction for binaries created by the producers.
• For llvm-objdump and may be llvm-nm/llvm-size, the read-only consumers, we want to use the latest arch flag so that all features can be decoded correctly.

[MaskRay]

In GNU objdump, -m is another option which takes an argument.

-m machine
 --architecture=machine
     Specify the architecture to use when disassembling object files.  This can be useful when disassembling object files which do not
     describe architecture information, such as S-records.  You can list the available architectures with the -i option.

     If the target is an ARM architecture then this switch has an additional effect.  It restricts the disassembly to only those
     instructions supported by the architecture specified by machine.  If it is necessary to use this switch because the input file does not
     contain any architecture information, but it is also desired to disassemble all the instructions use -marm.

Would it be possible to remove the -m alias for --macho?

llvm/test/tools/llvm-objdump/MachO/apple-latest-default.s

2

Add a comment about the purpose of the test.

## I would prefer comments starting with ##
# RUN: # is for RUN and CHECK lines
# CHECK: ...

Delete -show-encoding which is a no-op with the object output (-filetype=obj)

Prefer --macho to -m. It looks strange to me that without --macho, the instruction cannot be decoded (<unknown>). I did not expect the --macho flag should matter here.

Delete 2> %t because stderr is not needed by the test.

[mtrent]

In D78032#1978299, @MaskRay wrote:

In GNU objdump, -m is another option which takes an argument.

-m machine
 --architecture=machine
     Specify the architecture to use when disassembling object files.  This can be useful when disassembling object files which do not
     describe architecture information, such as S-records.  You can list the available architectures with the -i option.

     If the target is an ARM architecture then this switch has an additional effect.  It restricts the disassembly to only those
     instructions supported by the architecture specified by machine.  If it is necessary to use this switch because the input file does not
     contain any architecture information, but it is also desired to disassemble all the instructions use -marm.

Would it be possible to remove the -m alias for --macho?

I don't know. Maybe the time has come to remove otool from llvm and just let llvm be a copy of the gnu binutils. Feels like a topic for another thread.

[mtrent]

In D78032#1978249, @MaskRay wrote:

Can you clarify whether the intention is:

• For MachOObjectFile::getArchTriple, we want to use a conservative arch flag, because it may be used by both producers and consumers. We don't want to impose an arch feature restriction for binaries created by the producers.
• For llvm-objdump and may be llvm-nm/llvm-size, the read-only consumers, we want to use the latest arch flag so that all features can be decoded correctly.

I don't understand the question?

[MaskRay]

In D78032#1978339, @mtrent wrote:

In D78032#1978249, @MaskRay wrote:

Can you clarify whether the intention is:

• For MachOObjectFile::getArchTriple, we want to use a conservative arch flag, because it may be used by both producers and consumers. We don't want to impose an arch feature restriction for binaries created by the producers.
• For llvm-objdump and may be llvm-nm/llvm-size, the read-only consumers, we want to use the latest arch flag so that all features can be decoded correctly.

I don't understand the question?

Is it possible to make the (defaulting mcpu to "apple-latest") decision in a library, instead of in llvm-objdump and other individual tools?

[mtrent]

In D78032#1978566, @MaskRay wrote:

In D78032#1978339, @mtrent wrote:

In D78032#1978249, @MaskRay wrote:

Can you clarify whether the intention is:

• For MachOObjectFile::getArchTriple, we want to use a conservative arch flag, because it may be used by both producers and consumers. We don't want to impose an arch feature restriction for binaries created by the producers.
• For llvm-objdump and may be llvm-nm/llvm-size, the read-only consumers, we want to use the latest arch flag so that all features can be decoded correctly.

I don't understand the question?

Is it possible to make the (defaulting mcpu to "apple-latest") decision in a library, instead of in llvm-objdump and other individual tools?

Anything is possible, it's only software.

The request from Ahmed and others who work on the library code team was to do this in the tools. I think if they thought making this change that the library level was the right thing do to they would have done it themselves when they added "apple-latest" to llvm.org.

I can also make the change upstream from here, doing it in Apple's cctools otool interface. But people who use llvm-objdump to disassemble MachO binaries directly will find this useful, given that that the triple doesn't always infer the Mcpu option.

Ahmed, do you have anything to add here?

[mtrent]

Add a comment about the purpose of the test.

Reformat the test to use octothorpes instead of slashes.

Replace -m and -d with --macho and --disassemble

Removed 2> %t

clang-formatted MachODump.cpp

[ab]

Is it possible to make the (defaulting mcpu to "apple-latest") decision in a library, instead of in llvm-objdump and other individual tools?

Yes, that sounds reasonable. I'm curious, what tools do you have in mind?
The obvious (only?) candidate is MachOObjectFile::getArchTriple, as you mentioned, we can't really distinguish assemblers from disassemblers in the lower layers (other than MCDisassember obviously, but that's already too late). I think the mcpu inferred by getArchTriple is only used by MachODump. And to answer the other questions: llvm-nm and llvm-size shouldn't care about CPUs. lldb is another similar consumer that does care, and I don't think that uses the MachOObjectFile API either.

So picking apple-latest in getArchTriple should be NFC relative to this patch I think, but it sounds reasonable to me. I'm a bit uneasy about having that in MachOObjectFile, which isn't specific to disassembly, but I don't have an argument against it either.

[lhames]

I would prefer to keep this change in the tool for now (as this patch does).

For disassembly purposes it makes sense to assume the newest CPU in case the object uses instructions from it. As a MachOObject file client on the other hand I would have expected getArchTriple to be conservative (i.e. "this object uses / requires the CPU specified"). I'm open to changing / reinterpreting getArchTriple's behavior, but I think that's a separate discussion from this patch and shouldn't block this going in.

[lhames]

This looks good to me. Unless anyone else has any other feedback I think this should go in, minus the whole-file reformatting.

llvm/test/tools/llvm-objdump/MachO/AArch64/macho-print-mrs.test

-RUN: llvm-objdump -d -m --no-show-raw-insn %p/Inputs/print-mrs.obj.macho-aarch64 | FileCheck %s
+RUN: llvm-objdump -d -m --no-show-raw-insn --mcpu=cyclone %p/Inputs/print-mrs.obj.macho-aarch64 | FileCheck %s
 
 CHECK: 0:  mrs x0, CPM_IOACC_CTL_EL3

llvm/test/tools/llvm-objdump/MachO/apple-latest-default.s

+// RUN: llvm-mc -triple arm64-apple-ios -mattr=+apple-a13 -show-encoding \
+// RUN: -filetype=obj %s -o - 2> %t | llvm-objdump -m -d - | FileCheck %s
+
+fmlal  V0.2s, v1.2h, v2.h[0]
+
+// CHECK: (__TEXT,__text) section
+// CHECK:       0:  20 00 82 0f fmlal v0, v1, v2[0]

llvm/tools/llvm-objdump/MachODump.cpp

 //===-- MachODump.cpp - Object file dumping utility for llvm --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
static const Target *GetTarget(const MachOObjectFile *MachOObj,
                                const Target **ThumbTarget) {
   // Figure out the target triple.
   Triple TT(TripleName);
   if (TripleName.empty()) {
     TT = MachOObj->getArchTriple(McpuDefault);
     TripleName = TT.str();
   }
 
-  if (TT.getArch() == Triple::arm) {
+  Triple::ArchType arch = TT.getArch();
+  if (arch == Triple::arm) {
     // We've inferred a 32-bit ARM target from the object file. All MachO CPUs
     // that support ARM are also capable of Thumb mode.
     Triple ThumbTriple = TT;
     std::string ThumbName = (Twine("thumb") + TT.getArchName().substr(3)).str();
     ThumbTriple.setArchName(ThumbName);
     ThumbTripleName = ThumbTriple.str();
   }
+  else if (arch == Triple::aarch64 || 
+           arch == Triple::aarch64_32) {
+    // We've inferred a 64-bit ARM target. Return "apple-latest" for the
+    // McpuDefault.
+    if (*McpuDefault)
+      *McpuDefault = "apple-latest";
+  }
 
   // Get the target specific parser.
   std::string Error;
   const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
   if (TheTarget && ThumbTripleName.empty())
     return TheTarget;
 
   *ThumbTarget = TargetRegistry::lookupTarget(ThumbTripleName, Error);