[PATCH] D51348: CodeGen: Make computeRegisterLiveness consider successors

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:1446
+    for (MachineBasicBlock *S : successors()) {
+      for (MCSubRegIterator SubReg(Reg, TRI, /*IncludeSelf*/true);
+           SubReg.isValid(); ++SubReg) {
----------------
uabelho wrote:
> In the already existing loop in this function when we check live-ins, we use MCRegAliasIterator rather than MCSubRegIterator. Why not do the same here?
> 
> We are using computeRegisterLiveness in my out-of-tree target, and after D51350 we've found a case that goes wrong since this MCSubRegIterator loop decides that the register is dead, while the other loop thought that is was live (which it is).
> 
> In the case it fails we're interested in register $a0_32 (which consists of $a0h, $a0h and $a0l) and then we find that $a10h (which consists of $a0h and $a1h) is livein, but this new loop then determines that $a0_32 is dead.
> 
> So with MCSubRegIterator I think it might draw the wrong conclusions in cases where Reg and liveins are overlapping but neither is subregister of the other.
I'm not really sure what the difference is between these. MCRegAliasIterator visits sub and super registers vs. just subregisters?


https://reviews.llvm.org/D51348