[PATCH] D36260: [ARM] Use searchable-table for banked registers

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp:878
+  if (!TheReg) {
+    assert(false && "invalid banked register operand");
+    return;
----------------
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)


================
Comment at: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp:881
+  }
+  std::string name = TheReg->Name;
 
----------------
Why use a lowercase n instead of Name?


================
Comment at: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp:886
+  uint32_t R = (Banked & 0x20) >> 5;
+  if (R)
+    name.replace(0, 4, "SPSR"); // convert 'spsr_' to 'SPSR_'
----------------
can we just use an assert here and get rid of the `if`?


https://reviews.llvm.org/D36260