Properly handle PHIs with subregisters in UnreachableBlockElim

[kparzysz]

Summary

When a PHI operand has a subregister, create a COPY into a new register instead of simply replacing the PHI output with it.

[kparzysz]

Moved the phi deletion into the correct place.

[MatzeB]

lib/CodeGen/UnreachableBlockElim.cpp

208

Maybe add an assert(Output.getSubReg() == 0); just to document that we do not write subregisters with Phis.

213–221

Why go through the trouble of creating a new output register? You should be able to reuse "OutputReg" and not do any replaceRegWith operation.

test/CodeGen/Hexagon/unreachable-mbb-phi-subreg.mir

3–7

You can remove the whole IR block in this case.

21

And there's this new cool syntax (the MIPrinter just doesn't know how to use it yet), where you can do

%0 : doubleregs = COPY %d0
...
%1 : intregs = PHI ...

instead of using a registers: block.

[kparzysz]

lib/CodeGen/UnreachableBlockElim.cpp

213–221

Ha, indeed.

[kparzysz]

Don't create unnecessary registers, use shorter mir syntax.

[kparzysz]

Remove the LLVM IR from the mir testcase.

[MatzeB]

LGTM.

lib/CodeGen/UnreachableBlockElim.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/UnreachableBlockElim.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/CodeGen/MachineDominators.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
while (phi != BB->end() && phi->isPHI()) {
       for (unsigned i = phi->getNumOperands() - 1; i >= 2; i-=2)
         if (!preds.count(phi->getOperand(i).getMBB())) {
           phi->RemoveOperand(i);
           phi->RemoveOperand(i-1);
           ModifiedPHI = true;
         }
 
       if (phi->getNumOperands() == 3) {
-        unsigned Input = phi->getOperand(1).getReg();
-        unsigned Output = phi->getOperand(0).getReg();
+        const MachineOperand &Input = phi->getOperand(1);
+        const MachineOperand &Output = phi->getOperand(0);
+        unsigned InputReg = Input.getReg();
+        unsigned OutputReg = Output.getReg();
 
-        phi++->eraseFromParent();
         ModifiedPHI = true;
 
-        if (Input != Output) {
+        if (InputReg != OutputReg) {
           MachineRegisterInfo &MRI = F.getRegInfo();
-          MRI.constrainRegClass(Input, MRI.getRegClass(Output));
-          MRI.replaceRegWith(Output, Input);
+          unsigned InputSub = Input.getSubReg();
+          if (InputSub == 0) {
+            MRI.constrainRegClass(InputReg, MRI.getRegClass(OutputReg));
+            MRI.replaceRegWith(OutputReg, InputReg);
+          } else {
+            // The input register to the PHI has a subregister:
+            // create a new register and insert a COPY instead.
+            const TargetRegisterClass *OutRC = MRI.getRegClass(OutputReg);
+            unsigned NewOutput = MRI.createVirtualRegister(OutRC);
+            const TargetInstrInfo *TII = F.getSubtarget().getInstrInfo();
+            BuildMI(*BB, BB->getFirstNonPHI(), phi->getDebugLoc(),
+                    TII->get(TargetOpcode::COPY), NewOutput)
+                .addReg(InputReg, getRegState(Input), InputSub);
+            MRI.replaceRegWith(OutputReg, NewOutput);
+          }
+          phi++->eraseFromParent();
         }
-
         continue;
       }
 
       ++phi;
     }
   }
 
   F.RenumberBlocks();
 
   return (!DeadBlocks.empty() || ModifiedPHI);
 }

test/CodeGen/Hexagon/unreachable-mbb-phi-subreg.mir

+# RUN: llc -march=hexagon -run-pass unreachable-mbb-elimination %s -o - | FileCheck %s
+
+--- |
+  define void @fred() {
+    ret void
+  }
+...
+
+---
+name: fred
+tracksRegLiveness: true
+
+registers:
+  - { id: 0, class: doubleregs }
+  - { id: 1, class: intregs }
+body: |
+  bb.0:
+    liveins: %d0
+    successors: %bb.2
+
+    %0 = COPY %d0
+    J2_jump %bb.2, implicit-def %pc
+
+  bb.1:
+    successors: %bb.2
+    A2_nop
+
+  bb.2:
+    ; Make sure that the subregister from the PHI operand is preserved.
+    ; CHECK: %[[REG:[0-9]+]] = COPY %0.isub_lo
+    ; CHECK: %r0 = COPY %[[REG]]
+    %1 = PHI %0.isub_lo, %bb.0, %0.isub_hi, %bb.1
+    %r0 = COPY %1
+...
+