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

[Ahmed Bougacha via llvm-commits]

ab accepted this revision.
ab added a reviewer: ab.
ab added a comment.
This revision is now accepted and ready to land.

A few final nitpicks. Do you want to consider the lib/Support move separately?  If so, LGTM.


================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:60
@@ +59,3 @@
+    }
+    llvm_unreachable("invalid field type, expected: string, bits, bit or code");
+  }
----------------
PrintFatalError?  Here and elsewhere.

================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:232
@@ +231,3 @@
+                                         raw_ostream &OS) {
+  std::string TableName = InstanceClass->getName();
+  std::vector<Record *> Items = Records.getAllDerivedDefinitions(TableName);
----------------
const&? Or is the copy intentional?

================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:253
@@ +252,3 @@
+
+  for (auto Field : *InstanceClass->getValueAsListInit("SearchableFields")) {
+    SearchTables.emplace_back();
----------------
auto *?

Also, I'm not a huge fan of Almost Always Auto, so I'd just use Record*, but either is fine.

================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:259
@@ +258,3 @@
+  int Idx = 0;
+  for (auto Item : Items) {
+    for (unsigned i = 0; i < SearchFieldNames.size(); ++i) {
----------------
auto *?

================
Comment at: utils/TableGen/SearchableTableEmitter.cpp:306
@@ +305,3 @@
+    if (Class->getSuperClasses().size() != 1 ||
+        !Class->isSubClassOf("SearchableTable"))
+      continue;
----------------
Hoist out RK.getClass("SearchableTable") and use isSubClassOf(Record*) ?


Repository:
  rL LLVM

http://reviews.llvm.org/D21091