[globalisel][aarch64] Make getCopyMapping() take register banks ID's rather than IsGPR booleans

[dsanders]

Summary

This allows the function to handle architectures with more than two register banks.

Depends on D27978

[dsanders]

Rebase following changes to the previous patches

[rovka]

lib/Target/AArch64/AArch64GenRegisterBankInfo.def

147

Why is this last member needed? Isn't it invalid to access it in the first place?

150–151

You should update the comment.

[dsanders]

Update comment.
Remove redundant element from BankIDToCopyMapIdx

[dsanders]

Thanks. I've updated the patch

lib/Target/AArch64/AArch64GenRegisterBankInfo.def

147

This is a leftover from before I had the assertions on line 158-159. I'll remove it

[rovka]

Cool, LGTM.

[dsanders]

Moved this patch to the new patch series.

[qcolombet]

Hi Daniel,

I fail to see the goal of this patch.
It does not enable new capabilities nor simplify things.
What am I missing?

Cheers,
-Quentin

[dsanders]

In D27339#628035, @qcolombet wrote:

Hi Daniel,

I fail to see the goal of this patch.
It does not enable new capabilities nor simplify things.
What am I missing?

Cheers,
-Quentin

The aim is to generalize getCopyMapping() so it can support more than two register banks. This will be useful for targets that have special purpose register banks (e.g. the HI/LO registers used by MIPS's multiply and divide and DSP operations, also the co-processor registers), or separate address and data register banks. It should also be useful if AArch64 ever needs to move values between CCR and GPR banks. Later on, I would expect the underlying data tablegen-erated and for getCopyMapping() to be moved to target independent code.

A less important reason (but the one that drew my attention), was that I didn't like the way CHECK_VALUEMAP_CROSSREGCPY() and getCopyMapping() together were converting PartialMappingIdx's to booleans and then back to PartialMappingIdx's and this also caused some problems with my original patch series (CCR needed to be excluded from the copy mapping table).

[dsanders]

Rebased onto new D27978

[dsanders]

Refresh before commit.

lib/Target/AArch64/AArch64GenRegisterBankInfo.def

unsigned ValMappingIdx = First3OpsIdx +
                             getRegBankBaseIdxOffset(Size)) *
                                ValueMappingIdx::DistanceBetweenRegBanks;
   assert(ValMappingIdx >= AArch64::First3OpsIdx &&
          ValMappingIdx <= AArch64::Last3OpsIdx && "Mapping out of bound");
 
   return &ValMappings[ValMappingIdx];
 }
 
+PartialMappingIdx BankIDToCopyMapIdx[] {
+  PMI_None, // CCR
+  PMI_FirstFPR, // FPR
+  PMI_FirstGPR, // GPR
+};
+
 /// 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.
+/// instruction from the \p SrcBankID register bank to the \p DstBankID
+/// register bank with a size of \p Size.
 const RegisterBankInfo::ValueMapping *
