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

[Daniel Sanders via Phabricator via llvm-commits]

dsanders created this revision.
dsanders added reviewers: ab, qcolombet, t.p.northover.
dsanders added a subscriber: llvm-commits.
dsanders added a dependency: D27336: [globalisel][aarch64] Replace magic numbers with corresponding enumerators in ValMappings. NFC.
Herald added subscribers: rovka, dberris, vkalintiris, rengolin, aemerson.

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 https://reviews.llvm.org/D27336


https://reviews.llvm.org/D27337

Files:
  lib/Target/AArch64/AArch64GenRegisterBankInfo.def
  lib/Target/AArch64/AArch64RegisterBankInfo.cpp


Index: lib/Target/AArch64/AArch64RegisterBankInfo.cpp
===================================================================
--- lib/Target/AArch64/AArch64RegisterBankInfo.cpp
+++ lib/Target/AArch64/AArch64RegisterBankInfo.cpp
@@ -115,7 +115,7 @@
 #define CHECK_PARTIALMAP(Idx, ValStartIdx, ValLength, RB)                      \
   do {                                                                         \
     const PartialMapping &Map =                                                \
-        AArch64::PartMappings[AArch64::PartialMappingIdx::Idx];                \
+        AArch64::PartMappings[AArch64::Idx - AArch64::PartialMappingIdx_Min];  \
     (void) Map;                                                                \
     assert(Map.StartIdx == ValStartIdx && Map.Length == ValLength &&           \
            Map.RegBank == &RB && #Idx " is incorrectly initialized");          \
@@ -132,8 +132,8 @@
 // Check value mapping.
 #define CHECK_VALUEMAP_IMPL(RBName, Size, Offset)                              \
   do {                                                                         \
-    AArch64::PartialMappingIdx PartialMapBaseIdx =                             \
-        AArch64::PartialMappingIdx::RBName##Size;                              \
+    unsigned PartialMapBaseIdx =                                               \
+        AArch64::RBName##Size - AArch64::PartialMappingIdx_Min;                \
     (void) PartialMapBaseIdx;                                                  \
     const ValueMapping &Map =                                                  \
         AArch64::getValueMapping(AArch64::First##RBName, Size)[Offset];        \
@@ -172,10 +172,10 @@
 
 #define CHECK_VALUEMAP_CROSSREGCPY(RBNameDst, RBNameSrc, Size)                 \
   do {                                                                         \
-    AArch64::PartialMappingIdx PartialMapDstIdx =                              \
-        AArch64::PartialMappingIdx::RBNameDst##Size;                           \
-    AArch64::PartialMappingIdx PartialMapSrcIdx =                              \
-        AArch64::PartialMappingIdx::RBNameSrc##Size;                           \
+    unsigned PartialMapDstIdx =                                                \
+        AArch64::RBNameDst##Size - AArch64::PartialMappingIdx_Min;             \
+    unsigned PartialMapSrcIdx =                                                \
+        AArch64::RBNameSrc##Size - AArch64::PartialMappingIdx_Min;             \
     (void) PartialMapDstIdx;                                                   \
     (void) PartialMapSrcIdx;                                                   \
     const ValueMapping *Map = AArch64::getCopyMapping(                         \
Index: lib/Target/AArch64/AArch64GenRegisterBankInfo.def
===================================================================
--- lib/Target/AArch64/AArch64GenRegisterBankInfo.def
+++ lib/Target/AArch64/AArch64GenRegisterBankInfo.def
@@ -27,7 +27,7 @@
 // PartialMappings.
 enum PartialMappingIdx {
   None = -1,
-  GPR32 = 0,
+  GPR32 = 1,
   GPR64,
   FPR32,
   FPR64,
@@ -117,11 +117,12 @@
 const RegisterBankInfo::ValueMapping *
 getValueMapping(PartialMappingIdx RBIdx, unsigned Size) {
   assert(RBIdx != PartialMappingIdx::None && "No mapping needed for that");
-  unsigned ValMappingIdx = First3OpsIdx +
-                      (RBIdx + getRegBankBaseIdxOffset(Size)) *
-                          ValueMappingIdx::DistanceBetweenRegBanks;
-    assert(ValMappingIdx >= AArch64::First3OpsIdx &&
-           ValMappingIdx <= AArch64::Last3OpsIdx && "Mapping out of bound");
+  unsigned ValMappingIdx =
+      First3OpsIdx +
+      (RBIdx - AArch64::PartialMappingIdx_Min + getRegBankBaseIdxOffset(Size)) *
+          ValueMappingIdx::DistanceBetweenRegBanks;
+  assert(ValMappingIdx >= AArch64::First3OpsIdx &&
+         ValMappingIdx <= AArch64::Last3OpsIdx && "Mapping out of bound");
 
   return &ValMappings[ValMappingIdx];
 }
@@ -141,7 +142,7 @@
   assert(Size <= 64 && "GPR cannot handle that size");
   unsigned ValMappingIdx =
       FirstCrossRegCpyIdx +
-      (DstRBIdx - FirstGPR + getRegBankBaseIdxOffset(Size)) *
+      (DstRBIdx - PartialMappingIdx_Min + getRegBankBaseIdxOffset(Size)) *
           ValueMappingIdx::DistanceBetweenCrossRegCpy;
   assert(ValMappingIdx >= AArch64::FirstCrossRegCpyIdx &&
          ValMappingIdx <= AArch64::LastCrossRegCpyIdx &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27337.80054.patch
Type: text/x-patch
Size: 4452 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161202/a095f49a/attachment.bin>