GlobalISel: Avoid producing Illegal copies in RegBankSelect

[arsenm]

Summary

Avoid producing illegal register bank copies for reg_sequence and
phi. The default implementation assumes it is possible to pick any
operand's bank and use that for the result, introducing a copy for
operands with a different bank. This does not check for illegal
copies. It is not legal to introduce a VGPR->SGPR copy, so any VGPR
operand requires the result to be a VGPR.

The changes in getInstrMappingImpl aren't strictly necessary, since
AMDGPU now just bypasses this for reg_sequence/phi. This could be
replaced with an assert in case other targets run into this. It is
currently responsible for producing the error for unsatisfiable
copies, but this will be better served with a verifier check.

For phis, for now assume any undetermined operands must be
VGPRs. Eventually, this needs to be able to defer mapping these
operations. This also does not yet have a way to check for whether the
block is in a divergent region.

[arsenm]

Remove unused variable

[qcolombet]

LGTM.

Nitpicks below.

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

252

From LLVM standard: no brackets here.

418

Ditto

[arsenm]

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

252

I will die on the hill that there should be braces here. There are multiple lines after the if due to the comment. I don't see where it says this, and clang-format doesn't touch it

[qcolombet]

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

252

I don't see anything in the standard mentioning the one-line-block thing now that you mention it. Which is strange because then it would be left to interpretation for case like:

if ()
  continue;

or

if () {
  continue;
}

whereas I thought we were very strict at using the former.

FWIW, clang will not remove the braces for the latter case either, so that's not a good indication of what is in the standard, unless the standard is: we don't care :).

Anyway, I don't really care, but I apply the rule for every one-statement-block. To me comments don't count as lines.

For instance, lib/Analysis/LazyCallGraph.cpp has one-statement block with comments (i.e., more than one line block in the end), but it still doesn't have braces.

[arsenm]

r363410

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

252

There are examples of both, so I don't think there's a clear rule

include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h

virtual unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
                             unsigned Size) const {
     // Optimistically assume that copies are coalesced. I.e., when
     // they are on the same bank, they are free.
     // Otherwise assume a non-zero cost of 1. The targets are supposed
     // to override that properly anyway if they care.
     return &A != &B;
   }
 
+  /// \returns true if emitting a copy from \p Src to \p Dst is impossible.
+  bool cannotCopy(const RegisterBank &Dst, const RegisterBank &Src,
+                     unsigned Size) const {
+    return copyCost(Dst, Src, Size) == std::numeric_limits<unsigned>::max();
+  }
+
   /// Get the cost of using \p ValMapping to decompose a register. This is
   /// similar to ::copyCost, except for cases where multiple copy-like
   /// operations need to be inserted. If the register is used as a source
   /// operand and already has a bank assigned, \p CurBank is non-null.
   virtual unsigned getBreakDownCost(const ValueMapping &ValMapping,
                                     const RegisterBank *CurBank = nullptr) const {
     return std::numeric_limits<unsigned>::max();
   }

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

if (IsCopyLike) {
         if (MI.isRegSequence()) {
           // For reg_sequence, the result size does not match the input.
           unsigned ResultSize = getSizeInBits(MI.getOperand(0).getReg(),
                                               MRI, TRI);
           OperandsMapping[0] = &getValueMapping(0, ResultSize, *CurRegBank);
         } else {
           OperandsMapping[0] = ValMapping;
         }
+      }
 
-        CompleteMapping = true;
+      // The default handling assumes any register bank can be copied to any
+      // other. If this isn't the case, the target should specially deal with
+      // reg_sequence/phi. There may also be unsatisfiable copies.
+      for ( ; OpIdx != EndIdx; ++OpIdx) {
+        const MachineOperand &MO = MI.getOperand(OpIdx);
+        if (!MO.isReg())
+          continue;
+        unsigned Reg = MO.getReg();
+        if (!Reg)
+          continue;
+
+        const RegisterBank *AltRegBank = getRegBank(Reg, MRI, TRI);
+        if (cannotCopy(*CurRegBank, *AltRegBank,
+                       getSizeInBits(Reg, MRI, TRI)))
+          return getInvalidInstructionMapping();
       }
 
