[UnreachableBlockElim] Check return value of constrainRegClass().

[uabelho]

Summary

MachineRegisterInfo::constrainRegClass() can fail if two register classes
don't have a common subclass or if the register class doesn't contain
enough registers. Check the return value before trying to remove Phi nodes,
and if we can't constrain, we output a COPY instead of simply replacing
registers.

[uabelho]

Not sure who is code owner of IfConversion.cpp so I added the last three persons who changed
anything there as reviwers.

I found this problem on my out-of-tree target. I've tried to make a reproducer also on Hexagon
but I failed.

My out-of-tree test case looks like:

#RUN: llc -O0 -mtriple phoenix -mcpu=phoenixV -o - %s -run-pass=unreachable-mbb-elimination | FileCheck %s

• | define void @foo() { bb0: %_tmp0 = load i16*, i16** undef, align 1 br label %bb1
  
  bb1: ; preds = %bb0 %_tmp4 = phi i16* [ %_tmp0, %bb0 ] %_tmp5 = icmp eq i16* %_tmp4, undef br label %bb2
  
  bb2: ; preds = %bb1 ret void }

...

name: foo
alignment: 0
exposesReturnsTwice: false
noVRegs: false
legalized: false
regBankSelected: false
selected: false
tracksRegLiveness: true
registers:

• { id: 0, class: rn }
• { id: 1, class: anh_0_7 }
• { id: 2, class: rn }
• { id: 3, class: anh_0_7 }

frameInfo:

isFrameAddressTaken: false
isReturnAddressTaken: false
hasStackMap:     false
hasPatchPoint:   false
stackSize:       0
offsetAdjustment: 0
maxAlignment:    0
adjustsStack:    false
hasCalls:        true
maxCallFrameSize: 0
hasOpaqueSPAdjustment: false
hasVAStart:      false
hasMustTailInVarArgFunc: false

body: |

bb.0:
  successors: %bb.1

  %2 = IMPLICIT_DEF
  %0 = mv_r16_rmod1_ar16 killed %2, 0, _, 0, implicit %dc0, implicit %dc1, implicit %ac0, implicit %ac1
  brr_uncond %bb.1

bb.1:
  successors: %bb.2

  %1 = PHI %0, %bb.0
  %3 = IMPLICIT_DEF
  cmp_a16_a16 %1, killed %3, 0, _, 0, implicit-def %ccreg
  brr_uncond %bb.2

bb.2:
  rets 0, _, 0, implicit-def dead %sp, implicit %sp, implicit %cuc

...

1. The important part here is that we do not use %0 directly in the cmp since %0
2. has regclass rn and the cmp only accepts anh_0_7. So we need a copy from %0
3. to %1, and %1 has regclass anh_0_7

#CHECK: registers:
#CHECK: - { id: 0, class: rn }
#CHECK: - { id: 1, class: anh_0_7 }

#CHECK: bb.0:
#CHECK: successors: %bb.1
#CHECK: %0 = mv_r16_rmod1_ar16 killed %2

#CHECK: bb.1:
#CHECK: %1 = COPY %0
#CHECK: cmp_a16_a16 %1, killed %3

[kparzysz]

LGTM with an update to the comment.

lib/CodeGen/UnreachableBlockElim.cpp

213 ↗

(On Diff #98273)

You should update the comment as well.

[uabelho]

Updated a comment according to review comments.

llvm/trunk/lib/CodeGen/UnreachableBlockElim.cpp

while (phi != BB->end() && phi->isPHI()) {
         unsigned InputReg = Input.getReg();
         unsigned OutputReg = Output.getReg();
         assert(Output.getSubReg() == 0 && "Cannot have output subregister");
         ModifiedPHI = true;
 
         if (InputReg != OutputReg) {
           MachineRegisterInfo &MRI = F.getRegInfo();
           unsigned InputSub = Input.getSubReg();
-          if (InputSub == 0) {
-            MRI.constrainRegClass(InputReg, MRI.getRegClass(OutputReg));
+          if (InputSub == 0 &&
+              MRI.constrainRegClass(InputReg, MRI.getRegClass(OutputReg))) {
             MRI.replaceRegWith(OutputReg, InputReg);
           } else {
-            // The input register to the PHI has a subregister:
+            // The input register to the PHI has a subregister or it can't be
+            // constrained to the proper register class:
             // insert a COPY instead of simply replacing the output
             // with the input.
             const TargetInstrInfo *TII = F.getSubtarget().getInstrInfo();
             BuildMI(*BB, BB->getFirstNonPHI(), phi->getDebugLoc(),
                     TII->get(TargetOpcode::COPY), OutputReg)
                 .addReg(InputReg, getRegState(Input), InputSub);
           }
           phi++->eraseFromParent();