From 19f5f71bba08e690611fa213647ac6bae814756b Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Fri, 21 May 2010 17:36:32 +0000 Subject: [PATCH] Revert "Use MachineInstr::readsWritesVirtualRegister to determine if a register is read." This reverts r104322. I think it was causing miscompilations. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@104323 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/MachineInstr.h | 12 +------ lib/CodeGen/LiveIntervalAnalysis.cpp | 54 +++++++++++++++++++++++----- lib/CodeGen/MachineInstr.cpp | 26 ++++++-------- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h index cd72344dc71..5e62f0bcfb8 100644 --- a/include/llvm/CodeGen/MachineInstr.h +++ b/include/llvm/CodeGen/MachineInstr.h @@ -28,7 +28,6 @@ namespace llvm { -template class SmallVectorImpl; class AliasAnalysis; class TargetInstrDesc; class TargetInstrInfo; @@ -240,16 +239,7 @@ public: /// readsVirtualRegister - Return true if the MachineInstr reads the specified /// virtual register. Take into account that a partial define is a /// read-modify-write operation. - bool readsVirtualRegister(unsigned Reg) const { - return readsWritesVirtualRegister(Reg).first; - } - - /// readsWritesVirtualRegister - Return a pair of bools (reads, writes) - /// indicating if this instruction reads or writes Reg. This also considers - /// partial defines. - /// If Ops is not null, all operand indices for Reg are added. - std::pair readsWritesVirtualRegister(unsigned Reg, - SmallVectorImpl *Ops = 0) const; + bool readsVirtualRegister(unsigned Reg) const; /// killsRegister - Return true if the MachineInstr kills the specified /// register. If TargetRegisterInfo is passed, then it also checks if there is diff --git a/lib/CodeGen/LiveIntervalAnalysis.cpp b/lib/CodeGen/LiveIntervalAnalysis.cpp index 5b1c36932d2..c07802ee26b 100644 --- a/lib/CodeGen/LiveIntervalAnalysis.cpp +++ b/lib/CodeGen/LiveIntervalAnalysis.cpp @@ -1098,6 +1098,7 @@ rewriteInstructionForSpills(const LiveInterval &li, const VNInfo *VNI, if (!mop.isReg()) continue; unsigned Reg = mop.getReg(); + unsigned RegI = Reg; if (Reg == 0 || TargetRegisterInfo::isPhysicalRegister(Reg)) continue; if (Reg != li.reg) @@ -1139,8 +1140,26 @@ rewriteInstructionForSpills(const LiveInterval &li, const VNInfo *VNI, // // Keep track of whether we replace a use and/or def so that we can // create the spill interval with the appropriate range. + + HasUse = mop.isUse(); + HasDef = mop.isDef(); SmallVector Ops; - tie(HasUse, HasDef) = MI->readsWritesVirtualRegister(Reg, &Ops); + Ops.push_back(i); + for (unsigned j = i+1, e = MI->getNumOperands(); j != e; ++j) { + const MachineOperand &MOj = MI->getOperand(j); + if (!MOj.isReg()) + continue; + unsigned RegJ = MOj.getReg(); + if (RegJ == 0 || TargetRegisterInfo::isPhysicalRegister(RegJ)) + continue; + if (RegJ == RegI) { + Ops.push_back(j); + if (!MOj.isUndef()) { + HasUse |= MOj.isUse(); + HasDef |= MOj.isDef(); + } + } + } // Create a new virtual register for the spill interval. // Create the new register now so we can map the fold instruction @@ -1293,7 +1312,10 @@ namespace { struct RewriteInfo { SlotIndex Index; MachineInstr *MI; - RewriteInfo(SlotIndex i, MachineInstr *mi) : Index(i), MI(mi) {} + bool HasUse; + bool HasDef; + RewriteInfo(SlotIndex i, MachineInstr *mi, bool u, bool d) + : Index(i), MI(mi), HasUse(u), HasDef(d) {} }; struct RewriteInfoCompare { @@ -1372,7 +1394,7 @@ rewriteInstructionsForSpills(const LiveInterval &li, bool TrySplit, // easily see a situation where both registers are reloaded before // the INSERT_SUBREG and both target registers that would overlap. continue; - RewriteMIs.push_back(RewriteInfo(index, MI)); + RewriteMIs.push_back(RewriteInfo(index, MI, O.isUse(), O.isDef())); } std::sort(RewriteMIs.begin(), RewriteMIs.end(), RewriteInfoCompare()); @@ -1382,11 +1404,18 @@ rewriteInstructionsForSpills(const LiveInterval &li, bool TrySplit, RewriteInfo &rwi = RewriteMIs[i]; ++i; SlotIndex index = rwi.Index; + bool MIHasUse = rwi.HasUse; + bool MIHasDef = rwi.HasDef; MachineInstr *MI = rwi.MI; // If MI def and/or use the same register multiple times, then there // are multiple entries. + unsigned NumUses = MIHasUse; while (i != e && RewriteMIs[i].MI == MI) { assert(RewriteMIs[i].Index == index); + bool isUse = RewriteMIs[i].HasUse; + if (isUse) ++NumUses; + MIHasUse |= isUse; + MIHasDef |= RewriteMIs[i].HasDef; ++i; } MachineBasicBlock *MBB = MI->getParent(); @@ -1411,8 +1440,7 @@ rewriteInstructionsForSpills(const LiveInterval &li, bool TrySplit, // = use // It's better to start a new interval to avoid artifically // extend the new interval. - if (MI->readsWritesVirtualRegister(li.reg) == - std::make_pair(false,true)) { + if (MIHasDef && !MIHasUse) { MBBVRegsMap.erase(MBB->getNumber()); ThisVReg = 0; } @@ -1646,9 +1674,19 @@ addIntervalsForSpillsFast(const LiveInterval &li, MachineInstr* MI = &*RI; SmallVector Indices; - bool HasUse, HasDef; - tie(HasUse, HasDef) = MI->readsWritesVirtualRegister(li.reg, &Indices); - + bool HasUse = false; + bool HasDef = false; + + for (unsigned i = 0; i != MI->getNumOperands(); ++i) { + MachineOperand& mop = MI->getOperand(i); + if (!mop.isReg() || mop.getReg() != li.reg) continue; + + HasUse |= MI->getOperand(i).isUse(); + HasDef |= MI->getOperand(i).isDef(); + + Indices.push_back(i); + } + if (!tryFoldMemoryOperand(MI, vrm, NULL, getInstructionIndex(MI), Indices, true, slot, li.reg)) { unsigned NewVReg = mri_->createVirtualRegister(rc); diff --git a/lib/CodeGen/MachineInstr.cpp b/lib/CodeGen/MachineInstr.cpp index 319059b4bbf..595fddda6f6 100644 --- a/lib/CodeGen/MachineInstr.cpp +++ b/lib/CodeGen/MachineInstr.cpp @@ -782,31 +782,27 @@ int MachineInstr::findRegisterUseOperandIdx(unsigned Reg, bool isKill, return -1; } -/// readsWritesVirtualRegister - Return a pair of bools (reads, writes) -/// indicating if this instruction reads or writes Reg. This also considers -/// partial defines. -std::pair -MachineInstr::readsWritesVirtualRegister(unsigned Reg, - SmallVectorImpl *Ops) const { - bool PartDef = false; // Partial redefine. - bool FullDef = false; // Full define. - bool Use = false; +/// readsVirtualRegister - Return true if the MachineInstr reads the specified +/// virtual register. Take into account that a partial define is a +/// read-modify-write operation. +bool MachineInstr::readsVirtualRegister(unsigned Reg) const { + bool PartDef = false; // Partial redefine + bool FullDef = false; // Full define for (unsigned i = 0, e = getNumOperands(); i != e; ++i) { const MachineOperand &MO = getOperand(i); if (!MO.isReg() || MO.getReg() != Reg) continue; - if (Ops) - Ops->push_back(i); if (MO.isUse()) - Use |= !MO.isUndef(); - else if (MO.getSubReg()) + return true; + if (MO.getSubReg()) PartDef = true; else FullDef = true; } - // A partial redefine uses Reg unless there is also a full define. - return std::make_pair(Use || (PartDef && !FullDef), PartDef || FullDef); + // A partial register definition causes a read unless the full register is + // also defined. + return PartDef && !FullDef; } /// findRegisterDefOperandIdx() - Returns the operand index that is a def of