[PATCH] D27336: [globalisel][aarch64] Replace magic numbers with corresponding enumerators in ValMappings. NFC

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: lib/Target/AArch64/AArch64GenRegisterBankInfo.def:41
+  LastFPR = FPR512,
+  PartialMappingIdx_Min = FirstGPR,
 };
----------------
qcolombet wrote:
> I'd use a shorter name: MinIdx or FirstIdx
I was concerned about polluting the llvm::AArch64 namespace with generic-sounding identifiers. Looking at it again, this enum isn't in line with the coding standards (enumerators should have a common prefix) and fixing that would fix this problem. I think we should prefix them all with 'PMI_' to get:
  PMI_None,
  PMI_GPR32,
  ...
  PMI_LastFPR = PMI_FPR512,
  PMI_Min = PMI_FirstGPR
Is that ok with you?


================
Comment at: lib/Target/AArch64/AArch64GenRegisterBankInfo.def:85
   // 0: GPR 32-bit value. <-- This must match First3OpsIdx.
-  {&PartMappings[0], 1}, {&PartMappings[0], 1}, {&PartMappings[0], 1},
+  {&PartMappings[GPR32 - PartialMappingIdx_Min], 1}, {&PartMappings[GPR32 - PartialMappingIdx_Min], 1}, {&PartMappings[GPR32 - PartialMappingIdx_Min], 1},
   // 3: GPR 64-bit value.
----------------
qcolombet wrote:
> Could you clang-format those lines?
Ok.


https://reviews.llvm.org/D27336