+      CompleteMapping = true;
       break;
     }
+
     OperandsMapping[OpIdx] = ValMapping;
   }
 
-  if (IsCopyLike && !CompleteMapping)
+  if (IsCopyLike && !CompleteMapping) {
     // No way to deduce the type from what we have.
     return getInvalidInstructionMapping();
+  }
 
   assert(CompleteMapping && "Setting an uncomplete mapping");
   return getInstructionMapping(
       DefaultMappingID, /*Cost*/ 1,
       /*OperandsMapping*/ getOperandsMapping(OperandsMapping),
       NumOperandsForMapping);
 }
 
RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
   if (Mapping.isValid())
     return Mapping;
   llvm_unreachable("The target must implement this");
 }
 
 RegisterBankInfo::InstructionMappings
 RegisterBankInfo::getInstrPossibleMappings(const MachineInstr &MI) const {
   InstructionMappings PossibleMappings;
+  const auto &Mapping = getInstrMapping(MI);
+  if (Mapping.isValid()) {
     // Put the default mapping first.
-  PossibleMappings.push_back(&getInstrMapping(MI));
+    PossibleMappings.push_back(&Mapping);
+  }
+
   // Then the alternative mapping, if any.
   InstructionMappings AltMappings = getInstrAlternativeMappings(MI);
   for (const InstructionMapping *AltMapping : AltMappings)
     PossibleMappings.push_back(AltMapping);
 #ifndef NDEBUG
   for (const InstructionMapping *Mapping : PossibleMappings)
     assert(Mapping->verify(MI) && "Mapping is invalid");
 #endif

lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

 ///
 /// This function must return a legal mapping, because
 /// AMDGPURegisterBankInfo::getInstrAlternativeMappings() is not called
 /// in RegBankSelect::Mode::Fast.  Any mapping that would cause a
 /// VGPR to SGPR generated is illegal.
 ///
 const RegisterBankInfo::InstructionMapping &
 AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
-  const RegisterBankInfo::InstructionMapping &Mapping = getInstrMappingImpl(MI);
+  const MachineFunction &MF = *MI.getParent()->getParent();
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
+
+  if (MI.isRegSequence()) {
+    // If any input is a VGPR, the result must be a VGPR. The default handling
+    // assumes any copy between banks is legal.
+    unsigned BankID = AMDGPU::SGPRRegBankID;
+
+    for (unsigned I = 1, E = MI.getNumOperands(); I != E; I += 2) {
+      auto OpBank = getRegBankID(MI.getOperand(I).getReg(), MRI, *TRI);
+      // It doesn't make sense to use vcc or scc banks here, so just ignore
+      // them.
+      if (OpBank != AMDGPU::SGPRRegBankID) {
+        BankID = AMDGPU::VGPRRegBankID;
+        break;
+      }
+    }
+    unsigned Size = getSizeInBits(MI.getOperand(0).getReg(), MRI, *TRI);
+
+    const ValueMapping &ValMap = getValueMapping(0, Size, getRegBank(BankID));
+    return getInstructionMapping(
+      1, /*Cost*/ 1,
+      /*OperandsMapping*/ getOperandsMapping({&ValMap}),
+      1);
+  }
+
+  // The default handling is broken and doesn't handle illegal SGPR->VGPR copies
+  // properly.
+  //
+  // TODO: There are additional exec masking dependencies to analyze.
+  if (MI.getOpcode() == TargetOpcode::G_PHI) {
+    // TODO: Generate proper invalid bank enum.
+    int ResultBank = -1;
+
+    for (unsigned I = 1, E = MI.getNumOperands(); I != E; I += 2) {
+      unsigned Reg = MI.getOperand(I).getReg();
+      const RegisterBank *Bank = getRegBank(Reg, MRI, *TRI);
+
+      // FIXME: Assuming VGPR for any undetermined inputs.
+      if (!Bank || Bank->getID() == AMDGPU::VGPRRegBankID) {
+        ResultBank = AMDGPU::VGPRRegBankID;
+        break;
+      }
+
+      unsigned OpBank = Bank->getID();
+      // scc, scc -> sgpr
+      if (OpBank == AMDGPU::SCCRegBankID) {
+        // There's only one SCC register, so a phi requires copying to SGPR.
+        OpBank = AMDGPU::SGPRRegBankID;
+      } else if (OpBank == AMDGPU::VCCRegBankID) {
+        // vcc, vcc -> vcc
+        // vcc, sgpr -> vgpr
+        if (ResultBank != -1 && ResultBank != AMDGPU::VCCRegBankID) {
+          ResultBank = AMDGPU::VGPRRegBankID;
+          break;
+        }
+      }
+
+      ResultBank = OpBank;
+    }
+
+    assert(ResultBank != -1);
 
+    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
+
+    const ValueMapping &ValMap = getValueMapping(0, Size, getRegBank(ResultBank));
+    return getInstructionMapping(
+      1, /*Cost*/ 1,
+      /*OperandsMapping*/ getOperandsMapping({&ValMap}),
+      1);
+  }
+
+  const RegisterBankInfo::InstructionMapping &Mapping = getInstrMappingImpl(MI);
   if (Mapping.isValid())
     return Mapping;
 
-  const MachineFunction &MF = *MI.getParent()->getParent();
-  const MachineRegisterInfo &MRI = MF.getRegInfo();
   SmallVector<const ValueMapping*, 8> OpdsMapping(MI.getNumOperands());
 
   switch (MI.getOpcode()) {
   default:
     return getInvalidInstructionMapping();
 
   case AMDGPU::G_AND:
   case AMDGPU::G_OR:

test/CodeGen/AMDGPU/GlobalISel/regbankselect-illegal-copy.mir

+# RUN: not llc -march=amdgcn -run-pass=regbankselect  -regbankselect-fast  %s -o /dev/null 2>&1 | FileCheck %s
+# RUN: not llc -march=amdgcn -run-pass=regbankselect  -regbankselect-greedy  %s -o /dev/null 2>&1 | FileCheck %s
+
+# Check behavior for illegal copies.
+
+# CHECK: LLVM ERROR: unable to map instruction: $sgpr0 = COPY %0:vgpr(s32) (in function: illegal_copy_s32_v_to_s)
+
+---
+name: illegal_copy_s32_v_to_s
+legalized: true
+
+body: |
+  bb.0:
+    liveins: $vgpr0
+
+    %0:_(s32) = COPY $vgpr0
+    $sgpr0 = COPY %0
+...

test/CodeGen/AMDGPU/GlobalISel/regbankselect-phi.mir

body: |
   ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[COPY3:%[0-9]+]]:vgpr(s32) = COPY [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s32) = G_PHI [[COPY]](s32), %bb.0, [[COPY3]](s32), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, [[COPY3]](s32), %bb.1
   ; CHECK:   $vgpr0 = COPY [[PHI]](s32)
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $sgpr0, $vgpr0, $sgpr1
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $vgpr0
body: |
   ; CHECK:   [[ICMP:%[0-9]+]]:vcc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[COPY3:%[0-9]+]]:vgpr(s32) = COPY [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s32) = G_PHI [[COPY]](s32), %bb.0, [[COPY3]](s32), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, [[COPY3]](s32), %bb.1
   ; CHECK:   $vgpr0 = COPY [[PHI]](s32)
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $sgpr0, $vgpr0, $vgpr1
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $vgpr0
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[ICMP2:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:scc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1
-  ; CHECK:   [[SELECT:%[0-9]+]]:sgpr(s32) = G_SELECT [[PHI]](s1), [[C]], [[COPY]]
+  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $sgpr0, $sgpr1, $sgpr2
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $sgpr1
     %2:_(s32) = COPY $sgpr2
body: |
   ; CHECK:   G_BRCOND [[ICMP3]](s1), %bb.3
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
   ; CHECK:   successors: %bb.3(0x80000000)
   ; CHECK:   [[C3:%[0-9]+]]:sgpr(s32) = G_CONSTANT i32 123
   ; CHECK:   [[ICMP4:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C3]]
   ; CHECK:   G_BR %bb.3
   ; CHECK: bb.3:
-  ; CHECK:   [[PHI:%[0-9]+]]:scc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1, [[ICMP4]](s1), %bb.2
-  ; CHECK:   [[SELECT:%[0-9]+]]:sgpr(s32) = G_SELECT [[PHI]](s1), [[COPY]], [[COPY1]]
+  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1, [[ICMP4]](s1), %bb.2
+  ; CHECK:   [[COPY4:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
+  ; CHECK:   [[COPY6:%[0-9]+]]:vgpr(s32) = COPY [[COPY1]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY4]](s1), [[COPY5]], [[COPY6]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.3
     liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $sgpr1
     %2:_(s32) = COPY $sgpr2
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[ICMP2:%[0-9]+]]:vcc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:scc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1
-  ; CHECK:   [[SELECT:%[0-9]+]]:sgpr(s32) = G_SELECT [[PHI]](s1), [[C]], [[COPY]]
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $sgpr0, $sgpr1, $vgpr0
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $vgpr0
     %2:_(s32) = COPY $sgpr1
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[ICMP2:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:vcc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1
-  ; CHECK:   [[COPY3:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
-  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[COPY1]](s32)
-  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[PHI]](s1), [[COPY3]], [[COPY4]]
+  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[ICMP2]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY1]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $sgpr0, $sgpr1
 
     %0:_(s32) = COPY $vgpr0
     %1:_(s32) = COPY $sgpr0
     %2:_(s32) = COPY $sgpr1
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[TRUNC:%[0-9]+]]:sgpr(s1) = G_TRUNC [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:scc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
-  ; CHECK:   [[SELECT:%[0-9]+]]:sgpr(s32) = G_SELECT [[PHI]](s1), [[C]], [[COPY]]
+  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $sgpr0, $sgpr1, $sgpr2
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $sgpr1
     %2:_(s32) = COPY $sgpr2
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:scc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
-  ; CHECK:   [[SELECT:%[0-9]+]]:sgpr(s32) = G_SELECT [[PHI]](s1), [[C]], [[COPY]]
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $sgpr0, $sgpr1, $sgpr2
 
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $vgpr0
     %2:_(s32) = COPY $sgpr2
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[TRUNC:%[0-9]+]]:sgpr(s1) = G_TRUNC [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:vcc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
-  ; CHECK:   [[COPY3:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
-  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[PHI]](s1), [[COPY3]], [[COPY]]
+  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $sgpr0, $sgpr1
 
     %0:_(s32) = COPY $vgpr0
     %1:_(s32) = COPY $sgpr0
     %2:_(s32) = COPY $sgpr1
body: |
   ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[ICMP1:%[0-9]+]]:vcc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[TRUNC]](s1), %bb.0, [[ICMP1]](s1), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s1) = G_PHI [[TRUNC]](s1), %bb.0, [[ICMP1]](s1), %bb.1
   ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
   ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
   ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
   ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $sgpr0, $sgpr1
