[ARM] Use searchable-table for banked registers

[javed.absar]

Summary

This is a continuation of https://reviews.llvm.org/D36219

This patch uses reverse mapping (encoding->name) in ARMInstPrinter::printBankedRegOperand to get rid of hard-coded values (as pointed out by @olista01).

[fhahn]

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

878 ↗

(On Diff #109491)

How about using assert(TheReg && "invalid banked register operand"); instead of the if + assert + return ? The original code seems to behave similar to the assert (no early return in the error case)

881 ↗

(On Diff #109491)

Why use a lowercase n instead of Name?

886 ↗

(On Diff #109491)

can we just use an assert here and get rid of the if?

[javed.absar]

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

886 ↗

(On Diff #109491)

I don't see how that would work since its just a conditional action and not a constraint

[javed.absar]

Revised based on review comments

[javed.absar]

ping!

[olista01]

LGTM with one nit.

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

880 ↗

(On Diff #109679)

This comment isn't needed any more.

[fhahn]

lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

882 ↗

(On Diff #109679)

Hm maybe we should rename R to isSPSR or something, to make that clearer or just get rid of the variable and move it into the condition (with a comment)

885 ↗

(On Diff #109679)

Should this be Name (declared on line 878?)

[fhahn]

Besides the outstanding comments, LGTM

llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp

if (Mask & 1)
       O << 'c';
   }
 }
 
 void ARMInstPrinter::printBankedRegOperand(const MCInst *MI, unsigned OpNum,
                                            const MCSubtargetInfo &STI,
                                            raw_ostream &O) {
   uint32_t Banked = MI->getOperand(OpNum).getImm();
-  uint32_t R = (Banked & 0x20) >> 5;
-  uint32_t SysM = Banked & 0x1f;
-
-  // Nothing much we can do about this, the encodings are specified in B9.2.3 of
-  // the ARM ARM v7C, and are all over the shop.
-  if (R) {
-    O << "SPSR_";
-
-    switch (SysM) {
-    case 0x0e:
-      O << "fiq";
-      return;
-    case 0x10:
-      O << "irq";
-      return;
-    case 0x12:
-      O << "svc";
-      return;
-    case 0x14:
-      O << "abt";
-      return;
-    case 0x16:
-      O << "und";
-      return;
-    case 0x1c:
-      O << "mon";
-      return;
-    case 0x1e:
-      O << "hyp";
-      return;
-    default:
-      llvm_unreachable("Invalid banked SPSR register");
-    }
-  }
-
-  assert(!R && "should have dealt with SPSR regs");
-  const char *RegNames[] = {
-      "r8_usr", "r9_usr", "r10_usr", "r11_usr", "r12_usr", "sp_usr",  "lr_usr",
-      "",       "r8_fiq", "r9_fiq",  "r10_fiq", "r11_fiq", "r12_fiq", "sp_fiq",
-      "lr_fiq", "",       "lr_irq",  "sp_irq",  "lr_svc",  "sp_svc",  "lr_abt",
-      "sp_abt", "lr_und", "sp_und",  "",        "",        "",        "",
-      "lr_mon", "sp_mon", "elr_hyp", "sp_hyp"};
-  const char *Name = RegNames[SysM];
-  assert(Name[0] && "invalid banked register operand");
-
+  auto TheReg = ARMBankedReg::lookupBankedRegByEncoding(Banked);
+  assert(TheReg && "invalid banked register operand");
+  std::string Name = TheReg->Name;
+
+  uint32_t isSPSR = (Banked & 0x20) >> 5;
+  if (isSPSR)
+    Name.replace(0, 4, "SPSR"); // convert 'spsr_' to 'SPSR_'
   O << Name;
 }
 
 void ARMInstPrinter::printPredicateOperand(const MCInst *MI, unsigned OpNum,
                                            const MCSubtargetInfo &STI,
                                            raw_ostream &O) {
   ARMCC::CondCodes CC = (ARMCC::CondCodes)MI->getOperand(OpNum).getImm();
   // Handle the undefined 15 CC value here for printing so we don't abort().