Fix PR26655: Bail out if all regs of an inst BUNDLE have the correct kill flag

Summary:
While setting kill flags on instructions inside a BUNDLE, we bail out as soon
as we set kill flag on a register.  But we are missing a check when all the
registers already have the correct kill flag set. We need to bail out in that
case as well.

This patch refactors the old code and simply makes use of the addRegisterKilled
function in MachineInstr.cpp in order to determine whether to set/remove kill
on an instruction.

Reviewers: apazos, t.p.northover, pete, MatzeB

Subscribers: MatzeB, davide, llvm-commits

Differential Revision: http://reviews.llvm.org/D17356

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@269092 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/MachineInstr.cpp b/lib/CodeGen/MachineInstr.cpp
index 6b46cb356c2560c02476da1abe9ad17e1eafb3f7..8e8af1b37b3f35b3ce15c80f5868c1fca4d3dd33 100644 (file)
--- a/lib/CodeGen/MachineInstr.cpp
+++ b/lib/CodeGen/MachineInstr.cpp
@@ -1952,6 +1952,13 @@ bool MachineInstr::addRegisterKilled(unsigned IncomingReg,
     MachineOperand &MO = getOperand(i);
     if (!MO.isReg() || !MO.isUse() || MO.isUndef())
       continue;
+
+    // DEBUG_VALUE nodes do not contribute to code generation and should
+    // always be ignored. Failure to do so may result in trying to modify
+    // KILL flags on DEBUG_VALUE nodes.
+    if (MO.isDebug())
+      continue;
+
     unsigned Reg = MO.getReg();
     if (!Reg)
       continue;

diff --git a/lib/CodeGen/ScheduleDAGInstrs.cpp b/lib/CodeGen/ScheduleDAGInstrs.cpp
index 282019f3037235adba62b1b64e031ef968347ec1..e9ab6a24f8ac59f6e801b308194a082afdcdec61 100644 (file)
--- a/lib/CodeGen/ScheduleDAGInstrs.cpp
+++ b/lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -1213,7 +1213,8 @@ void ScheduleDAGInstrs::startBlockForKills(MachineBasicBlock *BB) {
 /// operands, then we also need to propagate that to any instructions inside
 /// the bundle which had the same kill state.
 static void toggleBundleKillFlag(MachineInstr *MI, unsigned Reg,
-                                 bool NewKillState) {
+                                 bool NewKillState,
+                                 const TargetRegisterInfo *TRI) {
   if (MI->getOpcode() != TargetOpcode::BUNDLE)
     return;
 
@@ -1224,28 +1225,11 @@ static void toggleBundleKillFlag(MachineInstr *MI, unsigned Reg,
   MachineBasicBlock::instr_iterator Begin = MI->getIterator();
   MachineBasicBlock::instr_iterator End = getBundleEnd(*MI);
   while (Begin != End) {
-    for (MachineOperand &MO : (--End)->operands()) {
-      if (!MO.isReg() || MO.isDef() || Reg != MO.getReg())
-        continue;
-
-      // DEBUG_VALUE nodes do not contribute to code generation and should
-      // always be ignored.  Failure to do so may result in trying to modify
-      // KILL flags on DEBUG_VALUE nodes, which is distressing.
-      if (MO.isDebug())
-        continue;
-
-      // If the register has the internal flag then it could be killing an
-      // internal def of the register.  In this case, just skip.  We only want
-      // to toggle the flag on operands visible outside the bundle.
-      if (MO.isInternalRead())
-        continue;
-
-      if (MO.isKill() == NewKillState)
-        continue;
-      MO.setIsKill(NewKillState);
-      if (NewKillState)
-        return;
-    }
+    if (NewKillState) {
+      if ((--End)->addRegisterKilled(Reg, TRI, /* addIfNotFound= */ false))
+         return;
+    } else
+        (--End)->clearRegisterKills(Reg, TRI);
   }
 }
 
@@ -1253,21 +1237,21 @@ bool ScheduleDAGInstrs::toggleKillFlag(MachineInstr *MI, MachineOperand &MO) {
   // Setting kill flag...
   if (!MO.isKill()) {
     MO.setIsKill(true);
-    toggleBundleKillFlag(MI, MO.getReg(), true);
+    toggleBundleKillFlag(MI, MO.getReg(), true, TRI);
     return false;
   }
 
   // If MO itself is live, clear the kill flag...
   if (LiveRegs.test(MO.getReg())) {
     MO.setIsKill(false);
-    toggleBundleKillFlag(MI, MO.getReg(), false);
+    toggleBundleKillFlag(MI, MO.getReg(), false, TRI);
     return false;
   }
 
   // If any subreg of MO is live, then create an imp-def for that
   // subreg and keep MO marked as killed.
   MO.setIsKill(false);
-  toggleBundleKillFlag(MI, MO.getReg(), false);
+  toggleBundleKillFlag(MI, MO.getReg(), false, TRI);
   bool AllDead = true;
   const unsigned SuperReg = MO.getReg();
   MachineInstrBuilder MIB(MF, MI);
@@ -1280,7 +1264,7 @@ bool ScheduleDAGInstrs::toggleKillFlag(MachineInstr *MI, MachineOperand &MO) {
 
   if(AllDead) {
     MO.setIsKill(true);
-    toggleBundleKillFlag(MI, MO.getReg(), true);
+    toggleBundleKillFlag(MI, MO.getReg(), true, TRI);
   }
   return false;
 }

diff --git a/test/CodeGen/Thumb2/thumb2-cpsr-liveness.ll b/test/CodeGen/Thumb2/thumb2-cpsr-liveness.ll
new file mode 100644 (file)
index 0000000..7987859
--- /dev/null
+++ b/test/CodeGen/Thumb2/thumb2-cpsr-liveness.ll
@@ -0,0 +1,41 @@
+; RUN: llc < %s -mtriple=thumbv7-linux-gnueabi -misched-postra=true
+
+define i32 @test_cpsr() {
+entry:
+  %a = alloca [10 x i32], align 4
+  %0 = bitcast [10 x i32]* %a to i8*
+  %arrayidx.gep = getelementptr [10 x i32], [10 x i32]* %a, i32 0, i32 0
+  br label %for.body
+
+for.cond.cleanup:
+  %c.1.reg2mem.0.lcssa = phi i32 [ %c.1.reg2mem.0, %for.inc ]
+  ret i32 %c.1.reg2mem.0.lcssa
+
+for.body:
+  %1 = phi i32 [ 0, %entry ], [ %.pre, %for.inc.for.body_crit_edge ]
+  %c.018.reg2mem.0 = phi i32 [ 0, %entry ], [ %c.1.reg2mem.0, %for.inc.for.body_crit_edge ]
+  %b.017.reg2mem.0 = phi double [ 0.000000e+00, %entry ], [ %b.1.reg2mem.0, %for.inc.for.body_crit_edge ]
+  %arrayidx.phi = phi i32* [ %arrayidx.gep, %entry ], [ %arrayidx.inc, %for.inc.for.body_crit_edge ]
+  %i.019 = phi i32 [ 0, %entry ], [ %inc, %for.inc.for.body_crit_edge ]
+  %cmp1 = icmp slt i32 %1, 10
+  %arrayidx.inc = getelementptr i32, i32* %arrayidx.phi, i32 1
+  br i1 %cmp1, label %for.inc, label %if.end
+
+if.end:
+  %conv = sitofp i32 %i.019 to double
+  %cmp2 = fcmp nsz ogt double %conv, %b.017.reg2mem.0
+  %selv = select i1 %cmp2, double %conv, double %b.017.reg2mem.0
+  %selv7 = select i1 %cmp2, i32 %i.019, i32 %c.018.reg2mem.0
+  br label %for.inc
+
+for.inc:
+  %b.1.reg2mem.0 = phi double [ %b.017.reg2mem.0, %for.body ], [ %selv, %if.end ]
+  %c.1.reg2mem.0 = phi i32 [ %c.018.reg2mem.0, %for.body ], [ %selv7, %if.end ]
+  %exitcond = icmp eq i32 %i.019, 9
+  br i1 %exitcond, label %for.cond.cleanup, label %for.inc.for.body_crit_edge
+
+for.inc.for.body_crit_edge:
+  %inc = add nuw nsw i32 %i.019, 1
+  %.pre = load i32, i32* %arrayidx.inc, align 4
+  br label %for.body
+}