[PATCH] D52410: Use TRI->regsOverlap() in MachineBasicBlock::computeRegisterLiveness

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

uabelho created this revision.
uabelho added reviewers: MatzeB, arsenm.
Herald added a subscriber: wdng.
uabelho added a comment.
arsenm added a comment.
uabelho added a comment.

I would like to create a testcase exposing that we actually fix a bug when we use regsOverlap() instead of MCSubRegIterator, but then I think I need help.

In https://reviews.llvm.org/D51348 Matt suggested it can be triggered in an "AMDGPU testcase where VCC_LO or VCC_HI is defined and VCC is the live out".
I'm not sure about this though. I don't know AMDGPU or how computeRegisterLivenessis used there, but just by looking at the definition of VCC I'm not sure
it's enough to trigger the problem:

def VCC : RegisterWithSubRegs<"vcc", [VCC_LO, VCC_HI]>,

  DwarfRegAlias<VCC_LO> {

Since both VCC_LO and VCC_HI are sub registers of VCC I think the old MCSubRegIterator code works?

What we would need would be some register VCC_X that is made up of both VCC_LO or VCC_HI _and_ some other part so it overlaps/aliases VCC, but it's not
a sub register of VCC.


An instruction just needs to def one or the other and the live out would be vcc, since it needs to visit the super register


In https://reviews.llvm.org/D52410#1243309, @arsenm wrote:

> An instruction just needs to def one or the other and the live out would be vcc, since it needs to visit the super register


Hm, you think any of the cases in test/CodeGen/AMDGPU/fold-immediate-operand-shrink.mir could be copied and modifed
into exposing it then? Any idea which one would be most appropriate?


For the loop that used MCRegAliasIterator this should be NFC.

For the loop that previously used MCSubRegIterator we should
now detect more cases where the register is actually live out that
we previously missed.


Repository:
  rL LLVM

https://reviews.llvm.org/D52410

Files:
  lib/CodeGen/MachineBasicBlock.cpp


Index: lib/CodeGen/MachineBasicBlock.cpp
===================================================================
--- lib/CodeGen/MachineBasicBlock.cpp
+++ lib/CodeGen/MachineBasicBlock.cpp
@@ -1404,9 +1404,8 @@
   // no successor has it live in.
   if (I == end()) {
     for (MachineBasicBlock *S : successors()) {
-      for (MCSubRegIterator SubReg(Reg, TRI, /*IncludeSelf*/true);
-           SubReg.isValid(); ++SubReg) {
-        if (S->isLiveIn(*SubReg))
+      for (const MachineBasicBlock::RegisterMaskPair &LI : S->liveins()) {
+        if (TRI->regsOverlap(LI.PhysReg, Reg))
           return LQR_Live;
       }
     }
@@ -1460,9 +1459,8 @@
   // Did we get to the start of the block?
   if (I == begin()) {
     // If so, the register's state is definitely defined by the live-in state.
-    for (MCRegAliasIterator RAI(Reg, TRI, /*IncludeSelf=*/true); RAI.isValid();
-         ++RAI)
-      if (isLiveIn(*RAI))
+    for (const MachineBasicBlock::RegisterMaskPair &LI : liveins())
+      if (TRI->regsOverlap(LI.PhysReg, Reg))
         return LQR_Live;
 
     return LQR_Dead;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52410.166654.patch
Type: text/x-patch
Size: 1084 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180924/ad1cf92c/attachment.bin>