[globalisel][aarch64] Fix unintended assumptions about PartialMappingIdx. NFC.

[dsanders]

Summary

This is NFC but prevents assertions when PartialMappingIdx is tablegen-erated.
The assumptions were:

1. FirstGPR is 0
2. FirstGPR is the first of the First* enumerators.

GPR32 is changed to 1 to demonstrate that assumption #1 is fixed. #2 will
be covered by a subsequent patch that tablegen-erates information and swaps
the order of GPR and FPR as a side effect.

Depends on D27336

[qcolombet]

LGTM.

Nitpicks below

lib/Target/AArch64/AArch64GenRegisterBankInfo.def

164

Use AArch64::PartialMappingIdx_Min in both places for consistency.

lib/Target/AArch64/AArch64RegisterBankInfo.cpp

117–118

I'd keep ::PartialMappingIdx::, I like being explicit about the type we use (and that shouldn't be redundant for the _Min part when you'd use a shorter name, see the other review).

135–136

Could we keep the PartialMappingIdx type instead of unsigned?

136

Ditto.

[dsanders]

lib/Target/AArch64/AArch64RegisterBankInfo.cpp

117–118

Ok. Would you like me to make the explicit namespace mandatory by changing the type to an 'enum class'? This would also fix the prefix issue I mentioned on the other review.

135–136

The compiler warns about using PartialMappingIdx because it's not a PartialMappingIdx after the subtraction. The warning could be silenced with a cast like so:

PartialMappingIdx PartialMapBaseIdx = (PartialMappingIdx)(AArch64::PartialMappingIdx::RBName##Size - AArch64::PartialMappingIdx::PartialMappingIdx_Min);

but this is still misleading because the values don't match up with the enumerators.

[dsanders]

Rebase and fix nits about including the ::PartialMappingIdx::

lib/Target/AArch64/AArch64GenRegisterBankInfo.def

 RegisterBank FPRRegBank;
 RegisterBank CCRRegBank;
 
 RegisterBank *RegBanks[] = {&GPRRegBank, &FPRRegBank, &CCRRegBank};
 
 // PartialMappings.
 enum PartialMappingIdx {
   PMI_None = -1,
-  PMI_GPR32 = 0,
+  PMI_GPR32 = 1,
   PMI_GPR64,
   PMI_FPR32,
   PMI_FPR64,
   PMI_FPR128,
   PMI_FPR256,
   PMI_FPR512,
   PMI_FirstGPR = PMI_GPR32,
   PMI_LastGPR = PMI_GPR64,

 /// The returned mapping works for instructions with the same kind of
 /// operands for up to 3 operands.
 ///
 /// \pre \p RBIdx != PartialMappingIdx::None
 const RegisterBankInfo::ValueMapping *
 getValueMapping(PartialMappingIdx RBIdx, unsigned Size) {
   assert(RBIdx != PartialMappingIdx::PMI_None && "No mapping needed for that");
   unsigned ValMappingIdx = First3OpsIdx +
-                      (RBIdx + getRegBankBaseIdxOffset(Size)) *
+                           (RBIdx - AArch64::PartialMappingIdx::PMI_Min +
+                            getRegBankBaseIdxOffset(Size)) *
                                ValueMappingIdx::DistanceBetweenRegBanks;
-    assert(ValMappingIdx >= AArch64::First3OpsIdx &&
-           ValMappingIdx <= AArch64::Last3OpsIdx && "Mapping out of bound");
 
   return &ValMappings[ValMappingIdx];
 }
 
 /// Get the pointer to the ValueMapping of the operands of a copy
 /// instruction from a GPR or FPR register to a GPR or FPR register
 /// with a size of \p Size.
 ///
 /// If \p DstIsGPR is true, the destination of the copy is on GPR,
 /// otherwise it is on FPR. Same thing for \p SrcIsGPR.
 const RegisterBankInfo::ValueMapping *
 getCopyMapping(bool DstIsGPR, bool SrcIsGPR, unsigned Size) {
   PartialMappingIdx DstRBIdx = DstIsGPR ? PMI_FirstGPR : PMI_FirstFPR;
   PartialMappingIdx SrcRBIdx = SrcIsGPR ? PMI_FirstGPR : PMI_FirstFPR;
   if (DstRBIdx == SrcRBIdx)
     return getValueMapping(DstRBIdx, Size);
   assert(Size <= 64 && "GPR cannot handle that size");
   unsigned ValMappingIdx =
       FirstCrossRegCpyIdx +
-      (DstRBIdx - PMI_FirstGPR + getRegBankBaseIdxOffset(Size)) *
+      (DstRBIdx - PMI_Min + getRegBankBaseIdxOffset(Size)) *
           ValueMappingIdx::DistanceBetweenCrossRegCpy;
   assert(ValMappingIdx >= AArch64::FirstCrossRegCpyIdx &&
          ValMappingIdx <= AArch64::LastCrossRegCpyIdx &&
          "Mapping out of bound");
   return &ValMappings[ValMappingIdx];
 }
 
 } // End AArch64 namespace.
 } // End llvm namespace.

