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

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 16 03:58:36 PST 2016

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).


More information about the llvm-commits mailing list