[RegisterBankInfo] Emit proper type for remapped registers.

When the OperandsMapper creates virtual registers, it used to just create
plain scalar register with the right size. This may confuse the
instruction selector because we lose the information of the instruction
using those registers what supposed to do. The MachineVerifier complains
about that already.

With this patch, the OperandsMapper still creates plain scalar register,
but the expectation is for the mapping function to remap the type
properly. The default mapping function has been updated to do that.

rdar://problem/30231850

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@293362 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h

@@ -317,12 +317,18 @@ public:
 
     /// The final mapping of the instruction.
     const InstructionMapping &getInstrMapping() const { return InstrMapping; }
+
+    /// The MachineRegisterInfo we used to realize the mapping.
+    MachineRegisterInfo &getMRI() const { return MRI; }
     /// @}
 
     /// Create as many new virtual registers as needed for the mapping of the \p
     /// OpIdx-th operand.
     /// The number of registers is determined by the number of breakdown for the
     /// related operand in the instruction mapping.
+    /// The type of the new registers is a plain scalar of the right size.
+    /// The proper type is expected to be set when the mapping is applied to
+    /// the instruction(s) that realizes the mapping.
     ///
     /// \pre getMI().getOperand(OpIdx).isReg()
     ///
@@ -487,6 +493,12 @@ protected:
   /// Basically, that means that \p OpdMapper.getMI() is left untouched
   /// aside from the reassignment of the register operand that have been
   /// remapped.
+  ///
+  /// The type of all the new registers that have been created by the
+  /// mapper are properly remapped to the type of the original registers
+  /// they replace. In other words, the semantic of the instruction does
+  /// not change, only the register banks.
+  ///
   /// If the mapping of one of the operand spans several registers, this
   /// method will abort as this is not like a default mapping anymore.
   ///

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

@@ -351,6 +351,7 @@ RegisterBankInfo::getInstrAlternativeMappings(const MachineInstr &MI) const {
 
 void RegisterBankInfo::applyDefaultMapping(const OperandsMapper &OpdMapper) {
   MachineInstr &MI = OpdMapper.getMI();
+  MachineRegisterInfo &MRI = OpdMapper.getMRI();
   DEBUG(dbgs() << "Applying default-like mapping\n");
   for (unsigned OpIdx = 0,
                 EndIdx = OpdMapper.getInstrMapping().getNumOperands();
@@ -370,9 +371,25 @@ void RegisterBankInfo::applyDefaultMapping(const OperandsMapper &OpdMapper) {
       DEBUG(dbgs() << " has not been repaired, nothing to be done\n");
       continue;
     }
-    DEBUG(dbgs() << " changed, replace " << MO.getReg());
-    MO.setReg(*NewRegs.begin());
-    DEBUG(dbgs() << " with " << MO.getReg());
+    unsigned OrigReg = MO.getReg();
+    unsigned NewReg = *NewRegs.begin();
+    DEBUG(dbgs() << " changed, replace " << PrintReg(OrigReg, nullptr));
+    MO.setReg(NewReg);
+    DEBUG(dbgs() << " with " << PrintReg(NewReg, nullptr));
+
+    // The OperandsMapper creates plain scalar, we may have to fix that.
+    // Check if the types match and if not, fix that.
+    LLT OrigTy = MRI.getType(OrigReg);
+    LLT NewTy = MRI.getType(NewReg);
+    if (OrigTy != NewTy) {
+      assert(OrigTy.getSizeInBits() == NewTy.getSizeInBits() &&
+             "Types with difference size cannot be handled by the default "
+             "mapping");
+      DEBUG(dbgs() << "\nChange type of new opd from " << NewTy << " to "
+                   << OrigTy);
+      MRI.setType(NewReg, OrigTy);
+    }
+    DEBUG(dbgs() << '\n');
   }
 }
 
@@ -584,6 +601,11 @@ void RegisterBankInfo::OperandsMapper::createVRegs(unsigned OpIdx) {
   for (unsigned &NewVReg : NewVRegsForOpIdx) {
     assert(PartMap != ValMapping.end() && "Out-of-bound access");
     assert(NewVReg == 0 && "Register has already been created");
+    // The new registers are always bound to scalar with the right size.
+    // The actual type has to be set when the target does the mapping
+    // of the instruction.
+    // The rationale is that this generic code cannot guess how the
+    // target plans to split the input type.
     NewVReg = MRI.createGenericVirtualRegister(LLT::scalar(PartMap->Length));
     MRI.setRegBank(NewVReg, *PartMap->RegBank);
     ++PartMap;

test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir

@@ -1,5 +1,5 @@
-# RUN: llc -O0 -run-pass=regbankselect -global-isel %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=FAST
-# RUN: llc -O0 -run-pass=regbankselect -global-isel %s -regbankselect-greedy -o - 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=GREEDY
+# RUN: llc -O0 -run-pass=regbankselect -global-isel %s -o - -verify-machineinstrs | FileCheck %s --check-prefix=CHECK --check-prefix=FAST
+# RUN: llc -O0 -run-pass=regbankselect -global-isel %s -regbankselect-greedy -o - -verify-machineinstrs | FileCheck %s --check-prefix=CHECK --check-prefix=GREEDY
 
 --- |
   ; ModuleID = 'generic-virtual-registers-type-error.mir'
@@ -315,8 +315,8 @@ body: |
     ; Fast mode tries to reuse the source of the copy for the destination.
     ; Now, the default mapping says that %0 and %1 need to be in FPR.
     ; The repairing code insert two copies to materialize that.
-    ; FAST-NEXT: %3(s64) = COPY %0
-    ; FAST-NEXT: %4(s64) = COPY %1
+    ; FAST-NEXT: %3(<2 x s32>) = COPY %0
+    ; FAST-NEXT: %4(<2 x s32>) = COPY %1
     ; The mapping of G_OR is on FPR.
     ; FAST-NEXT: %2(<2 x s32>) = G_OR %3, %4
 
@@ -362,13 +362,13 @@ body: |
     ; Fast mode tries to reuse the source of the copy for the destination.
     ; Now, the default mapping says that %0 and %1 need to be in FPR.
     ; The repairing code insert two copies to materialize that.
-    ; FAST-NEXT: %3(s64) = COPY %0
-    ; FAST-NEXT: %4(s64) = COPY %1
+    ; FAST-NEXT: %3(<2 x s32>) = COPY %0
+    ; FAST-NEXT: %4(<2 x s32>) = COPY %1
     ; The mapping of G_OR is on FPR.
     ; FAST-NEXT: %2(<2 x s32>) = G_OR %3, %4
 
     ; Greedy mode remapped the instruction on the GPR bank.
-    ; GREEDY-NEXT: %3(s64) = G_OR %0, %1
+    ; GREEDY-NEXT: %3(<2 x s32>) = G_OR %0, %1
     ; We need to keep %2 into FPR because we do not know anything about it.
     ; GREEDY-NEXT: %2(<2 x s32>) = COPY %3
     %0(<2 x s32>) = COPY %x0