[PATCH] D32650: Properly handle PHIs with subregisters in UnreachableBlockElim

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/CodeGen/UnreachableBlockElim.cpp:208
           MachineRegisterInfo &MRI = F.getRegInfo();
-          MRI.constrainRegClass(Input, MRI.getRegClass(Output));
-          MRI.replaceRegWith(Output, Input);
+          unsigned InputSub = Input.getSubReg();
+          if (InputSub == 0) {
----------------
Maybe add an `assert(Output.getSubReg() == 0);` just to document that we do not write subregisters with Phis.


================
Comment at: lib/CodeGen/UnreachableBlockElim.cpp:213-221
+            // 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)
----------------
Why go through the trouble of creating a new output register? You should be able to reuse "OutputReg" and not do any `replaceRegWith` operation.


================
Comment at: test/CodeGen/Hexagon/unreachable-mbb-phi-subreg.mir:3-7
+--- |
+  define void @fred() {
+    ret void
+  }
+...
----------------
You can remove the whole IR block in this case.


================
Comment at: test/CodeGen/Hexagon/unreachable-mbb-phi-subreg.mir:21
+
+    %0 = COPY %d0
+    J2_jump %bb.2, implicit-def %pc
----------------
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.



Repository:
  rL LLVM

https://reviews.llvm.org/D32650