[PATCH] D27337: [globalisel][aarch64] Fix unintended assumptions about PartialMappingIdx. NFC.

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

LGTM.

Nitpicks below



================
Comment at: lib/Target/AArch64/AArch64GenRegisterBankInfo.def:145
       FirstCrossRegCpyIdx +
-      (DstRBIdx - FirstGPR + getRegBankBaseIdxOffset(Size)) *
+      (DstRBIdx - PartialMappingIdx_Min + getRegBankBaseIdxOffset(Size)) *
           ValueMappingIdx::DistanceBetweenCrossRegCpy;
----------------
Use `AArch64::`PartialMappingIdx_Min in both places for consistency.


================
Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.cpp:118
     const PartialMapping &Map =                                                \
-        AArch64::PartMappings[AArch64::PartialMappingIdx::Idx];                \
+        AArch64::PartMappings[AArch64::Idx - AArch64::PartialMappingIdx_Min];  \
     (void) Map;                                                                \
----------------
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).


================
Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.cpp:135
   do {                                                                         \
-    AArch64::PartialMappingIdx PartialMapBaseIdx =                             \
-        AArch64::PartialMappingIdx::RBName##Size;                              \
+    unsigned PartialMapBaseIdx =                                               \
+        AArch64::RBName##Size - AArch64::PartialMappingIdx_Min;                \
----------------
Could we keep the PartialMappingIdx type instead of unsigned?


================
Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.cpp:136
+    unsigned PartialMapBaseIdx =                                               \
+        AArch64::RBName##Size - AArch64::PartialMappingIdx_Min;                \
     (void) PartialMapBaseIdx;                                                  \
----------------
Ditto.


https://reviews.llvm.org/D27337