[PATCH] D27976: [aarch64][globalisel] Move data into <Target>GenRegisterBankInfo. NFC.

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: lib/Target/AArch64/AArch64GenRegisterBankInfo.def:99
+RegisterBank AArch64GenRegisterBankInfo::FPRRegBank(AArch64GenRegisterBankInfo::RegisterBankData[AArch64::FPRRegBankID]);
+RegisterBank AArch64GenRegisterBankInfo::CCRRegBank(AArch64GenRegisterBankInfo::RegisterBankData[AArch64::CCRRegBankID]);
 
----------------
dsanders wrote:
> qcolombet wrote:
> > I would rather have the register banks in the AArch64 namespace. I expect we will use them directly, and having to specify AArch64GenRegisterBankInfo seems painful.
> Hmm, I can move it to llvm::AArch64 but I have to either hard-code the number of register classes and eliminate finishInit() or allow these variables to be in a partially-initialized state until the TargetRegisterInfo exists and something (currently <Target>RegisterBankInfo) can finish the initialization. If we do this after the tablegen patch then we can have tablegen emit the correct size.
> 
> Which would you prefer?
> 
> Also, just so we're on the same page: Which passes do you expect to use them directly?
> If we do this after the tablegen patch then we can have tablegen emit the correct size.

That would be my preference.

> Which passes do you expect to use them directly?

I expect some AArch64 specific passes to use them directly like what we do with GPR64RegClass and such. Anyway, it would be strange if the register classes and the register banks are in different namespace IMHO.


================
Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.h:32
 
+class AArch64GenRegisterBankInfo : public RegisterBankInfo {
+private:
----------------
dsanders wrote:
> qcolombet wrote:
> > FWIW, I believe that generating that part could be part of the tablegen switch. I indeed don't think it makes much to have a XXXGen manually written. I.e., could be squashed with the tablegen switch.
> > 
> > But I am fine with the intermediate step if it does not last long.
> It could be part of the tablegen switch. The reason it isn't is that Ahmed suggested adding the class first and then having a tablegen patch make tablegen emit that same implementation.
Up to you.
I would say that either way is fine as long as XXXGenRegisterBankInfo gets generated by TableGen shortly.


https://reviews.llvm.org/D27976