body: |
   ; CHECK:   [[ICMP1:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP1]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:vcc(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
-  ; CHECK:   [[COPY3:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
-  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[PHI]](s1), [[COPY3]], [[COPY]]
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s1) = G_PHI [[ICMP]](s1), %bb.0, [[TRUNC]](s1), %bb.1
+  ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
+  ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+  ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $vgpr1, $sgpr0
 
     %0:_(s32) = COPY $vgpr0
     %1:_(s32) = COPY $vgpr1
     %2:_(s32) = COPY $sgpr0
body: |
   ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[TRUNC1:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY1]](s32)
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
-  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s1) = G_PHI [[TRUNC]](s1), %bb.0, [[TRUNC1]](s1), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s1) = G_PHI [[TRUNC]](s1), %bb.0, [[TRUNC1]](s1), %bb.1
   ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
   ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
   ; CHECK:   [[COPY5:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
   ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY5]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $sgpr0, $sgpr1
body: |
   ; CHECK:   liveins: $sgpr0, $sgpr1, $vgpr0
   ; CHECK:   [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
   ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s32) = COPY $sgpr1
   ; CHECK:   [[COPY2:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
   ; CHECK:   [[C:%[0-9]+]]:sgpr(s32) = G_CONSTANT i32 0
   ; CHECK:   G_BR %bb.1
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s32) = G_PHI [[COPY]](s32), %bb.0, %5(s32), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, %5(s32), %bb.1
   ; CHECK:   [[COPY3:%[0-9]+]]:vgpr(s32) = COPY [[COPY2]](s32)
   ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[PHI]](s32)
   bb.0:
     successors: %bb.1
body: |
   ; CHECK:   liveins: $sgpr0, $sgpr1, $sgpr2
   ; CHECK:   [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
   ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s32) = COPY $sgpr1
   ; CHECK:   [[COPY2:%[0-9]+]]:sgpr(s32) = COPY $sgpr2
   ; CHECK:   [[C:%[0-9]+]]:sgpr(s32) = G_CONSTANT i32 0
   ; CHECK:   G_BR %bb.1
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:sgpr(s32) = G_PHI [[COPY]](s32), %bb.0, %5(s32), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, %5(s32), %bb.1
   ; CHECK:   [[COPY3:%[0-9]+]]:sgpr(s32) = COPY [[COPY2]](s32)
   ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[PHI]](s32)
   bb.0:
     successors: %bb.1