lib/Target/AArch64/AArch64RegisterBankInfo.cpp

assert(AArch64::PartialMappingIdx::PMI_FPR32 + 1 ==
              AArch64::PartialMappingIdx::PMI_FPR256 &&
          AArch64::PartialMappingIdx::PMI_FPR256 + 1 ==
              AArch64::PartialMappingIdx::PMI_FPR512 &&
          "FPR indices not properly ordered");
 // Now, the content.
 // Check partial mapping.
 #define CHECK_PARTIALMAP(Idx, ValStartIdx, ValLength, RB)                      \
   do {                                                                         \
     const PartialMapping &Map =                                                \
-        AArch64::PartMappings[AArch64::PartialMappingIdx::Idx];                \
-    (void) Map;                                                                \
+        AArch64::PartMappings[AArch64::PartialMappingIdx::Idx -                \
+                              AArch64::PartialMappingIdx::PMI_Min];            \
+    (void)Map;                                                                 \
     assert(Map.StartIdx == ValStartIdx && Map.Length == ValLength &&           \
            Map.RegBank == &RB && #Idx " is incorrectly initialized");          \
   } while (0)
 
   CHECK_PARTIALMAP(PMI_GPR32, 0, 32, RBGPR);
   CHECK_PARTIALMAP(PMI_GPR64, 0, 64, RBGPR);
   CHECK_PARTIALMAP(PMI_FPR32, 0, 32, RBFPR);
   CHECK_PARTIALMAP(PMI_FPR64, 0, 64, RBFPR);
   CHECK_PARTIALMAP(PMI_FPR128, 0, 128, RBFPR);
   CHECK_PARTIALMAP(PMI_FPR256, 0, 256, RBFPR);
   CHECK_PARTIALMAP(PMI_FPR512, 0, 512, RBFPR);
 
 // Check value mapping.
 #define CHECK_VALUEMAP_IMPL(RBName, Size, Offset)                              \
   do {                                                                         \
-    AArch64::PartialMappingIdx PartialMapBaseIdx =                             \
-        AArch64::PartialMappingIdx::PMI_##RBName##Size;                        \
-    (void) PartialMapBaseIdx;                                                  \
-    const ValueMapping &Map =                                                  \
-        AArch64::getValueMapping(AArch64::PMI_First##RBName, Size)[Offset];    \
-    (void) Map;                                                                \
+    unsigned PartialMapBaseIdx =                                               \
+        AArch64::PartialMappingIdx::PMI_##RBName##Size -                       \
+        AArch64::PartialMappingIdx::PMI_Min;                                   \
+    (void)PartialMapBaseIdx;                                                   \
+    const ValueMapping &Map = AArch64::getValueMapping(                        \
+        AArch64::PartialMappingIdx::PMI_First##RBName, Size)[Offset];          \
+    (void)Map;                                                                 \
     assert(Map.BreakDown == &AArch64::PartMappings[PartialMapBaseIdx] &&       \
            Map.NumBreakDowns == 1 && #RBName #Size                             \
            " " #Offset " is incorrectly initialized");                         \
   } while (0)
 
 #define CHECK_VALUEMAP(RBName, Size) CHECK_VALUEMAP_IMPL(RBName, Size, 0)
 
   CHECK_VALUEMAP(GPR, 32);
#define CHECK_VALUEMAP_3OPS(RBName, Size)                                      \
   CHECK_VALUEMAP_3OPS(FPR, 32);
   CHECK_VALUEMAP_3OPS(FPR, 64);
   CHECK_VALUEMAP_3OPS(FPR, 128);
   CHECK_VALUEMAP_3OPS(FPR, 256);
   CHECK_VALUEMAP_3OPS(FPR, 512);
 
 #define CHECK_VALUEMAP_CROSSREGCPY(RBNameDst, RBNameSrc, Size)                 \
   do {                                                                         \
-    AArch64::PartialMappingIdx PartialMapDstIdx =                              \
-        AArch64::PartialMappingIdx::PMI_##RBNameDst##Size;                     \
-    AArch64::PartialMappingIdx PartialMapSrcIdx =                              \
-        AArch64::PartialMappingIdx::PMI_##RBNameSrc##Size;                     \
+    unsigned PartialMapDstIdx =                                                \
+        AArch64::PMI_##RBNameDst##Size - AArch64::PMI_Min;                     \
+    unsigned PartialMapSrcIdx =                                                \
+        AArch64::PMI_##RBNameSrc##Size - AArch64::PMI_Min;                     \
     (void) PartialMapDstIdx;                                                   \
     (void) PartialMapSrcIdx;                                                   \
     const ValueMapping *Map = AArch64::getCopyMapping(                         \
         AArch64::PMI_First##RBNameDst == AArch64::PMI_FirstGPR,                \
         AArch64::PMI_First##RBNameSrc == AArch64::PMI_FirstGPR, Size);         \
     (void) Map;                                                                \
     assert(Map[0].BreakDown == &AArch64::PartMappings[PartialMapDstIdx] &&     \
            Map[0].NumBreakDowns == 1 && #RBNameDst #Size                       \