[llvm-objdump] Add -M {att,intel} & deprecate --x86-asm-syntax={att,intel}

[MaskRay]

Summary

The internal cl::opt option --x86-asm-syntax sets the AsmParser and AsmWriter
dialect. The option is used by llc and llvm-mc tests to set the AsmWriter dialect.

This patch adds -M {att,intel} as GNU objdump compatible aliases (PR43413).

Note: the dialect is initialized when the MCAsmInfo is constructed.
MCInstPrinter::applyTargetSpecificCLOption is called too late and its MCAsmInfo
reference is const, so changing the cl::opt in
MCInstPrinter::applyTargetSpecificCLOption is not an option, at least without
large amount of refactoring.

[thakis]

Thanks!

As mentioned in D101761, consider using the code bits there (especially if the plan is to remove the old-style options after the next branch).

llvm/test/tools/llvm-objdump/X86/syntax-mode.s

5

Maybe spell it -dMIntel here to cover that spelling

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

equals_lower() for better objdump compat?

2472

(same)

[hoy]

Thanks for working on this.

llvm/docs/CommandGuide/llvm-objdump.rst

153

Do these RISC-V switches just work? Wondering if testing is needed.

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

Can this be made an alias switch of OBJDUMP_x86_asm_syntax_att?

[MaskRay]

Add release note.
Improve code

[MaskRay]

llvm/docs/CommandGuide/llvm-objdump.rst

153

It just works (D66139), but ideally there should be test/tools/llvm-objdump specific tests.

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

I think GNU objdump uses startswith for case-sensitive "intel" and "att" and ignores unrecognized values.

I don't think anyone wants to use "intel*" (other than the unimplemented "intel-mnenomic" so I'll just use rigid == here.

2468

Can this be made an alias switch of OBJDUMP_x86_asm_syntax_att?

No. llvm-objdump --x86-asm-syntax will retire.

[hoy]

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

Or the other way around? The AsmSyntax assignments looks a bit redundant. It's fine if you are going to remove the OBJDUMP_x86_asm cases later.

[MaskRay]

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

No, it is needed to avoid duplicate --x86-asm-syntax.

[jhenderson]

llvm/docs/CommandGuide/llvm-objdump.rst

249

If this option is to be deprecated, we should update this part of the docs one way or another. What's the motivation to remove it though? We have many other cases of keeping older option names around in the LLVM tools. I'm not opposed to it. Just trying to understand the thought process.

(For the record, I don't think we have any internal users of this option - I'll make sure this is looked at downstream)

llvm/test/tools/llvm-objdump/X86/syntax-mode.s

2

Perhaps also worth the long-form version of the option (i.e. --disassembler-options)? With and without the =?

[hoy]

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

I'm confused. What do you mean by "--x86-asm-syntax will retire."? Are OBJDUMP_x86_asm_syntax_att and OBJDUMP_x86_asm_syntax_intel going away?

[MaskRay]

comments

[MaskRay]

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

llvm-objdump --x86-asm-syntax= will go away.

x86-asm-syntax is an internal cl::opt option, not intended to be exposed. -M intel will work by setting the internal x86-asm-syntax option.

[hoy]

Current change LGTM. More thoughts may be needed on retiring the --x86-asm-syntax= options which may break downstream usage.

llvm/tools/llvm-objdump/llvm-objdump.cpp

2468

Thanks for the clarification.

[jhenderson]

LGTM. Awaiting confirmation on likely lack of downstream usage, but I think even if there is no usage downstream, it should be easy enough to migrate over, given a period of at least 1 release between deprecation and removal.

[MaskRay]

rebase

llvm/docs/CommandGuide/llvm-objdump.rst

 
 .. option:: -l, --line-numbers
 
   When disassembling, display source line numbers. Implies
   :option:`--disassemble`.
 
 .. option:: -M, --disassembler-options=<opt1[,opt2,...]>
 
