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 ↗

(On Diff #97126)

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

213–221 ↗

(On Diff #97126)

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 ↗

(On Diff #97126)

You can remove the whole IR block in this case.

21 ↗

(On Diff #97126)

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 ↗

(On Diff #97126)

Ha, indeed.

[kparzysz]

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

[kparzysz]

Remove the LLVM IR from the mir testcase.

[MatzeB]

LGTM.

llvm/trunk/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();
-
-        phi++->eraseFromParent();
+        const MachineOperand &Input = phi->getOperand(1);
+        const MachineOperand &Output = phi->getOperand(0);
+        unsigned InputReg = Input.getReg();
+        unsigned OutputReg = Output.getReg();
+        assert(Output.getSubReg() == 0 && "Cannot have output subregister");
         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:
+            // 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();
         }
-
         continue;
       }
 
       ++phi;
     }
   }
 
   F.RenumberBlocks();
 
   return (!DeadBlocks.empty() || ModifiedPHI);
 }

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

+# RUN: llc -march=hexagon -run-pass unreachable-mbb-elimination %s -o - | FileCheck %s
+
+---
+name: fred
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: %d0
+    successors: %bb.2
+
+    %0 : doubleregs = 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 : intregs = PHI %0.isub_lo, %bb.0, %0.isub_hi, %bb.1
+    %r0 = COPY %1
+...
+