From 868ef847a627b95c93d0eac5080cdcc3f989f56e Mon Sep 17 00:00:00 2001
From: Quentin Colombet <qcolombet@apple.com>
Date: Fri, 7 Jul 2017 19:25:45 +0000
Subject: [PATCH] [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

llvm-svn: 307428
---
 llvm/lib/CodeGen/RegAllocFast.cpp                  | 11 +++++--
 .../SystemZ/regalloc-fast-invalid-kill-flag.mir    | 34 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 llvm/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir

diff --git a/llvm/lib/CodeGen/RegAllocFast.cpp b/llvm/lib/CodeGen/RegAllocFast.cpp
index 7d9bc8c..d5538be 100644
--- a/llvm/lib/CodeGen/RegAllocFast.cpp
+++ b/llvm/lib/CodeGen/RegAllocFast.cpp
@@ -246,8 +246,15 @@ void RAFast::addKillFlag(const LiveReg &LR) {
   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
   }
 }
 
diff --git a/llvm/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir b/llvm/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir
new file mode 100644
index 0000000..8798fce
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir
@@ -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, _
+
+...
-- 
2.7.4