[PATCH] D21091: AArch64: refactor sysreg handling (new TableGen backend!)

[Tim Northover via llvm-commits]

t.p.northover added a comment.

Thanks for the comments, I've uploaded a new version of the patch incorporating most of them.


================
Comment at: lib/Target/AArch64/AArch64.td:135-140
@@ -134,2 +134,8 @@
 //===----------------------------------------------------------------------===//
+// Named operands for MRS/MSR/TLBI/...
+//===----------------------------------------------------------------------===//
+
+include "AArch64SystemOperands.td"
+
+//===----------------------------------------------------------------------===//
 // AArch64 Processors supported.
----------------
ab wrote:
> The file looks independent from the rest of the target; use a separate top-level .td?  CMake might make it tricky though..
Yes, I did try that originally but CMake really only wants one .td file per target. The issue becomes moot if move it to lib/Support or wherever.

================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:50-51
@@ +49,4 @@
+  std::string getName(Record *R) {
+    if (R->isValueUnset("Name"))
+      return R->getName();
+    return R->getValueAsString("Name");
----------------
ab wrote:
> Should this be an error?
Should probably be removed entirely. It's a relic from an earlier implementation.

================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:59-61
@@ +58,5 @@
+    else if (BitsInit *BI = dyn_cast<BitsInit>(I)) {
+      std::ostringstream OS;
+      OS << "0x" << std::hex << getAsInt(BI);
+      return OS.str();
+    }
----------------
ab wrote:
> utohexstr() ?
I thought it was weird we didn't have one!


Repository:
  rL LLVM

http://reviews.llvm.org/D21091