[PATCH] D51348: CodeGen: Make computeRegisterLiveness consider successors

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

I have a post-review question about this change.



================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:1446
+    for (MachineBasicBlock *S : successors()) {
+      for (MCSubRegIterator SubReg(Reg, TRI, /*IncludeSelf*/true);
+           SubReg.isValid(); ++SubReg) {
----------------
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.


https://reviews.llvm.org/D51348