[PATCH] D27338: [globalisel] Tablegen-erate current Register Bank Information

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.h:26
 
-namespace AArch64 {
-enum {
-  GPRRegBankID = 0, /// General Purpose Registers: W, X.
-  FPRRegBankID = 1, /// Floating Point/Vector Registers: B, H, S, D, Q.
-  CCRRegBankID = 2, /// Conditional register: NZCV.
-  NumRegisterBanks
-};
-} // End AArch64 namespace.
-
 class AArch64GenRegisterBankInfo : public RegisterBankInfo {
 private:
----------------
ab wrote:
> qcolombet wrote:
> > dsanders wrote:
> > > qcolombet wrote:
> > > > This shouldn't be "AArch64GenRegisterBankInfo : public RegisterBankInfo"
> > > > But "AArch64RegisterBankInfo : public AArch64GenRegisterBankInfo"
> > > No, this is the declaration of AArch64GenRegisterBankInfo which will eventually be fully tablegen-erated. AArch64RegisterBankInfo is declared next (line 95 at the moment) as `class AArch64RegisterBankInfo final : public AArch64GenRegisterBankInfo`.
> > Ah right, I missed the second declaration.
> So, here's an alternative approach (more in line with what Quentin and I were thinking, I suspect):  from day one, generate the entire AArch64GenRegisterBankInfo class.  If something currently needs to be manually written code, keep it in AArch64RegisterBankInfo.  When tablegen gains support for it, move it from the subclass to the generated base.  Because AArch64RegisterBankInfo is-a AArch64GenRegisterBankInfo, all of these changes should be NFC.  WDYT?
Is there an advantage to re-factoring the patch series that way? It seems equal to the current patch series to me in which case I'm leaning towards the current patches.


https://reviews.llvm.org/D27338