Index: lib/CodeGen/RegisterCoalescer.cpp =================================================================== --- lib/CodeGen/RegisterCoalescer.cpp +++ lib/CodeGen/RegisterCoalescer.cpp @@ -265,14 +265,6 @@ /// Return true if a copy involving a physreg should be joined. bool canJoinPhys(const CoalescerPair &CP); - /// When merging SrcReg and DstReg together, and the operand of the - /// specified DBG_VALUE refers to one of them, would the def that a - /// DBG_VALUE refers to change? This can happen when the DBG_VALUEs - /// operand is dead and it's merged into a different live value, - /// meaning the DBG_VALUE operands must be updated. - bool mergingChangesDbgValue(MachineInstr *DbgV, unsigned SrcReg, - unsigned DstReg) const; - /// Replace all defs and uses of SrcReg to DstReg and update the subregister /// number if it is not zero. If DstReg is a physical register and the /// existing subregister number of the def / use being updated is not zero, @@ -1656,58 +1648,6 @@ } } -bool RegisterCoalescer::mergingChangesDbgValue(MachineInstr *DbgV, - unsigned SrcReg, - unsigned DstReg) const { - assert(DbgV->isDebugValue()); - assert(DbgV->getParent() && "DbgValue with no parent"); - assert(DbgV->getOperand(0).isReg()); - unsigned DbgReg = DbgV->getOperand(0).getReg(); - - SlotIndex MIIdx = LIS->getSlotIndexes()->getIndexAfter(*DbgV); - const LiveInterval &SrcLI = LIS->getInterval(SrcReg); - - // Is the source register live across the DBG_VALUE? - bool SrcLive = false; - auto LII = SrcLI.find(MIIdx); - if (LII != SrcLI.end() && LII->contains(MIIdx)) - SrcLive = true; - - bool DstLive = false; - // Destination register can be physical or virtual. - if (TargetRegisterInfo::isVirtualRegister(DstReg)) { - // Is DstReg live across the DBG_VALUE? - const LiveInterval &DstLI = LIS->getInterval(DstReg); - LII = DstLI.find(MIIdx); - DstLive = (LII != DstLI.end() && LII->contains(MIIdx)); - } else if (MRI->isConstantPhysReg(DstReg)) { - // Constant physical registers are always live. - DstLive = true; - } else { - // For physical registers, see if any register unit containing DstReg - // is live across the DBG_VALUE. - for (MCRegUnitIterator UI(DstReg, TRI); UI.isValid(); ++UI) { - const LiveRange &DstLI = LIS->getRegUnit(*UI); - auto DstLII = DstLI.find(MIIdx); - if (DstLII != DstLI.end() && DstLII->contains(MIIdx)) { - DstLive = true; - break; - } - } - } - - // We now know whether src and dst are live. Best case: we have a DBG_VALUE - // of a live register, coalesing won't change its value. - if ((DstLive && DbgReg == DstReg) || (SrcLive && DbgReg == SrcReg)) - return false; - // If neither register are live, no damage done. - if (!DstLive && !SrcLive) - return false; - // Otherwise, we will end up resurrecting the DBG_VALUE with a different - // register, which is unsafe. - return true; -} - void RegisterCoalescer::updateRegDefsUses(unsigned SrcReg, unsigned DstReg, unsigned SubIdx) { bool DstIsPhys = TargetRegisterInfo::isPhysicalRegister(DstReg); @@ -1920,20 +1860,6 @@ ShrinkMask = LaneBitmask::getNone(); ShrinkMainRange = false; - // Although we can update the DBG_VALUEs to the merged register, as debug uses - // do not contribute to liveness it might not be a sound update. Collect - // DBG_VALUEs that would change value were this interval merging to succeed. - SmallVector DbgValuesToChange; - auto CheckForDbgUser = [this, &CP, &DbgValuesToChange](MachineInstr &MI) { - if (MI.isDebugValue() && MI.getOperand(0).isReg() && - mergingChangesDbgValue(&MI, CP.getSrcReg(), CP.getDstReg())) - DbgValuesToChange.push_back(&MI); - }; - for (auto &RegIt : MRI->reg_instructions(CP.getSrcReg())) - CheckForDbgUser(RegIt); - for (auto &RegIt : MRI->reg_instructions(CP.getDstReg())) - CheckForDbgUser(RegIt); - // Okay, attempt to join these two intervals. On failure, this returns false. // Otherwise, if one of the intervals being joined is a physreg, this method // always canonicalizes DstInt to be it. The output "SrcInt" will not have @@ -2002,16 +1928,6 @@ updateRegDefsUses(CP.getDstReg(), CP.getDstReg(), CP.getDstIdx()); updateRegDefsUses(CP.getSrcReg(), CP.getDstReg(), CP.getSrcIdx()); - // The updates to these DBG_VALUEs are not sound -- mark them undef. - // FIXME: further analysis might recover them, this is the minimal sound - // solution. - for (MachineInstr *MI : DbgValuesToChange) { - assert(MI->getOperand(0).isReg()); - LLVM_DEBUG(dbgs() << "Update of " << MI->getOperand(0) << " would be " - << "unsound, setting undef\n"); - MI->getOperand(0).setReg(0); - } - // Shrink subregister ranges if necessary. if (ShrinkMask.any()) { LiveInterval &LI = LIS->getInterval(CP.getDstReg()); @@ -2058,6 +1974,47 @@ return true; } +// See comment for CheckDbgValuesInBlock; this is a specialised copy for +// when one coalesced register is a physical register. +static void CheckDbgValuesInBlockForPhysReg(const SlotIndexes &Slots, + MachineBasicBlock *MBB, + const LiveInterval &RegLiveness, + unsigned Reg) { + SlotIndex BlockStart = Slots.getMBBStartIdx(MBB); + SlotIndex BlockEnd = Slots.getMBBEndIdx(MBB); + + auto RegLivenessIt = RegLiveness.find(BlockStart); + + // If the Reg is live all the way through this block, no non-live DBG_VALUEs + // can be merged in. Early exit. + if (RegLivenessIt != RegLiveness.end() && + RegLivenessIt->start <= BlockStart && RegLivenessIt->end >= BlockEnd) + return; + + MachineInstr *StartMI = Slots.getInstructionFromIndex(BlockStart); + auto InstIt = (StartMI) ? StartMI->getIterator() : MBB->begin(); + MachineInstr *EndMI = Slots.getInstructionFromIndex(BlockEnd); + auto InstEndIt = (EndMI) ? EndMI->getIterator() : MBB->end(); + bool RegIsLive = RegLiveness.liveAt(BlockStart); + + for (; InstIt != InstEndIt; ++InstIt) { + MachineInstr &MI = *InstIt; + // Is this a DBG_VALUE for Reg, where Reg is dead? + if (MI.isDebugValue() && !RegIsLive && + MI.getOperand(0).isReg() && MI.getOperand(0).getReg() == Reg) { + // If so, point the DBG_VALUE to $noreg + MI.getOperand(0).setReg(0); + } else if (!MI.isDebugInstr()) { + // If not, update current liveness record. + SlotIndex Slot = Slots.getInstructionIndex(MI); + Slot = Slot.getRegSlot(); + RegIsLive = RegLiveness.liveAt(Slot); + } + } + + return; +} + bool RegisterCoalescer::joinReservedPhysReg(CoalescerPair &CP) { unsigned DstReg = CP.getDstReg(); unsigned SrcReg = CP.getSrcReg(); @@ -2103,6 +2060,19 @@ // register live range doesn't need to be accurate as long as all the // defs are there. + // Clear any non-live debug users of the vreg -- we can't guarantee the + // reserved register has the same value once they're merged. + SmallPtrSet SeenBlocks; + const SlotIndexes &Slots = *LIS->getSlotIndexes(); + for (auto &MI : MRI->use_instructions(SrcReg)) { + if (MI.isDebugValue()) + SeenBlocks.insert(MI.getParent()); + } + + for (auto *MBB : SeenBlocks) { + CheckDbgValuesInBlockForPhysReg(Slots, MBB, RHS, SrcReg); + } + // Delete the identity copy. MachineInstr *CopyMI; if (CP.isFlipped()) { @@ -3382,6 +3352,63 @@ return true; } +// Scan a basic block for unsound DBG_VALUE updates if two live ranges are +// coalesced. RegLiveness is the LiveInterval for Reg, while OtherLiveness is +// the other range it will be merged with. If, at a particular DBG_VALUE, +// Reg is not live, but OtherLiveness is, then merging the two would be +// unsound (PR40010). +// Walk through all instructions in the block, storing the liveness properties +// at a particular location. This is to work around the fact that slot-index +// queries for DBG_VALUEs is slow. +static void CheckDbgValuesInBlock(const SlotIndexes &Slots, + MachineBasicBlock *MBB, + const LiveInterval &RegLiveness, + const LiveInterval &OtherLiveness, + unsigned Reg) { + SlotIndex BlockStart = Slots.getMBBStartIdx(MBB); + SlotIndex BlockEnd = Slots.getMBBEndIdx(MBB); + + auto RegLivenessIt = RegLiveness.find(BlockStart); + auto OtherLivenessIt = OtherLiveness.find(BlockStart); + + // If the Other range is never live, it can never interfere with Reg in this + // block. Exit early. + if (OtherLivenessIt == OtherLiveness.end() || + OtherLivenessIt->start > BlockEnd) + return; + + // If the Reg is live all the way through this block, no non-live DBG_VALUEs + // can be merged with the live range of Other. Exit early. + if (RegLivenessIt != RegLiveness.end() && + RegLivenessIt->start <= BlockStart && RegLivenessIt->end >= BlockEnd) + return; + + MachineInstr *StartMI = Slots.getInstructionFromIndex(BlockStart); + auto InstIt = (StartMI) ? StartMI->getIterator() : MBB->begin(); + MachineInstr *EndMI = Slots.getInstructionFromIndex(BlockEnd); + auto InstEndIt = (EndMI) ? EndMI->getIterator() : MBB->end(); + bool RegIsLive = RegLiveness.liveAt(BlockStart); + bool OtherIsLive = OtherLiveness.liveAt(BlockStart); + + for (; InstIt != InstEndIt; ++InstIt) { + MachineInstr &MI = *InstIt; + // Is this a DBG_VALUE for Reg, where Reg is dead and Other is live? + if (MI.isDebugValue() && !RegIsLive && OtherIsLive && + MI.getOperand(0).isReg() && MI.getOperand(0).getReg() == Reg) { + // If so, point the DBG_VALUE to $noreg + MI.getOperand(0).setReg(0); + } else if (!MI.isDebugInstr()) { + // If not, update current liveness record. + SlotIndex Slot = Slots.getInstructionIndex(MI); + Slot = Slot.getRegSlot(); + RegIsLive = RegLiveness.liveAt(Slot); + OtherIsLive = OtherLiveness.liveAt(Slot); + } + } + + return; +} + bool RegisterCoalescer::joinVirtRegs(CoalescerPair &CP) { SmallVector NewVNInfo; LiveInterval &RHS = LIS->getInterval(CP.getSrcReg()); @@ -3406,6 +3433,40 @@ if (!LHSVals.resolveConflicts(RHSVals) || !RHSVals.resolveConflicts(LHSVals)) return false; + // Although we can update the DBG_VALUEs to the merged register, as debug uses + // do not contribute to liveness it might not be a sound update. The VReg + // referred to may have a different value/def at the DBG_VALUEs location. + // Scan for and mark these DBG_VALUEs as undef. + // + // However, because debug intrinsics don't get slots and don't appear in + // the slot index map, it can be extremely slow to query for each DBG_VALUEs + // slot to then perform a liveness query on it. Instead, identify blocks that + // definitely contain DBG_VALUEs, then scan from start to end. + const SlotIndexes &Slots = *LIS->getSlotIndexes(); + + // Helper delegate to collect blocks of interest, then call + // CheckDbgValuesInBlock on each one once. + auto DbgValueRegScanner = [&](const LiveInterval &RegLiveness, + const LiveInterval &OtherLiveness, + unsigned Reg) { + SmallPtrSet SeenBlocks; + for (auto &RegIt : MRI->reg_instructions(Reg)) { + if (RegIt.isDebugValue()) + SeenBlocks.insert(RegIt.getParent()); + } + + for (auto *MBB : SeenBlocks) + CheckDbgValuesInBlock(Slots, MBB, RegLiveness, OtherLiveness, Reg); + }; + + unsigned SrcReg = CP.getSrcReg(); + unsigned DstReg = CP.getDstReg(); + const LiveInterval &SrcLI = LIS->getInterval(SrcReg); + const LiveInterval &DstLI = LIS->getInterval(DstReg); + + DbgValueRegScanner(SrcLI, DstLI, SrcReg); + DbgValueRegScanner(DstLI, SrcLI, DstReg); + // All clear, the live ranges can be merged. if (RHS.hasSubRanges() || LHS.hasSubRanges()) { BumpPtrAllocator &Allocator = LIS->getVNInfoAllocator();