-getCopyMapping(bool DstIsGPR, bool SrcIsGPR, unsigned Size) {
-  PartialMappingIdx DstRBIdx = DstIsGPR ? PMI_FirstGPR : PMI_FirstFPR;
-  PartialMappingIdx SrcRBIdx = SrcIsGPR ? PMI_FirstGPR : PMI_FirstFPR;
+getCopyMapping(unsigned DstBankID, unsigned SrcBankID, unsigned Size) {
+  assert(DstBankID < NumRegisterBanks && "Invalid bank ID");
+  assert(SrcBankID < NumRegisterBanks && "Invalid bank ID");
+  PartialMappingIdx DstRBIdx = BankIDToCopyMapIdx[DstBankID];
+  PartialMappingIdx SrcRBIdx = BankIDToCopyMapIdx[SrcBankID];
+  assert(DstRBIdx != PMI_None && "No such mapping");
+  assert(SrcRBIdx != PMI_None && "No such mapping");
+
   if (DstRBIdx == SrcRBIdx)
     return getValueMapping(DstRBIdx, Size);
   assert(Size <= 64 && "GPR cannot handle that size");
   unsigned ValMappingIdx =
       FirstCrossRegCpyIdx +
       (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

#define CHECK_VALUEMAP_CROSSREGCPY(RBNameDst, RBNameSrc, Size)                 \
   do {                                                                         \
     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);         \
+        AArch64::RBNameDst##RegBankID, AArch64::RBNameSrc##RegBankID, Size);   \
     (void) Map;                                                                \
     assert(Map[0].BreakDown == &AArch64::PartMappings[PartialMapDstIdx] &&     \
            Map[0].NumBreakDowns == 1 && #RBNameDst #Size                       \
            " Dst is incorrectly initialized");                                 \
     assert(Map[1].BreakDown == &AArch64::PartMappings[PartialMapSrcIdx] &&     \
            Map[1].NumBreakDowns == 1 && #RBNameSrc #Size                       \
            " Src is incorrectly initialized");                                 \
                                                                                \
case TargetOpcode::G_BITCAST: {
     // If the instruction has any implicit-defs or uses,
     // do not mess with it.
     if (MI.getNumOperands() != 2)
       break;
 
     InstructionMappings AltMappings;
     InstructionMapping GPRMapping(
         /*ID*/ 1, /*Cost*/ 1,
-        AArch64::getCopyMapping(/*DstIsGPR*/ true, /*SrcIsGPR*/ true, Size),
+        AArch64::getCopyMapping(AArch64::GPRRegBankID, AArch64::GPRRegBankID,
+                                Size),
         /*NumOperands*/ 2);
     InstructionMapping FPRMapping(
         /*ID*/ 2, /*Cost*/ 1,
-        AArch64::getCopyMapping(/*DstIsGPR*/ false, /*SrcIsGPR*/ false, Size),
+        AArch64::getCopyMapping(AArch64::FPRRegBankID, AArch64::FPRRegBankID,
+                                Size),
         /*NumOperands*/ 2);
     InstructionMapping GPRToFPRMapping(
         /*ID*/ 3,
         /*Cost*/ copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank, Size),
-        AArch64::getCopyMapping(/*DstIsGPR*/ false, /*SrcIsGPR*/ true, Size),
+        AArch64::getCopyMapping(AArch64::FPRRegBankID, AArch64::GPRRegBankID,
+                                Size),
         /*NumOperands*/ 2);
     InstructionMapping FPRToGPRMapping(
         /*ID*/ 3,
         /*Cost*/ copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank, Size),
-        AArch64::getCopyMapping(/*DstIsGPR*/ true, /*SrcIsGPR*/ false, Size),
+        AArch64::getCopyMapping(AArch64::GPRRegBankID, AArch64::FPRRegBankID,
+                                Size),
         /*NumOperands*/ 2);
 
     AltMappings.emplace_back(std::move(GPRMapping));
     AltMappings.emplace_back(std::move(FPRMapping));
     AltMappings.emplace_back(std::move(GPRToFPRMapping));
     AltMappings.emplace_back(std::move(FPRToGPRMapping));
     return AltMappings;
   }
case TargetOpcode::G_BITCAST: {
     LLT SrcTy = MRI.getType(MI.getOperand(1).getReg());
     unsigned Size = DstTy.getSizeInBits();
     bool DstIsGPR = !DstTy.isVector();
     bool SrcIsGPR = !SrcTy.isVector();
     const RegisterBank &DstRB =
         DstIsGPR ? AArch64::GPRRegBank : AArch64::FPRRegBank;
     const RegisterBank &SrcRB =
         SrcIsGPR ? AArch64::GPRRegBank : AArch64::FPRRegBank;
-    return InstructionMapping{DefaultMappingID, copyCost(DstRB, SrcRB, Size),
-                              AArch64::getCopyMapping(DstIsGPR, SrcIsGPR, Size),
-                              /*NumOperands*/ 2};
+    return InstructionMapping{
+        DefaultMappingID, copyCost(DstRB, SrcRB, Size),
+        AArch64::getCopyMapping(DstRB.getID(), SrcRB.getID(), Size),
   }
   default:
     break;
   }
 
   unsigned NumOperands = MI.getNumOperands();
 
   // Track the size and bank of each register.  We don't do partial mappings.