Reapply r160194, switching to use LV information for finding local kills.

The notable fix is to look at any dependencies attached to the kill
instruction (or other instructions between MI nad the kill) where the
dependencies are specific to the register in question.

The old code implicitly handled this by rejecting the transform if *any*
other uses were found within the block, but after the start point. The
new code directly finds the kill, and has to re-use the existing
dependency scan to check for non-kill uses.

This was caught by self-host, but I found the bug via inspection and use
of absurd assert scaffolding to compute the kills in two ways and
compare them. So I have no useful testcase for this other than
"bootstrap". I'd work harder to reduce a test case if this particular
code were likely to live for a long time.

Thanks to Benjamin Kramer for reviewing the fix itself.

llvm-svn: 160228
This commit is contained in:
Chandler Carruth 2012-07-15 03:29:46 +00:00
parent faf556b745
commit db5536f09d
1 changed files with 32 additions and 56 deletions

View File

@ -483,32 +483,6 @@ static bool isTwoAddrUse(MachineInstr &MI, unsigned Reg, unsigned &DstReg) {
return false; return false;
} }
/// findLocalKill - Look for an instruction below MI in the MBB that kills the
/// specified register. Returns null if there are any other Reg use between the
/// instructions.
static
MachineInstr *findLocalKill(unsigned Reg, MachineBasicBlock *MBB,
MachineInstr *MI, MachineRegisterInfo *MRI,
DenseMap<MachineInstr*, unsigned> &DistanceMap) {
MachineInstr *KillMI = 0;
for (MachineRegisterInfo::use_nodbg_iterator
UI = MRI->use_nodbg_begin(Reg),
UE = MRI->use_nodbg_end(); UI != UE; ++UI) {
MachineInstr *UseMI = &*UI;
if (UseMI == MI || UseMI->getParent() != MBB)
continue;
if (DistanceMap.count(UseMI))
continue;
if (!UI.getOperand().isKill())
return 0;
if (KillMI)
return 0; // -O0 kill markers cannot be trusted?
KillMI = UseMI;
}
return KillMI;
}
/// findOnlyInterestingUse - Given a register, if has a single in-basic block /// findOnlyInterestingUse - Given a register, if has a single in-basic block
/// use, return the use instruction if it's a copy or a two-address use. /// use, return the use instruction if it's a copy or a two-address use.
static static
@ -905,14 +879,19 @@ TwoAddressInstructionPass::RescheduleMIBelowKill(MachineBasicBlock *MBB,
MachineBasicBlock::iterator &mi, MachineBasicBlock::iterator &mi,
MachineBasicBlock::iterator &nmi, MachineBasicBlock::iterator &nmi,
unsigned Reg) { unsigned Reg) {
// Bail immediately if we don't have LV available. We use it to find kills
// efficiently.
if (!LV)
return false;
MachineInstr *MI = &*mi; MachineInstr *MI = &*mi;
DenseMap<MachineInstr*, unsigned>::iterator DI = DistanceMap.find(MI); DenseMap<MachineInstr*, unsigned>::iterator DI = DistanceMap.find(MI);
if (DI == DistanceMap.end()) if (DI == DistanceMap.end())
// Must be created from unfolded load. Don't waste time trying this. // Must be created from unfolded load. Don't waste time trying this.
return false; return false;
MachineInstr *KillMI = findLocalKill(Reg, MBB, mi, MRI, DistanceMap); MachineInstr *KillMI = LV->getVarInfo(Reg).findKill(MBB);
if (!KillMI || KillMI->isCopy() || KillMI->isCopyLike()) if (!KillMI || MI == KillMI || KillMI->isCopy() || KillMI->isCopyLike())
// Don't mess with copies, they may be coalesced later. // Don't mess with copies, they may be coalesced later.
return false; return false;
@ -999,6 +978,12 @@ TwoAddressInstructionPass::RescheduleMIBelowKill(MachineBasicBlock *MBB,
((MO.isKill() && Uses.count(MOReg)) || Kills.count(MOReg))) ((MO.isKill() && Uses.count(MOReg)) || Kills.count(MOReg)))
// Don't want to extend other live ranges and update kills. // Don't want to extend other live ranges and update kills.
return false; return false;
if (MOReg == Reg && !MO.isKill())
// We can't schedule across a use of the register in question.
return false;
// Ensure that if this is register in question, its the kill we expect.
assert((MOReg != Reg || OtherMI == KillMI) &&
"Found multiple kills of a register in a basic block");
} }
} }
} }
@ -1012,19 +997,9 @@ TwoAddressInstructionPass::RescheduleMIBelowKill(MachineBasicBlock *MBB,
MBB->splice(KillPos, MBB, From, To); MBB->splice(KillPos, MBB, From, To);
DistanceMap.erase(DI); DistanceMap.erase(DI);
if (LV) { // Update live variables
// Update live variables LV->removeVirtualRegisterKilled(Reg, KillMI);
LV->removeVirtualRegisterKilled(Reg, KillMI); LV->addVirtualRegisterKilled(Reg, MI);
LV->addVirtualRegisterKilled(Reg, MI);
} else {
for (unsigned i = 0, e = KillMI->getNumOperands(); i != e; ++i) {
MachineOperand &MO = KillMI->getOperand(i);
if (!MO.isReg() || !MO.isUse() || MO.getReg() != Reg)
continue;
MO.setIsKill(false);
}
MI->addRegisterKilled(Reg, 0);
}
return true; return true;
} }
@ -1061,14 +1036,19 @@ TwoAddressInstructionPass::RescheduleKillAboveMI(MachineBasicBlock *MBB,
MachineBasicBlock::iterator &mi, MachineBasicBlock::iterator &mi,
MachineBasicBlock::iterator &nmi, MachineBasicBlock::iterator &nmi,
unsigned Reg) { unsigned Reg) {
// Bail immediately if we don't have LV available. We use it to find kills
// efficiently.
if (!LV)
return false;
MachineInstr *MI = &*mi; MachineInstr *MI = &*mi;
DenseMap<MachineInstr*, unsigned>::iterator DI = DistanceMap.find(MI); DenseMap<MachineInstr*, unsigned>::iterator DI = DistanceMap.find(MI);
if (DI == DistanceMap.end()) if (DI == DistanceMap.end())
// Must be created from unfolded load. Don't waste time trying this. // Must be created from unfolded load. Don't waste time trying this.
return false; return false;
MachineInstr *KillMI = findLocalKill(Reg, MBB, mi, MRI, DistanceMap); MachineInstr *KillMI = LV->getVarInfo(Reg).findKill(MBB);
if (!KillMI || KillMI->isCopy() || KillMI->isCopyLike()) if (!KillMI || MI == KillMI || KillMI->isCopy() || KillMI->isCopyLike())
// Don't mess with copies, they may be coalesced later. // Don't mess with copies, they may be coalesced later.
return false; return false;
@ -1094,6 +1074,8 @@ TwoAddressInstructionPass::RescheduleKillAboveMI(MachineBasicBlock *MBB,
continue; continue;
if (isDefTooClose(MOReg, DI->second, MI, MBB)) if (isDefTooClose(MOReg, DI->second, MI, MBB))
return false; return false;
if (MOReg == Reg && !MO.isKill())
return false;
Uses.insert(MOReg); Uses.insert(MOReg);
if (MO.isKill() && MOReg != Reg) if (MO.isKill() && MOReg != Reg)
Kills.insert(MOReg); Kills.insert(MOReg);
@ -1135,6 +1117,9 @@ TwoAddressInstructionPass::RescheduleKillAboveMI(MachineBasicBlock *MBB,
if (Kills.count(MOReg)) if (Kills.count(MOReg))
// Don't want to extend other live ranges and update kills. // Don't want to extend other live ranges and update kills.
return false; return false;
if (OtherMI != MI && MOReg == Reg && !MO.isKill())
// We can't schedule across a use of the register in question.
return false;
} else { } else {
OtherDefs.push_back(MOReg); OtherDefs.push_back(MOReg);
} }
@ -1165,19 +1150,10 @@ TwoAddressInstructionPass::RescheduleKillAboveMI(MachineBasicBlock *MBB,
nmi = llvm::prior(InsertPos); // Backtrack so we process the moved instr. nmi = llvm::prior(InsertPos); // Backtrack so we process the moved instr.
DistanceMap.erase(DI); DistanceMap.erase(DI);
if (LV) { // Update live variables
// Update live variables LV->removeVirtualRegisterKilled(Reg, KillMI);
LV->removeVirtualRegisterKilled(Reg, KillMI); LV->addVirtualRegisterKilled(Reg, MI);
LV->addVirtualRegisterKilled(Reg, MI);
} else {
for (unsigned i = 0, e = KillMI->getNumOperands(); i != e; ++i) {
MachineOperand &MO = KillMI->getOperand(i);
if (!MO.isReg() || !MO.isUse() || MO.getReg() != Reg)
continue;
MO.setIsKill(false);
}
MI->addRegisterKilled(Reg, 0);
}
return true; return true;
} }