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

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 added inline comments.


================
Comment at: lib/Target/ARM/ARMRegisterInfo.td:16
 // 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> {
----------------
evgeny777 wrote:
> ikudrin wrote:
> > evgeny777 wrote:
> > > ikudrin wrote:
> > > > evgeny777 wrote:
> > > > > Can we simply use "r" + string(Enc) for `-Mreg-names-raw` ?
> > > > I don't think I understand you. Could you explain where in the code you expect that line to be added?
> > > 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(">");
> > >   }
> > > };
> > > ```
> > 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.
> 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 ...
>    }
> };
> ```
Nope, seems this won't work either as printRegName is called inside printInst


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57680/new/

https://reviews.llvm.org/D57680