-  Pass target-specific disassembler options. Currently supported for ARM targets
-  only. Available options are ``reg-names-std`` and ``reg-names-raw``.
+  Pass target-specific disassembler options. Available options:
+
+  * ``reg-names-std``: ARM only (default). Print in ARM 's instruction set documentation, with r13/r14/r15 replaced by sp/lr/pc.
+  * ``reg-names-raw``: ARM only. Use r followed by the register number.
+  * ``no-aliases``: RISC-V only. Print raw instruction mnemonic instead of pesudo instruction mnemonic.
+  * ``numeric``: RISC-V only. Print raw register names instead of ABI mnemonic. (e.g. print x1 instead of ra)
+  * ``att``: x86 only (default). Print in the AT&T syntax.
+  * ``intel``: x86 only. Print in the intel syntax.
 
 .. option:: --mcpu=<cpu-name>
 
   Target a specific CPU type for disassembly. Specify ``--mcpu=help`` to display
   available CPUs.
 
 .. option:: --mattr=<a1,+a2,-a3,...>
 

 .. option:: --triple=<string>
 
   Target triple to disassemble for, see ``--version`` for available targets.
 
 .. option:: -w, --wide
 
   Ignored for compatibility with GNU objdump.
 
 .. option:: --x86-asm-syntax=<style>
 
+  Deprecated.
   When used with :option:`--disassemble`, choose style of code to emit from
   X86 backend. Supported values are:
 
    .. option:: att
 
     AT&T-style assembly
 
    .. option:: intel

llvm/docs/ReleaseNotes.rst

 ---------------------------------
 
 * The options ``--build-id-link-{dir,input,output}`` have been deleted.
   (`D96310 <https://reviews.llvm.org/D96310>`_)
 
 * Support for in-order processors has been added to ``llvm-mca``.
   (`D94928 <https://reviews.llvm.org/D94928>`_)
 
+* llvm-objdump supports ``-M {att,intel}`` now.
+  ``--x86-asm-syntax`` is a deprecated internal option which will be removed in LLVM 14.0.0.
+  (`D101695 <https://reviews.llvm.org/D101695>`_)
+
 Changes to LLDB
 ---------------------------------
 
 Changes to Sanitizers
 ---------------------
 
 External Open Source Projects Using LLVM 13
 ===========================================

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

 # RUN: yaml2obj %s -o %t
-# RUN: llvm-objdump %t -d --symbolize-operands --x86-asm-syntax=intel --no-show-raw-insn --no-leading-addr | \
+# RUN: llvm-objdump %t -d --symbolize-operands -M intel --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --match-full-lines --check-prefix=INTEL
-# RUN: llvm-objdump %t -d --symbolize-operands --x86-asm-syntax=att --no-show-raw-insn --no-leading-addr | \
+# RUN: llvm-objdump %t -d --symbolize-operands -M att --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --match-full-lines --check-prefix=ATT
 
 ## Expect to find the branch labels and global variable name.
 # ATT:      <_start>:
 # ATT-NEXT:   pushq %rax
 # ATT-NEXT: <L1>:
 # ATT-NEXT:   cmpl  , %eax <symbol>
 # ATT-NEXT:   jge    <L0>

llvm/test/tools/llvm-objdump/X86/syntax-mode.s

+## Test att and intel syntax modes.
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
+# RUN: llvm-objdump -d %t | FileCheck %s --check-prefix=ATT
+# RUN: llvm-objdump -d -M att %t | FileCheck %s --check-prefix=ATT
+# RUN: llvm-objdump -dMintel %t | FileCheck %s --check-prefix=INTEL
+# RUN: llvm-objdump -d --disassembler-options=intel %t | FileCheck %s --check-prefix=INTEL
+
+## The last wins.
+# RUN: llvm-objdump -dM att -M att,intel %t | FileCheck %s --check-prefix=INTEL
+
+## Test discouraged internal cl::opt options.
+# RUN: llvm-objdump -d --x86-asm-syntax=att %t | FileCheck %s --check-prefix=ATT
+# RUN: llvm-objdump -d --x86-asm-syntax=intel %t | FileCheck %s --check-prefix=INTEL
+
+# ATT: movw $1, %ax
+# ATT: imull %esi, %edi
+# ATT: leaq 5(%rsi,%rdi,4), %rax
+
+# INTEL: mov ax, 1
+# INTEL: imul edi, esi
+# INTEL: lea rax, [rsi + 4*rdi + 5]
+
+  movw $1, %ax
+  imull %esi, %edi
+  leaq 5(%rsi,%rdi,4), %rax

llvm/tools/llvm-objdump/llvm-objdump.cpp