bb.1:
     %6:_(s1) = G_ICMP intpred(eq), %1, %3
     G_BRCOND %6, %bb.1
     G_BR %bb.2
 
   bb.2:
     S_SETPC_B64 undef $sgpr30_sgpr31, implicit %4
 
 ...
+
+---
+name: phi_s32_ss_sbranch_cycle
+legalized: true
+tracksRegLiveness: true
+
+body: |
+  ; CHECK-LABEL: name: phi_s32_ss_sbranch_cycle
+  ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK:   liveins: $sgpr0, $sgpr1, $sgpr2
+  ; CHECK:   [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s32) = COPY $sgpr1
+  ; CHECK:   [[COPY2:%[0-9]+]]:sgpr(s32) = COPY $sgpr2
+  ; CHECK:   [[C:%[0-9]+]]:sgpr(s32) = G_CONSTANT i32 0
+  ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
+  ; CHECK:   G_BR %bb.2
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.2(0x80000000)
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, %6(s32), %bb.2
+  ; CHECK:   G_BR %bb.2
+  ; CHECK: bb.2:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[PHI1:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK:   G_BR %bb.1
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $sgpr0, $sgpr1, $sgpr2
+
+    %0:_(s32) = COPY $sgpr0
+    %1:_(s32) = COPY $sgpr1
+    %2:_(s32) = COPY $sgpr2
+    %3:_(s32) = G_CONSTANT i32 0
+    %4:_(s1) = G_ICMP intpred(eq), %2, %3
+    G_BRCOND %4, %bb.1
+    G_BR %bb.2
+
+  bb.1:
+    successors: %bb.2
+    %5:_(s32) = G_PHI %0, %bb.0, %6, %bb.2
+    G_BR %bb.2
+
+  bb.2:
+    successors: %bb.1
+    %6:_(s32) = G_PHI %0, %bb.0, %5, %bb.1
+    G_BR %bb.1
+...
+
+---
+name: phi_s32_vs_sbranch_cycle
+legalized: true
+tracksRegLiveness: true
+
+body: |
+  ; CHECK-LABEL: name: phi_s32_vs_sbranch_cycle
+  ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK:   liveins: $vgpr0, $sgpr1, $sgpr2
+  ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:sgpr(s32) = COPY $sgpr1
+  ; CHECK:   [[COPY2:%[0-9]+]]:sgpr(s32) = COPY $sgpr2
+  ; CHECK:   [[C:%[0-9]+]]:sgpr(s32) = G_CONSTANT i32 0
+  ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
+  ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
+  ; CHECK:   G_BR %bb.2
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.2(0x80000000)
+  ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, %6(s32), %bb.2
+  ; CHECK:   G_BR %bb.2
+  ; CHECK: bb.2:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[PHI1:%[0-9]+]]:vgpr(s32) = G_PHI [[COPY]](s32), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK:   G_BR %bb.1
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $vgpr0, $sgpr1, $sgpr2
+
+    %0:_(s32) = COPY $vgpr0
+    %1:_(s32) = COPY $sgpr1
+    %2:_(s32) = COPY $sgpr2
+    %3:_(s32) = G_CONSTANT i32 0
+    %4:_(s1) = G_ICMP intpred(eq), %2, %3
+    G_BRCOND %4, %bb.1
+    G_BR %bb.2
+
+  bb.1:
+    successors: %bb.2
+    %5:_(s32) = G_PHI %0, %bb.0, %6, %bb.2
+    G_BR %bb.2
+
+  bb.2:
+    successors: %bb.1
+    %6:_(s32) = G_PHI %0, %bb.0, %5, %bb.1
+    G_BR %bb.1
+...

