From: Mandeep Singh Grang Date: Tue, 10 May 2016 17:57:27 +0000 (+0000) Subject: Fix PR26655: Bail out if all regs of an inst BUNDLE have the correct kill flag X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=efde4d38acaab4fc67818b04843458b8bc5d7cce;p=llvm 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 6b46cb356c2..8e8af1b37b3 100644 --- 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 282019f3037..e9ab6a24f8a 100644 --- 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 index 00000000000..79878598893 --- /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 +}