[llvm] r307428 - [RegAllocFast] Don't insert kill flags of super-register for partial kill

[Quentin Colombet via llvm-commits]

Author: qcolombet
Date: Fri Jul  7 12:25:45 2017
New Revision: 307428

URL: http://llvm.org/viewvc/llvm-project?rev=307428&view=rev
Log:
[RegAllocFast] Don't insert kill flags of super-register for partial kill

When reusing a register for a new definition, the fast register allocator
used to insert a kill flag at the previous last use of that register to
inform later passes that this register is free between the redef and the
last use. However, this may be wrong when subregisters are involved.
Indeed, a partially redef would have trigger a kill of the full super
register, potentially wrongly marking all the other subregisters as
free. Given we don't track which lanes are still live, we cannot set the
kill flag in such case.

Note: This bug has been latent for about 7 years (r104056).

llvmg.org/PR33677

Added:
    llvm/trunk/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir
Modified:
    llvm/trunk/lib/CodeGen/RegAllocFast.cpp

Modified: llvm/trunk/lib/CodeGen/RegAllocFast.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocFast.cpp?rev=307428&r1=307427&r2=307428&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/RegAllocFast.cpp (original)
+++ llvm/trunk/lib/CodeGen/RegAllocFast.cpp Fri Jul  7 12:25:45 2017
@@ -246,8 +246,15 @@ void RAFast::addKillFlag(const LiveReg &
   if (MO.isUse() && !LR.LastUse->isRegTiedToDefOperand(LR.LastOpNum)) {
     if (MO.getReg() == LR.PhysReg)
       MO.setIsKill();
-    else
-      LR.LastUse->addRegisterKilled(LR.PhysReg, TRI, true);
+    // else, don't do anything we are problably redefining a
+    // subreg of this register and given we don't track which
+    // lanes are actually dead, we cannot insert a kill flag here.
+    // Otherwise we may end up in a situation like this:
+    // ... = (MO) physreg:sub1, physreg <implicit-use, kill>
+    // ... <== Here we would allow later pass to reuse physreg:sub1
+    //         which is potentially wrong.
+    // LR:sub0 = ...
+    // ... = LR.sub1 <== This is going to use physreg:sub1
   }
 }
 

Added: llvm/trunk/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir?rev=307428&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir (added)
+++ llvm/trunk/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir Fri Jul  7 12:25:45 2017
@@ -0,0 +1,34 @@
+# RUN: llc -verify-machineinstrs -run-pass regallocfast -mtriple s390x-ibm-linux -o - %s | FileCheck %s
+--- |
+  
+  @g_167 = external global [5 x i64], align 8
+  define void @main() local_unnamed_addr {
+    ret void
+  }
+...
+# Make sure the usage of different subregisters on the same virtual register
+# does not result in invalid kill flags.
+# PR33677
+---
+name:            main
+alignment:       2
+tracksRegLiveness: true
+registers:       
+  - { id: 0, class: gr128bit }
+  - { id: 1, class: gr64bit }
+  - { id: 2, class: addr64bit }
+# CHECK: %r0q = L128
+# CHECK-NEXT: %r0l = COPY %r1l
+# Although R0L partially redefines R0Q, it must not mark R0Q as kill
+# because R1D is still live through that instruction.
+# CHECK-NOT: %r0q<imp-use,kill>
+# CHECK-NEXT: %r2d = COPY %r1d
+# CHECK-NEXT: LARL
+body:             |
+  bb.0:
+    %0.subreg_hl32 = COPY %0.subreg_l32
+    %1 = COPY %0.subreg_l64
+    %2 = LARL @g_167
+    STC %1.subreg_l32, %2, 8, _
+
+...