[PATCH] D27807: [globalisel] Initialize RegisterBanks with static data. Part 1 of 2.

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

Hi Daniel,

LGTM with a couple of nitpicks.

No need for another room of review.

Cheers,
-Quentin



================
Comment at: include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h:428
+  void setRegBankData(unsigned ID, unsigned Size);
+  void setRegBankCoverage(unsigned ID, const uint32_t *CoveredClasses);
 
----------------
Could we merge these two methods?

Also, add a comment explaining the format of CoveredClasses and the effect on RegBank with ID. E.g., \post getRegBank(\p ID).getSize() == Size, etc.


================
Comment at: lib/CodeGen/GlobalISel/RegisterBank.cpp:27
-  assert(ContainedRegClasses.size() == TRI.getNumRegClasses() &&
-         "TRI does not match the initialization process?");
   for (unsigned RCId = 0, End = TRI.getNumRegClasses(); RCId != End; ++RCId) {
----------------
Maybe keep the assert but with >=?


================
Comment at: lib/CodeGen/GlobalISel/RegisterBankInfo.cpp:188
+  RB.dump(&TRI);
+  llvm_unreachable("Please call setRegBankData() and setRegBankCoverage() instead");
+}
----------------
Shouldn't we just delete this API instead?


https://reviews.llvm.org/D27807