[PATCH] D27809: [globalisel] Move as much RegisterBank initialization to the constructor as possible

[Diana Picus via Phabricator via llvm-commits]

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

LGTM with microscopic nits.



================
Comment at: include/llvm/CodeGen/GlobalISel/RegisterBank.h:52
+
+  void finishInit(const TargetRegisterInfo &TRI);
 
----------------
It's probably worthwhile to add the comment explaining why this is necessary here (instead of only in the commit message).


================
Comment at: lib/CodeGen/GlobalISel/RegisterBank.cpp:25
+void RegisterBank::finishInit(const TargetRegisterInfo &TRI) {
+  // Populate ContainedRegClasses from the
+  uint32_t Mask[] = {(uint32_t)Data.CoveredClasses,
----------------
Incomplete comment :)


================
Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.cpp:45
+
+  for (auto &ID : {AArch64::GPRRegBankID, AArch64::FPRRegBankID, AArch64::CCRRegBankID})
+    getRegBank(ID).finishInit(TRI);
----------------
I think you should move the asserts checking that getRegBank returns the expected banks before this loop (I can see how this will make the loop a bit more "buried", but it seems like the proper thing to do).


https://reviews.llvm.org/D27809