[llvm-objdump] Implement `-Mreg-names-raw`/`-std` options.

[ikudrin]

Summary

The --disassembler-options, or -M, is used to customize the disassembler
and affect its output.

This patch implements two options which allow selecting names for registers
for ARM architecture. The -raw is what I came across in practice, and -std
is the same as the default name set.

Other options and architectures might be added later.

[evgeny777]

lib/Target/ARM/ARMRegisterInfo.td

16–17

Can we simply use "r" + string(Enc) for -Mreg-names-raw ?

[grimar]

lib/MC/MCInstPrinter.cpp

33 ↗

(On Diff #185025)

Maybe inline it?

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

83

You can avoid using else after return:

if (Opt == "reg-names-std") {
  DefaultAltIdx = ARM::NoRegAltName;
  return true;
} 
if (Opt == "reg-names-raw") {
  DefaultAltIdx = ARM::RegNamesRaw;
  return true;
} 
return false;

tools/llvm-objdump/llvm-objdump.cpp

1488

You do not need curly bracers here:

for (const auto &Opt : DisassemblerOptions)
  if (!IP->applyTargetSpecificCLOption(Opt))
    error("Unrecognized disassembler option: " + Opt);

I would also don't use auto because the type is not obvious.

[ikudrin]

Addressed review comments.

[ikudrin]

lib/Target/ARM/ARMRegisterInfo.td

16–17

I don't think I understand you. Could you explain where in the code you expect that line to be added?

[evgeny777]

lib/Target/ARM/ARMRegisterInfo.td

16–17

Well, this patch is actually needed for llvm-objdump/ARM to be compatible with GNU objdump and not much else.
So instead of trying to be smart I suggest implementing the subclass of ARMInstPrinter in llvm-objdump

class ARMObjdumpInstPrinter : public ARMInstPrinter {
  bool DumpRaw = false;

  const char *getRegisterNameRaw(unsigned RegNo);
public:
  void printRegName(raw_ostream &OS, unsigned RegNo) const override {
     if (!DumpRaw) {
        ARMInstPrinter::printRegName(OS, RegNo);
        return;
     }
     OS << markup("<reg:") << getRegisterNameRaw(RegNo) << markup(">");
  }
};

[ikudrin]

lib/Target/ARM/ARMRegisterInfo.td

16–17

Well, that could work. However, the instruction printer is created in a general way, through a common interface of targets. Moreover, the declaration of ARMInstrPrinter, laying inside the lib folders hierarchy, may be considered as an internal thing, and, as such, is not expected to be used outside the library.

[evgeny777]

lib/Target/ARM/ARMRegisterInfo.td

16–17

May be proxy MCInstPrinter? You only have to implement printInst and printRegName

class ObjdumpInstPrinter : public MCInstPrinter {
  MCInstPrinter *Real;
  bool DumpRawOnArm;
public:
   void printInst(...) override { return Real->printInst(...); }
   const char* printRegName(...) override {
      if (!isArm() || !DumpRawOnArm)
         return Real->printRegName(...);
      // do stuff ...
   }
};

[evgeny777]

lib/Target/ARM/ARMRegisterInfo.td

16–17

Nope, seems this won't work either as printRegName is called inside printInst

[ikudrin]

Ping.

[jhenderson]

include/llvm/MC/MCInstPrinter.h

68

This comment should not mention llvm-objdump. It might be the only user now, but there's nothing stopping other users in the future.

69

False -> false. Use the @returns syntax too, so that it gets sensibly formatted by doxygen.

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

83

There's still an else if a few lines up here, which can just become a regular if, as @grimar proposed.

test/tools/llvm-objdump/ARM/reg-names.s

8

May be worth testing what happens if you specify both (i.e. show last-one-wins).

tools/llvm-objdump/llvm-objdump.cpp

1485

StringRef?

[ikudrin]

• Made DefaultAltIdx private.
• Extended the test.
• Addressed other review comments.

[ikudrin]

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

83

Oops! Didn't notice that. Thanks!

[jhenderson]

test/tools/llvm-objdump/ARM/reg-names.s

8–9

This is a duplicate of the check immediately above?

tools/llvm-objdump/llvm-objdump.cpp

1487

Test case for this error message?

[ikudrin]

Updated the test.

[jhenderson]

LGTM, although I don't really know anything about the ARM register stuff at all. Perhaps worth getting a second person to review before landing.

[ikudrin]

Thanks! I'll wait a bit, but not very long. I believe the patch is safe because the default behavior is not changed.

include/llvm/MC/MCInstPrinter.h

protected:
   void printAnnotation(raw_ostream &OS, StringRef Annot);
 
 public:
   MCInstPrinter(const MCAsmInfo &mai, const MCInstrInfo &mii,
                 const MCRegisterInfo &mri) : MAI(mai), MII(mii), MRI(mri) {}
 
   virtual ~MCInstPrinter();
 
+  /// Customize the printer according to a command line option.
+  /// @return true if the option is recognized and applied.
+  virtual bool applyTargetSpecificCLOption(StringRef Opt) { return false; }
+
   /// Specify a stream to emit comments to.
   void setCommentStream(raw_ostream &OS) { CommentStream = &OS; }
 
   /// Print the specified MCInst to the specified raw_ostream.
   virtual void printInst(const MCInst *MI, raw_ostream &OS, StringRef Annot,
                          const MCSubtargetInfo &STI) = 0;
 
   /// Return the name of the specified opcode enum (e.g. "MOV32ri") or

include/llvm/Target/Target.td

class ComposedSubRegIndex<SubRegIndex A, SubRegIndex B>
   // See SubRegIndex.
   let ComposedOf = [A, B];
 }
 
 // RegAltNameIndex - The alternate name set to use for register operands of
 // this register class when printing.
 class RegAltNameIndex {
   string Namespace = "";
+
+  // A set to be used if the name for a register is not defined in this set.
+  // This allows creating name sets with only a few alternative names.
+  RegAltNameIndex FallbackRegAltNameIndex = ?;
 }
 def NoRegAltName : RegAltNameIndex;
 
 // Register - You should define one instance of this class for each register
 // in the target machine.  String n will become the "name" of the register.
 class Register<string n, list<string> altNames = []> {
   string Namespace = "";
   string AsmName = n;

lib/Target/ARM/ARMRegisterInfo.td

 //===-- ARMRegisterInfo.td - ARM Register defs -------------*- tablegen -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 include "ARMSystemRegister.td"
 
 //===----------------------------------------------------------------------===//
 //  Declarations that describe the ARM register file
 //===----------------------------------------------------------------------===//
 
 // Registers are identified with 4-bit ID numbers.
-class ARMReg<bits<16> Enc, string n, list<Register> subregs = []> : Register<n> {
+class ARMReg<bits<16> Enc, string n, list<Register> subregs = [],
+             list<string> altNames = []> : Register<n, altNames> {
   let HWEncoding = Enc;
   let Namespace = "ARM";
   let SubRegs = subregs;
   // All bits of ARM registers with sub-registers are covered by sub-registers.
   let CoveredBySubRegs = 1;
 }
 
 class ARMFReg<bits<16> Enc, string n> : Register<n> {
   let HWEncoding = Enc;
   let Namespace = "ARM";
 }
 
+let Namespace = "ARM",
+    FallbackRegAltNameIndex = NoRegAltName in {
+  def RegNamesRaw : RegAltNameIndex;
+}
+
 // Subregister indices.
 let Namespace = "ARM" in {
 def qqsub_0 : SubRegIndex<256>;
 def qqsub_1 : SubRegIndex<256, 256>;
 
 // Note: Code depends on these having consecutive numbers.
 def qsub_0 : SubRegIndex<128>;
 def qsub_1 : SubRegIndex<128, 128>;

 def R7  : ARMReg< 7, "r7">,  DwarfRegNum<[7]>;
 // These require 32-bit instructions.
 let CostPerUse = 1 in {
 def R8  : ARMReg< 8, "r8">,  DwarfRegNum<[8]>;
 def R9  : ARMReg< 9, "r9">,  DwarfRegNum<[9]>;
 def R10 : ARMReg<10, "r10">, DwarfRegNum<[10]>;
 def R11 : ARMReg<11, "r11">, DwarfRegNum<[11]>;
 def R12 : ARMReg<12, "r12">, DwarfRegNum<[12]>;
-def SP  : ARMReg<13, "sp">,  DwarfRegNum<[13]>;
-def LR  : ARMReg<14, "lr">,  DwarfRegNum<[14]>;
-def PC  : ARMReg<15, "pc">,  DwarfRegNum<[15]>;
+let RegAltNameIndices = [RegNamesRaw] in {
+def SP  : ARMReg<13, "sp", [], ["r13"]>,  DwarfRegNum<[13]>;
+def LR  : ARMReg<14, "lr", [], ["r14"]>,  DwarfRegNum<[14]>;
+def PC  : ARMReg<15, "pc", [], ["r15"]>,  DwarfRegNum<[15]>;
+}
 }
 
 // Float registers
 def S0  : ARMFReg< 0, "s0">;  def S1  : ARMFReg< 1, "s1">;
 def S2  : ARMFReg< 2, "s2">;  def S3  : ARMFReg< 3, "s3">;
 def S4  : ARMFReg< 4, "s4">;  def S5  : ARMFReg< 5, "s5">;
 def S6  : ARMFReg< 6, "s6">;  def S7  : ARMFReg< 7, "s7">;
 def S8  : ARMFReg< 8, "s8">;  def S9  : ARMFReg< 9, "s9">;

lib/Target/ARM/InstPrinter/ARMInstPrinter.h

 //===- ARMInstPrinter.h - Convert ARM MCInst to assembly syntax -*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // This class prints an ARM MCInst to a .s file.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_ARM_INSTPRINTER_ARMINSTPRINTER_H
 #define LLVM_LIB_TARGET_ARM_INSTPRINTER_ARMINSTPRINTER_H
 
+#include "MCTargetDesc/ARMMCTargetDesc.h"
 #include "llvm/MC/MCInstPrinter.h"
 
 namespace llvm {
 
 class ARMInstPrinter : public MCInstPrinter {
 public:
   ARMInstPrinter(const MCAsmInfo &MAI, const MCInstrInfo &MII,
                  const MCRegisterInfo &MRI);
 
+  bool applyTargetSpecificCLOption(StringRef Opt) override;
+
   void printInst(const MCInst *MI, raw_ostream &O, StringRef Annot,
                  const MCSubtargetInfo &STI) override;
   void printRegName(raw_ostream &OS, unsigned RegNo) const override;
 
   // Autogenerated by tblgen.
   void printInstruction(const MCInst *MI, const MCSubtargetInfo &STI,
                         raw_ostream &O);
   virtual bool printAliasInstr(const MCInst *MI, const MCSubtargetInfo &STI,
                                raw_ostream &O);
   virtual void printCustomAliasOperand(const MCInst *MI, unsigned OpIdx,
                                        unsigned PrintMethodIdx,
                                        const MCSubtargetInfo &STI,
                                        raw_ostream &O);
-  static const char *getRegisterName(unsigned RegNo);
+  static const char *getRegisterName(unsigned RegNo,
+                                     unsigned AltIdx = ARM::NoRegAltName);
 
   void printOperand(const MCInst *MI, unsigned OpNo, const MCSubtargetInfo &STI,
                     raw_ostream &O);
 
   void printSORegRegOperand(const MCInst *MI, unsigned OpNum,
                             const MCSubtargetInfo &STI, raw_ostream &O);
   void printSORegImmOperand(const MCInst *MI, unsigned OpNum,
                             const MCSubtargetInfo &STI, raw_ostream &O);
void printVectorListFourSpacedAllLanes(const MCInst *MI, unsigned OpNum,
                                          raw_ostream &O);
   void printVectorListThreeSpaced(const MCInst *MI, unsigned OpNum,
                                   const MCSubtargetInfo &STI, raw_ostream &O);
   void printVectorListFourSpaced(const MCInst *MI, unsigned OpNum,
                                  const MCSubtargetInfo &STI, raw_ostream &O);
   template<int64_t Angle, int64_t Remainder>
   void printComplexRotationOp(const MCInst *MI, unsigned OpNum,
                               const MCSubtargetInfo &STI, raw_ostream &O);
+
+private:
+  unsigned DefaultAltIdx = ARM::NoRegAltName;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_ARM_INSTPRINTER_ARMINSTPRINTER_H

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

if (UseMarkup)
       O << ">";
   }
 }
 
 ARMInstPrinter::ARMInstPrinter(const MCAsmInfo &MAI, const MCInstrInfo &MII,
                                const MCRegisterInfo &MRI)
     : MCInstPrinter(MAI, MII, MRI) {}
 
+bool ARMInstPrinter::applyTargetSpecificCLOption(StringRef Opt) {
+  if (Opt == "reg-names-std") {
+    DefaultAltIdx = ARM::NoRegAltName;
+    return true;
+  }
+  if (Opt == "reg-names-raw") {
+    DefaultAltIdx = ARM::RegNamesRaw;
+    return true;
+  }
+  return false;
+}
+
 void ARMInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
-  OS << markup("<reg:") << getRegisterName(RegNo) << markup(">");
+  OS << markup("<reg:") << getRegisterName(RegNo, DefaultAltIdx) << markup(">");
 }
 
 void ARMInstPrinter::printInst(const MCInst *MI, raw_ostream &O,
                                StringRef Annot, const MCSubtargetInfo &STI) {
   unsigned Opcode = MI->getOpcode();
 
   switch (Opcode) {
   // Check for MOVs and print canonical forms, instead.

test/tools/llvm-objdump/ARM/reg-names.s

+@ RUN: llvm-mc %s -triple armv5-unknown-linux -filetype=obj -o %t
+@ RUN: llvm-objdump -d %t | FileCheck -check-prefix=STD %s
+@ RUN: llvm-objdump -d -Mreg-names-std %t \
+@ RUN:   | FileCheck -check-prefix=STD %s
+@ RUN: llvm-objdump -d --disassembler-options=reg-names-raw %t \
+@ RUN:   | FileCheck -check-prefix=RAW %s
+@ RUN: llvm-objdump -d -Mreg-names-raw,reg-names-std %t \
+@ RUN:   | FileCheck -check-prefix=STD %s
+@ RUN: llvm-objdump -d -Mreg-names-std,reg-names-raw %t \
+@ RUN:   | FileCheck -check-prefix=RAW %s
+@ RUN: not llvm-objdump -d -Munknown %t 2>&1 \
+@ RUN:   | FileCheck -check-prefix=ERR %s
+@ ERR: Unrecognized disassembler option: unknown
+
+.text
+  add r13, r14, r15
+@ STD: add sp, lr, pc
+@ RAW: add r13, r14, r15

tools/llvm-objdump/llvm-objdump.cpp

 cl::opt<bool> DisassembleZeroes(
                 "disassemble-zeroes",
                 cl::desc("Do not skip blocks of zeroes when disassembling"));
 cl::alias DisassembleZeroesShort("z",
                                  cl::desc("Alias for --disassemble-zeroes"),
                                  cl::NotHidden, cl::Grouping,
                                  cl::aliasopt(DisassembleZeroes));
 
+static cl::list<std::string>
+    DisassemblerOptions("disassembler-options",
+                        cl::desc("Pass target specific disassembler options"),
+                        cl::value_desc("options"), cl::CommaSeparated);
+static cl::alias
+    DisassemblerOptionsShort("M", cl::desc("Alias for --disassembler-options"),
+                             cl::NotHidden, cl::Prefix, cl::CommaSeparated,
+                             cl::aliasopt(DisassemblerOptions));
+
 static StringRef ToolName;
 
 typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;
 
 SectionFilter llvm::ToolSectionFilter(llvm::object::ObjectFile const &O) {
   return SectionFilter(
       [](llvm::object::SectionRef const &S) {
         if (FilterSections.empty())
static void disassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
   if (!IP)
     report_error(Obj->getFileName(),
                  "no instruction printer for target " + TripleName);
   IP->setPrintImmHex(PrintImmHex);
 
   PrettyPrinter &PIP = selectPrettyPrinter(Triple(TripleName));
   SourcePrinter SP(Obj, TheTarget->getName());
 
+  for (StringRef Opt : DisassemblerOptions)
+    if (!IP->applyTargetSpecificCLOption(Opt))
+      error("Unrecognized disassembler option: " + Opt);
+
   disassembleObject(TheTarget, Obj, Ctx, DisAsm.get(), MIA.get(), IP.get(),
                     STI.get(), PIP, SP, InlineRelocs);
 }
 
 void llvm::printRelocations(const ObjectFile *Obj) {
   StringRef Fmt = Obj->getBytesInAddress() > 4 ? "%016" PRIx64 :
                                                  "%08" PRIx64;
   // Regular objdump doesn't print relocations in non-relocatable object

utils/TableGen/AsmWriterEmitter.cpp

void AsmWriterEmitter::EmitGetRegisterName(raw_ostream &O) {
   if (hasAltNames) {
     O << "  switch(AltIdx) {\n"
       << "  default: llvm_unreachable(\"Invalid register alt name index!\");\n";
     for (const Record *R : AltNameIndices) {
       StringRef AltName = R->getName();
       O << "  case ";
       if (!Namespace.empty())
         O << Namespace << "::";
-      O << AltName << ":\n"
-        << "    assert(*(AsmStrs" << AltName << "+RegAsmOffset" << AltName
+      O << AltName << ":\n";
+      if (R->isValueUnset("FallbackRegAltNameIndex"))
+        O << "    assert(*(AsmStrs" << AltName << "+RegAsmOffset" << AltName
           << "[RegNo-1]) &&\n"
-        << "           \"Invalid alt name index for register!\");\n"
-        << "    return AsmStrs" << AltName << "+RegAsmOffset" << AltName
+          << "           \"Invalid alt name index for register!\");\n";
+      else {
+        O << "    if (!*(AsmStrs" << AltName << "+RegAsmOffset" << AltName
+          << "[RegNo-1]))\n"
+          << "      return getRegisterName(RegNo, ";
+        if (!Namespace.empty())
+          O << Namespace << "::";
+        O << R->getValueAsDef("FallbackRegAltNameIndex")->getName() << ");\n";
+      }
+      O << "    return AsmStrs" << AltName << "+RegAsmOffset" << AltName
         << "[RegNo-1];\n";
     }
     O << "  }\n";
   } else {
     O << "  assert (*(AsmStrs+RegAsmOffset[RegNo-1]) &&\n"
       << "          \"Invalid alt name index for register!\");\n"
       << "  return AsmStrs+RegAsmOffset[RegNo-1];\n";
   }