static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   ArchiveHeaders = InputArgs.hasArg(OBJDUMP_archive_headers);
   Demangle = InputArgs.hasArg(OBJDUMP_demangle);
   Disassemble = InputArgs.hasArg(OBJDUMP_disassemble);
   DisassembleAll = InputArgs.hasArg(OBJDUMP_disassemble_all);
   SymbolDescription = InputArgs.hasArg(OBJDUMP_symbol_description);
   DisassembleSymbols =
       commaSeparatedValues(InputArgs, OBJDUMP_disassemble_symbols_EQ);
   DisassembleZeroes = InputArgs.hasArg(OBJDUMP_disassemble_zeroes);
-  DisassemblerOptions =
-      commaSeparatedValues(InputArgs, OBJDUMP_disassembler_options_EQ);
   if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_dwarf_EQ)) {
     DwarfDumpType =
         StringSwitch<DIDumpType>(A->getValue()).Case("frames", DIDT_DebugFrame);
   }
   DynamicRelocations = InputArgs.hasArg(OBJDUMP_dynamic_reloc);
   FaultMapSection = InputArgs.hasArg(OBJDUMP_fault_map_section);
   FileHeaders = InputArgs.hasArg(OBJDUMP_file_headers);
   SectionContents = InputArgs.hasArg(OBJDUMP_full_contents);
if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_debug_vars_EQ)) {
     DbgVariables = StringSwitch<DebugVarsFormat>(A->getValue())
                        .Case("ascii", DVASCII)
                        .Case("unicode", DVUnicode);
   }
   parseIntArg(InputArgs, OBJDUMP_debug_vars_indent_EQ, DbgIndent);
 
   parseMachOOptions(InputArgs);
 
-  // Handle options that get forwarded to cl::opt<>s in libraries.
-  // FIXME: Depending on https://reviews.llvm.org/D84191#inline-946075 ,
-  // hopefully remove this again.
-  std::vector<const char *> LLVMArgs;
-  LLVMArgs.push_back("llvm-objdump (LLVM option parsing)");
-  if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_x86_asm_syntax_att,
-                                               OBJDUMP_x86_asm_syntax_intel)) {
+  // Parse -M (--disassembler-options) and deprecated
+  // --x86-asm-syntax={att,intel}.
+  //
+  // Note, for x86, the asm dialect (AssemblerDialect) is initialized when the
+  // MCAsmInfo is constructed. MCInstPrinter::applyTargetSpecificCLOption is
+  // called too late. For now we have to use the internal cl::opt option.
+  const char *AsmSyntax = nullptr;
+  for (const auto *A : InputArgs.filtered(OBJDUMP_disassembler_options_EQ,
+                                          OBJDUMP_x86_asm_syntax_att,
     switch (A->getOption().getID()) {
     case OBJDUMP_x86_asm_syntax_att:
-      LLVMArgs.push_back("--x86-asm-syntax=att");
-      break;
+      AsmSyntax = "--x86-asm-syntax=att";
+      continue;
     case OBJDUMP_x86_asm_syntax_intel:
-      LLVMArgs.push_back("--x86-asm-syntax=intel");
-      break;
-    }
+      AsmSyntax = "--x86-asm-syntax=intel";
+      continue;
+    }
+
+    SmallVector<StringRef, 2> Values;
+    llvm::SplitString(A->getValue(), Values, ",");
+    for (StringRef V : Values) {
+      if (V == "att")
+        AsmSyntax = "--x86-asm-syntax=att";
+      else if (V == "intel")
+        AsmSyntax = "--x86-asm-syntax=intel";
+      else
+        DisassemblerOptions.push_back(V.str());
+    }
+  if (AsmSyntax) {
+    const char *Argv[] = {"llvm-objdump", AsmSyntax};
+    llvm::cl::ParseCommandLineOptions(2, Argv);
   }
-  LLVMArgs.push_back(nullptr);
-  llvm::cl::ParseCommandLineOptions(LLVMArgs.size() - 1, LLVMArgs.data());
 
   // objdump defaults to a.out if no filenames specified.
   if (InputFilenames.empty())
     InputFilenames.push_back("a.out");
 }
 
 int main(int argc, char **argv) {
   using namespace llvm;