[llvm] r223133 - [MachineCSE] Clear kill-flag on registers imp-def'd by the CSE'd instruction.

[Ahmed Bougacha]

Author: ab
Date: Tue Dec  2 12:09:51 2014
New Revision: 223133

URL: http://llvm.org/viewvc/llvm-project?rev=223133&view=rev
Log:
[MachineCSE] Clear kill-flag on registers imp-def'd by the CSE'd instruction.

Go through implicit defs of CSMI and MI, and clear the kill flags on
their uses in all the instructions between CSMI and MI.
We might have made some of the kill flags redundant, consider:
  subs  ... %NZCV<imp-def>        <- CSMI
  csinc ... %NZCV<imp-use,kill>   <- this kill flag isn't valid anymore
  subs  ... %NZCV<imp-def>        <- MI, to be eliminated
  csinc ... %NZCV<imp-use,kill>
Since we eliminated MI, and reused a register imp-def'd by CSMI
(here %NZCV), that register, if it was killed before MI, should have
that kill flag removed, because it's lifetime was extended.

Also, add an exhaustive testcase for the motivating example.

Reviewed by: Juergen Ributzka <juergen at apple.com>

Added:
    llvm/trunk/test/CodeGen/AArch64/machine_cse_impdef_killflags.ll
Modified:
    llvm/trunk/lib/CodeGen/MachineCSE.cpp

Modified: llvm/trunk/lib/CodeGen/MachineCSE.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCSE.cpp?rev=223133&r1=223132&r2=223133&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineCSE.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineCSE.cpp Tue Dec  2 12:09:51 2014
@@ -451,6 +451,7 @@ bool MachineCSE::ProcessBlock(MachineBas
 
   SmallVector<std::pair<unsigned, unsigned>, 8> CSEPairs;
   SmallVector<unsigned, 2> ImplicitDefsToUpdate;
+  SmallVector<unsigned, 2> ImplicitDefs;
   for (MachineBasicBlock::iterator I = MBB->begin(), E = MBB->end(); I != E; ) {
     MachineInstr *MI = &*I;
     ++I;
@@ -542,6 +543,12 @@ bool MachineCSE::ProcessBlock(MachineBas
       // we should make sure it is not dead at CSMI.
       if (MO.isImplicit() && !MO.isDead() && CSMI->getOperand(i).isDead())
         ImplicitDefsToUpdate.push_back(i);
+
+      // Keep track of implicit defs of CSMI and MI, to clear possibly
+      // made-redundant kill flags.
+      if (MO.isImplicit() && !MO.isDead() && OldReg == NewReg)
+        ImplicitDefs.push_back(OldReg);
+
       if (OldReg == NewReg) {
         --NumDefs;
         continue;
@@ -582,6 +589,29 @@ bool MachineCSE::ProcessBlock(MachineBas
       for (unsigned i = 0, e = ImplicitDefsToUpdate.size(); i != e; ++i)
         CSMI->getOperand(ImplicitDefsToUpdate[i]).setIsDead(false);
 
+      // Go through implicit defs of CSMI and MI, and clear the kill flags on
+      // their uses in all the instructions between CSMI and MI.
+      // We might have made some of the kill flags redundant, consider:
+      //   subs  ... %NZCV<imp-def>        <- CSMI
+      //   csinc ... %NZCV<imp-use,kill>   <- this kill flag isn't valid anymore
+      //   subs  ... %NZCV<imp-def>        <- MI, to be eliminated
+      //   csinc ... %NZCV<imp-use,kill>
+      // Since we eliminated MI, and reused a register imp-def'd by CSMI
+      // (here %NZCV), that register, if it was killed before MI, should have
+      // that kill flag removed, because it's lifetime was extended.
+      if (CSMI->getParent() == MI->getParent()) {
+        for (MachineBasicBlock::iterator II = CSMI, IE = MI; II != IE; ++II)
+          for (auto ImplicitDef : ImplicitDefs)
+            if (MachineOperand *MO = II->findRegisterUseOperand(
+                    ImplicitDef, /*isKill=*/true, TRI))
+              MO->setIsKill(false);
+      } else {
+        // If the instructions aren't in the same BB, bail out and clear the
+        // kill flag on all uses of the imp-def'd register.
+        for (auto ImplicitDef : ImplicitDefs)
+          MRI->clearKillFlags(ImplicitDef);
+      }
+
       if (CrossMBBPhysDef) {
         // Add physical register defs now coming in from a predecessor to MBB
         // livein list.
@@ -606,6 +636,7 @@ bool MachineCSE::ProcessBlock(MachineBas
     }
     CSEPairs.clear();
     ImplicitDefsToUpdate.clear();
+    ImplicitDefs.clear();
   }
 
   return Changed;

Added: llvm/trunk/test/CodeGen/AArch64/machine_cse_impdef_killflags.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/machine_cse_impdef_killflags.ll?rev=223133&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/machine_cse_impdef_killflags.ll (added)
+++ llvm/trunk/test/CodeGen/AArch64/machine_cse_impdef_killflags.ll Tue Dec  2 12:09:51 2014
@@ -0,0 +1,26 @@
+; RUN: llc < %s -mtriple=aarch64-apple-ios -fast-isel -verify-machineinstrs | FileCheck %s
+
+; Check that the kill flag is cleared between CSE'd instructions on their
+; imp-def'd registers.
+; The verifier would complain otherwise.
+define i64 @csed-impdef-killflag(i64 %a) {
+; CHECK-LABEL: csed-impdef-killflag
+; CHECK-DAG:  mov    [[REG0:w[0-9]+]], wzr
+; CHECK-DAG:  orr    [[REG1:w[0-9]+]], wzr, #0x1
+; CHECK-DAG:  orr    [[REG2:x[0-9]+]], xzr, #0x2
+; CHECK-DAG:  orr    [[REG3:x[0-9]+]], xzr, #0x3
+; CHECK:      cmp    x0, #0
+; CHECK-DAG:  csel   w[[SELECT_WREG_1:[0-9]+]], [[REG0]], [[REG1]], ne
+; CHECK-DAG:  csel   [[SELECT_XREG_2:x[0-9]+]], [[REG2]], [[REG3]], ne
+; CHECK:      ubfx   [[SELECT_XREG_1:x[0-9]+]], x[[SELECT_WREG_1]], #0, #32
+; CHECK-NEXT: add    x0, [[SELECT_XREG_2]], [[SELECT_XREG_1]]
+; CHECK-NEXT: ret
+
+  %1 = icmp ne i64 %a, 0
+  %2 = select i1 %1, i32 0, i32 1
+  %3 = icmp ne i64 %a, 0
+  %4 = select i1 %3, i64 2, i64 3
+  %5 = zext i32 %2 to i64
+  %6 = add i64 %4, %5
+  ret i64 %6
+}