test/CodeGen/AMDGPU/GlobalISel/regbankselect-reg-sequence.mir

 body: |
   bb.0:
     liveins: $sgpr0, $vgpr0
 
     ; CHECK-LABEL: name: reg_sequence_sv_vreg
     ; CHECK: liveins: $sgpr0, $vgpr0
     ; CHECK: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
     ; CHECK: [[COPY1:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
-    ; CHECK: [[REG_SEQUENCE:%[0-9]+]]:sgpr(s64) = REG_SEQUENCE [[COPY]](s32), %subreg.sub0, [[COPY1]](s32), %subreg.sub1
+    ; CHECK: [[REG_SEQUENCE:%[0-9]+]]:vgpr(s64) = REG_SEQUENCE [[COPY]](s32), %subreg.sub0, [[COPY1]](s32), %subreg.sub1
     %0:_(s32) = COPY $sgpr0
     %1:_(s32) = COPY $vgpr0
     %2:_(s64) = REG_SEQUENCE %0, %subreg.sub0, %1, %subreg.sub1
 ...
 
 ---
 name: reg_sequence_sv_physreg
 legalized: true
 tracksRegLiveness: true
 
 body: |
   bb.0:
     liveins: $sgpr0, $vgpr0
 
     ; CHECK-LABEL: name: reg_sequence_sv_physreg
     ; CHECK: liveins: $sgpr0, $vgpr0
-    ; CHECK: [[REG_SEQUENCE:%[0-9]+]]:sgpr(s64) = REG_SEQUENCE $sgpr0, %subreg.sub0, $vgpr0, %subreg.sub1
+    ; CHECK: [[REG_SEQUENCE:%[0-9]+]]:vgpr(s64) = REG_SEQUENCE $sgpr0, %subreg.sub0, $vgpr0, %subreg.sub1
     %0:_(s64) = REG_SEQUENCE $sgpr0, %subreg.sub0, $vgpr0, %subreg.sub1
 ...
 
 ---
 name: reg_sequence_vs_vreg
 legalized: true
 tracksRegLiveness: true