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

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h:428
+  void setRegBankData(unsigned ID, unsigned Size);
+  void setRegBankCoverage(unsigned ID, const uint32_t *CoveredClasses);
 
----------------
qcolombet wrote:
> 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.
Sure. The next patch merges them at the moment but I can move that bit to this patch


================
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) {
----------------
qcolombet wrote:
> Maybe keep the assert but with >=?
Ok


================
Comment at: lib/CodeGen/GlobalISel/RegisterBankInfo.cpp:188
+  RB.dump(&TRI);
+  llvm_unreachable("Please call setRegBankData() and setRegBankCoverage() instead");
+}
----------------
qcolombet wrote:
> Shouldn't we just delete this API instead?
That's done in the next patch. It's been kept in this patch so that any targets that aren't migrated by this patch yet will dump the information needed to migrate instead of just failing to link.


https://reviews.llvm.org/D27807