[PATCH] D32563: Add LiveRangeShrink pass to shrink live range within BB.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.

Register class logic looks good to me with the nitpicks below addressed.



================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:164-165
+        // unless it is a constant physical register.
+        if (TargetRegisterInfo::isPhysicalRegister(Reg) &&
+            !MRI.isConstantPhysReg(Reg)) {
+          Insert = nullptr;
----------------
This should better be `!TargetRegisterInfo::isVirtualRegister(Reg)` which is not the same as `isPhysicalRegister()` because a register operand can also be NoReg (=0) which shouldn't stop movement.


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:177-178
+        } else if (MRI.hasOneNonDBGUse(Reg) && MRI.hasOneDef(Reg) && DefMO &&
+                   MRI.getRegClass(DefMO->getReg()) ==
+                       MRI.getRegClass(MO.getReg())) {
+          MachineInstr &DefInstr = *MRI.def_instr_begin(Reg);
----------------
- As we got surprised by it in the first try, it would be good to document the necessity for the regClass() check. Something along the lines of:
```
/// The heuristic does not handle different register classes yet (registers of different sizes, looser/tighter constraints).
```
- getRegClass() only works for virtual registers, as far as I understand this needs a check as we could get here with a constant physreg.


https://reviews.llvm.org/D32563