From b98807df05cb4cd728e744c3349a9dfd341f6f61 Mon Sep 17 00:00:00 2001 From: Hongtao Yu Date: Mon, 12 Apr 2021 23:51:44 -0700 Subject: [PATCH] [CSSPGO] Exclude pseudo probes from slot index Pseudo probe are currently given a slot index like other regular instructions. This affects register pressure and lifetime weight computation because of enlarged lifetime length with pseudo probe instructions. As a consequence, program could get different code generated w/ and w/o pseudo probes. I'm closing the gap by excluding pseudo probes from stack index and downstream register allocation related passes. Reviewed By: wmi Differential Revision: https://reviews.llvm.org/D100334 --- llvm/lib/CodeGen/LexicalScopes.cpp | 10 +++++----- llvm/lib/CodeGen/LiveDebugVariables.cpp | 4 ++-- llvm/lib/CodeGen/LiveIntervals.cpp | 6 +++--- llvm/lib/CodeGen/LiveRangeShrink.cpp | 4 ++-- llvm/lib/CodeGen/LiveVariables.cpp | 4 ++-- llvm/lib/CodeGen/MachineBasicBlock.cpp | 6 +++--- llvm/lib/CodeGen/MachineScheduler.cpp | 6 +++--- llvm/lib/CodeGen/MachineSink.cpp | 6 +++--- llvm/lib/CodeGen/RegAllocGreedy.cpp | 5 +++-- llvm/lib/CodeGen/RegisterCoalescer.cpp | 4 ++-- llvm/lib/CodeGen/RegisterPressure.cpp | 14 +++++++------- llvm/lib/CodeGen/RegisterScavenging.cpp | 4 ++-- llvm/lib/CodeGen/ScheduleDAGInstrs.cpp | 11 +++++++---- llvm/lib/CodeGen/SlotIndexes.cpp | 4 ++-- llvm/lib/CodeGen/SplitKit.cpp | 2 +- .../SampleProfile/pseudo-probe-slotindex.ll | 22 ++++++++++++++++++++++ 16 files changed, 69 insertions(+), 43 deletions(-) create mode 100644 llvm/test/Transforms/SampleProfile/pseudo-probe-slotindex.ll diff --git a/llvm/lib/CodeGen/LexicalScopes.cpp b/llvm/lib/CodeGen/LexicalScopes.cpp index 8139c2c..47c19c3 100644 --- a/llvm/lib/CodeGen/LexicalScopes.cpp +++ b/llvm/lib/CodeGen/LexicalScopes.cpp @@ -75,6 +75,11 @@ void LexicalScopes::extractLexicalScopes( const MachineInstr *PrevMI = nullptr; const DILocation *PrevDL = nullptr; for (const auto &MInsn : MBB) { + // Ignore DBG_VALUE and similar instruction that do not contribute to any + // instruction in the output. + if (MInsn.isMetaInstruction()) + continue; + // Check if instruction has valid location information. const DILocation *MIDL = MInsn.getDebugLoc(); if (!MIDL) { @@ -88,11 +93,6 @@ void LexicalScopes::extractLexicalScopes( continue; } - // Ignore DBG_VALUE and similar instruction that do not contribute to any - // instruction in the output. - if (MInsn.isMetaInstruction()) - continue; - if (RangeBeginMI) { // If we have already seen a beginning of an instruction range and // current instruction scope does not match scope of first instruction diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp index 751e4f6..ce898cd 100644 --- a/llvm/lib/CodeGen/LiveDebugVariables.cpp +++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp @@ -870,7 +870,7 @@ bool LDVImpl::collectDebugValues(MachineFunction &mf) { MBBI != MBBE;) { // Use the first debug instruction in the sequence to get a SlotIndex // for following consecutive debug instructions. - if (!MBBI->isDebugInstr()) { + if (!MBBI->isDebugOrPseudoInstr()) { ++MBBI; continue; } @@ -891,7 +891,7 @@ bool LDVImpl::collectDebugValues(MachineFunction &mf) { Changed = true; } else ++MBBI; - } while (MBBI != MBBE && MBBI->isDebugInstr()); + } while (MBBI != MBBE && MBBI->isDebugOrPseudoInstr()); } } return Changed; diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp index 7d0e0d9..c038ba94 100644 --- a/llvm/lib/CodeGen/LiveIntervals.cpp +++ b/llvm/lib/CodeGen/LiveIntervals.cpp @@ -1484,7 +1484,7 @@ private: MachineBasicBlock::iterator Begin = MBB->begin(); while (MII != Begin) { - if ((--MII)->isDebugInstr()) + if ((--MII)->isDebugOrPseudoInstr()) continue; SlotIndex Idx = Indexes->getInstructionIndex(*MII); @@ -1579,7 +1579,7 @@ void LiveIntervals::repairOldRegInRange(const MachineBasicBlock::iterator Begin, for (MachineBasicBlock::iterator I = End; I != Begin;) { --I; MachineInstr &MI = *I; - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) continue; SlotIndex instrIdx = getInstructionIndex(MI); @@ -1676,7 +1676,7 @@ LiveIntervals::repairIntervalsInRange(MachineBasicBlock *MBB, for (MachineBasicBlock::iterator I = End; I != Begin;) { --I; MachineInstr &MI = *I; - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) continue; for (MachineInstr::const_mop_iterator MOI = MI.operands_begin(), MOE = MI.operands_end(); diff --git a/llvm/lib/CodeGen/LiveRangeShrink.cpp b/llvm/lib/CodeGen/LiveRangeShrink.cpp index 6c03a87..054f437 100644 --- a/llvm/lib/CodeGen/LiveRangeShrink.cpp +++ b/llvm/lib/CodeGen/LiveRangeShrink.cpp @@ -130,7 +130,7 @@ bool LiveRangeShrink::runOnMachineFunction(MachineFunction &MF) { for (MachineBasicBlock::iterator Next = MBB.begin(); Next != MBB.end();) { MachineInstr &MI = *Next; ++Next; - if (MI.isPHI() || MI.isDebugInstr()) + if (MI.isPHI() || MI.isDebugOrPseudoInstr()) continue; if (MI.mayStore()) SawStore = true; @@ -219,7 +219,7 @@ bool LiveRangeShrink::runOnMachineFunction(MachineFunction &MF) { if (DefMO && Insert && NumEligibleUse > 1 && Barrier <= IOM[Insert]) { MachineBasicBlock::iterator I = std::next(Insert->getIterator()); // Skip all the PHI and debug instructions. - while (I != MBB.end() && (I->isPHI() || I->isDebugInstr())) + while (I != MBB.end() && (I->isPHI() || I->isDebugOrPseudoInstr())) I = std::next(I); if (I == MI.getIterator()) continue; diff --git a/llvm/lib/CodeGen/LiveVariables.cpp b/llvm/lib/CodeGen/LiveVariables.cpp index e0ef38d..7181dbc 100644 --- a/llvm/lib/CodeGen/LiveVariables.cpp +++ b/llvm/lib/CodeGen/LiveVariables.cpp @@ -497,7 +497,7 @@ void LiveVariables::UpdatePhysRegDefs(MachineInstr &MI, void LiveVariables::runOnInstr(MachineInstr &MI, SmallVectorImpl &Defs) { - assert(!MI.isDebugInstr()); + assert(!MI.isDebugOrPseudoInstr()); // Process all of the operands of the instruction... unsigned NumOperandsToProcess = MI.getNumOperands(); @@ -572,7 +572,7 @@ void LiveVariables::runOnBlock(MachineBasicBlock *MBB, const unsigned NumRegs) { DistanceMap.clear(); unsigned Dist = 0; for (MachineInstr &MI : *MBB) { - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) continue; DistanceMap.insert(std::make_pair(&MI, Dist++)); diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp index cf22230..d792ad4 100644 --- a/llvm/lib/CodeGen/MachineBasicBlock.cpp +++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp @@ -1515,7 +1515,7 @@ MachineBasicBlock::computeRegisterLiveness(const TargetRegisterInfo *TRI, // Try searching forwards from Before, looking for reads or defs. const_iterator I(Before); for (; I != end() && N > 0; ++I) { - if (I->isDebugInstr()) + if (I->isDebugOrPseudoInstr()) continue; --N; @@ -1553,7 +1553,7 @@ MachineBasicBlock::computeRegisterLiveness(const TargetRegisterInfo *TRI, do { --I; - if (I->isDebugInstr()) + if (I->isDebugOrPseudoInstr()) continue; --N; @@ -1587,7 +1587,7 @@ MachineBasicBlock::computeRegisterLiveness(const TargetRegisterInfo *TRI, // If all the instructions before this in the block are debug instructions, // skip over them. - while (I != begin() && std::prev(I)->isDebugInstr()) + while (I != begin() && std::prev(I)->isDebugOrPseudoInstr()) --I; // Did we get to the start of the block? diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp index 35facee..dd6e3a2 100644 --- a/llvm/lib/CodeGen/MachineScheduler.cpp +++ b/llvm/lib/CodeGen/MachineScheduler.cpp @@ -297,7 +297,7 @@ priorNonDebug(MachineBasicBlock::const_iterator I, MachineBasicBlock::const_iterator Beg) { assert(I != Beg && "reached the top of the region, cannot decrement"); while (--I != Beg) { - if (!I->isDebugInstr()) + if (!I->isDebugOrPseudoInstr()) break; } return I; @@ -317,7 +317,7 @@ static MachineBasicBlock::const_iterator nextIfDebug(MachineBasicBlock::const_iterator I, MachineBasicBlock::const_iterator End) { for(; I != End; ++I) { - if (!I->isDebugInstr()) + if (!I->isDebugOrPseudoInstr()) break; } return I; @@ -508,7 +508,7 @@ getSchedRegions(MachineBasicBlock *MBB, MachineInstr &MI = *std::prev(I); if (isSchedBoundary(&MI, &*MBB, MF, TII)) break; - if (!MI.isDebugInstr()) { + if (!MI.isDebugOrPseudoInstr()) { // MBB::size() uses instr_iterator to count. Here we need a bundle to // count as a single instruction. ++NumRegionInstrs; diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp index 6be9f4e..41fa3a2 100644 --- a/llvm/lib/CodeGen/MachineSink.cpp +++ b/llvm/lib/CodeGen/MachineSink.cpp @@ -530,7 +530,7 @@ bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) { if (!ProcessedBegin) --I; - if (MI.isDebugInstr()) { + if (MI.isDebugOrPseudoInstr()) { if (MI.isDebugValue()) ProcessDbgInst(MI); continue; @@ -718,7 +718,7 @@ MachineSinking::getBBRegisterPressure(MachineBasicBlock &MBB) { MIE = MBB.instr_begin(); MII != MIE; --MII) { MachineInstr &MI = *std::prev(MII); - if (MI.isDebugValue() || MI.isDebugLabel()) + if (MI.isDebugValue() || MI.isDebugLabel() || MI.isPseudoProbe()) continue; RegisterOperands RegOpers; RegOpers.collect(MI, *TRI, *MRI, false, false); @@ -1755,7 +1755,7 @@ bool PostRAMachineSinking::tryToSinkCopy(MachineBasicBlock &CurBB, continue; } - if (MI->isDebugInstr()) + if (MI->isDebugOrPseudoInstr()) continue; // Do not move any instruction across function call. diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp index d377ca6..c1da51e 100644 --- a/llvm/lib/CodeGen/RegAllocGreedy.cpp +++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp @@ -1315,8 +1315,9 @@ bool RAGreedy::addThroughConstraints(InterferenceCache::Cursor Intf, // Abort if the spill cannot be inserted at the MBB' start MachineBasicBlock *MBB = MF->getBlockNumbered(Number); if (!MBB->empty() && - SlotIndex::isEarlierInstr(LIS->getInstructionIndex(MBB->instr_front()), - SA->getFirstSplitPoint(Number))) + SlotIndex::isEarlierInstr( + LIS->getInstructionIndex(*MBB->getFirstNonDebugInstr()), + SA->getFirstSplitPoint(Number))) return false; // Interference for the live-in value. if (Intf.first() <= Indexes->getMBBStartIdx(Number)) diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp index 4a20648..f535688 100644 --- a/llvm/lib/CodeGen/RegisterCoalescer.cpp +++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp @@ -2962,7 +2962,7 @@ taintExtent(unsigned ValNo, LaneBitmask TaintedLanes, JoinVals &Other, bool JoinVals::usesLanes(const MachineInstr &MI, Register Reg, unsigned SubIdx, LaneBitmask Lanes) const { - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) return false; for (const MachineOperand &MO : MI.operands()) { if (!MO.isReg() || MO.isDef() || MO.getReg() != Reg) @@ -3607,7 +3607,7 @@ void RegisterCoalescer::buildVRegToDbgValueMap(MachineFunction &MF) return MO.isReg() && MO.getReg().isVirtual(); })) ToInsert.push_back(&MI); - } else if (!MI.isDebugInstr()) { + } else if (!MI.isDebugOrPseudoInstr()) { CurrentSlot = Slots.getInstructionIndex(MI); CloseNewDVRange(CurrentSlot); } diff --git a/llvm/lib/CodeGen/RegisterPressure.cpp b/llvm/lib/CodeGen/RegisterPressure.cpp index 8f1fc10..fae1ecf 100644 --- a/llvm/lib/CodeGen/RegisterPressure.cpp +++ b/llvm/lib/CodeGen/RegisterPressure.cpp @@ -764,7 +764,7 @@ void RegPressureTracker::bumpDeadDefs(ArrayRef DeadDefs) { /// instruction independent of liveness. void RegPressureTracker::recede(const RegisterOperands &RegOpers, SmallVectorImpl *LiveUses) { - assert(!CurrPos->isDebugInstr()); + assert(!CurrPos->isDebugOrPseudoInstr()); // Boost pressure for all dead defs together. bumpDeadDefs(RegOpers.DeadDefs); @@ -863,7 +863,7 @@ void RegPressureTracker::recedeSkipDebugValues() { CurrPos = prev_nodbg(CurrPos, MBB->begin()); SlotIndex SlotIdx; - if (RequireIntervals && !CurrPos->isDebugInstr()) + if (RequireIntervals && !CurrPos->isDebugOrPseudoInstr()) SlotIdx = LIS->getInstructionIndex(*CurrPos).getRegSlot(); // Open the top of the region using slot indexes. @@ -873,9 +873,9 @@ void RegPressureTracker::recedeSkipDebugValues() { void RegPressureTracker::recede(SmallVectorImpl *LiveUses) { recedeSkipDebugValues(); - if (CurrPos->isDebugValue()) { - // It's possible to only have debug_value instructions and hit the start of - // the block. + if (CurrPos->isDebugValue() || CurrPos->isPseudoProbe()) { + // It's possible to only have debug_value and pseudo probe instructions and + // hit the start of the block. assert(CurrPos == MBB->begin()); return; } @@ -1041,7 +1041,7 @@ static void computeMaxPressureDelta(ArrayRef OldMaxPressureVec, /// This is intended for speculative queries. It leaves pressure inconsistent /// with the current position, so must be restored by the caller. void RegPressureTracker::bumpUpwardPressure(const MachineInstr *MI) { - assert(!MI->isDebugInstr() && "Expect a nondebug instruction."); + assert(!MI->isDebugOrPseudoInstr() && "Expect a nondebug instruction."); SlotIndex SlotIdx; if (RequireIntervals) @@ -1282,7 +1282,7 @@ LaneBitmask RegPressureTracker::getLiveThroughAt(Register RegUnit, /// This is intended for speculative queries. It leaves pressure inconsistent /// with the current position, so must be restored by the caller. void RegPressureTracker::bumpDownwardPressure(const MachineInstr *MI) { - assert(!MI->isDebugInstr() && "Expect a nondebug instruction."); + assert(!MI->isDebugOrPseudoInstr() && "Expect a nondebug instruction."); SlotIndex SlotIdx; if (RequireIntervals) diff --git a/llvm/lib/CodeGen/RegisterScavenging.cpp b/llvm/lib/CodeGen/RegisterScavenging.cpp index 2ec4915..e35cf7aa 100644 --- a/llvm/lib/CodeGen/RegisterScavenging.cpp +++ b/llvm/lib/CodeGen/RegisterScavenging.cpp @@ -175,7 +175,7 @@ void RegScavenger::forward() { I.Restore = nullptr; } - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) return; determineKillsAndDefs(); @@ -298,7 +298,7 @@ Register RegScavenger::findSurvivorReg(MachineBasicBlock::iterator StartMI, bool inVirtLiveRange = false; for (++MI; InstrLimit > 0 && MI != ME; ++MI, --InstrLimit) { - if (MI->isDebugInstr()) { + if (MI->isDebugOrPseudoInstr()) { ++InstrLimit; // Don't count debug instructions continue; } diff --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp index 5899da7..62948ff 100644 --- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -514,7 +514,7 @@ void ScheduleDAGInstrs::addVRegDefDeps(SUnit *SU, unsigned OperIdx) { /// TODO: Handle ExitSU "uses" properly. void ScheduleDAGInstrs::addVRegUseDeps(SUnit *SU, unsigned OperIdx) { const MachineInstr *MI = SU->getInstr(); - assert(!MI->isDebugInstr()); + assert(!MI->isDebugOrPseudoInstr()); const MachineOperand &MO = MI->getOperand(OperIdx); Register Reg = MO.getReg(); @@ -572,7 +572,7 @@ void ScheduleDAGInstrs::initSUnits() { SUnits.reserve(NumRegionInstrs); for (MachineInstr &MI : make_range(RegionBegin, RegionEnd)) { - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) continue; SUnit *SU = newSUnit(&MI); @@ -814,6 +814,9 @@ void ScheduleDAGInstrs::buildSchedGraph(AAResults *AA, if (MI.isDebugLabel()) continue; + if (MI.isPseudoProbe()) + continue; + SUnit *SU = MISUnitMap[&MI]; assert(SU && "No SUnit mapped to this MI"); @@ -1117,7 +1120,7 @@ void ScheduleDAGInstrs::fixupKills(MachineBasicBlock &MBB) { // Examine block from end to start... for (MachineInstr &MI : make_range(MBB.rbegin(), MBB.rend())) { - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) continue; // Update liveness. Registers that are defed but not used in this @@ -1152,7 +1155,7 @@ void ScheduleDAGInstrs::fixupKills(MachineBasicBlock &MBB) { while (I->isBundledWithSucc()) ++I; do { - if (!I->isDebugInstr()) + if (!I->isDebugOrPseudoInstr()) toggleKills(MRI, LiveRegs, *I, true); --I; } while (I != Bundle); diff --git a/llvm/lib/CodeGen/SlotIndexes.cpp b/llvm/lib/CodeGen/SlotIndexes.cpp index c72bcf2..c933031e 100644 --- a/llvm/lib/CodeGen/SlotIndexes.cpp +++ b/llvm/lib/CodeGen/SlotIndexes.cpp @@ -83,7 +83,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) { SlotIndex blockStartIndex(&indexList.back(), SlotIndex::Slot_Block); for (MachineInstr &MI : MBB) { - if (MI.isDebugInstr()) + if (MI.isDebugOrPseudoInstr()) continue; // Insert a store index for the instr. @@ -241,7 +241,7 @@ void SlotIndexes::repairIndexesInRange(MachineBasicBlock *MBB, for (MachineBasicBlock::iterator I = End; I != Begin;) { --I; MachineInstr &MI = *I; - if (!MI.isDebugInstr() && mi2iMap.find(&MI) == mi2iMap.end()) + if (!MI.isDebugOrPseudoInstr() && mi2iMap.find(&MI) == mi2iMap.end()) insertMachineInstrInMaps(MI); } } diff --git a/llvm/lib/CodeGen/SplitKit.cpp b/llvm/lib/CodeGen/SplitKit.cpp index d778a9e..597f516 100644 --- a/llvm/lib/CodeGen/SplitKit.cpp +++ b/llvm/lib/CodeGen/SplitKit.cpp @@ -819,7 +819,7 @@ void SplitEditor::removeBackCopies(SmallVectorImpl &Copies) { MachineBasicBlock::iterator MBBI(MI); bool AtBegin; do AtBegin = MBBI == MBB->begin(); - while (!AtBegin && (--MBBI)->isDebugInstr()); + while (!AtBegin && (--MBBI)->isDebugOrPseudoInstr()); LLVM_DEBUG(dbgs() << "Removing " << Def << '\t' << *MI); LIS.removeVRegDefAt(*LI, Def); diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-slotindex.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-slotindex.ll new file mode 100644 index 0000000..4165318 --- /dev/null +++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-slotindex.ll @@ -0,0 +1,22 @@ +; REQUIRES: x86_64-linux +; RUN: llc -print-after=slotindexes -stop-after=slotindexes -mtriple=x86_64-- %s -filetype=asm -o %t 2>&1 | FileCheck %s + +define void @foo(i32* %p) { + store i32 0, i32* %p + call void @llvm.pseudoprobe(i64 5116412291814990879, i64 1, i32 0, i64 -1) + store i32 0, i32* %p + ret void +} + +;; Check the pseudo probe instruction isn't assigned a slot index. +;CHECK: IR Dump {{.*}} +;CHECK: # Machine code for function foo{{.*}} +;CHECK: {{[0-9]+}}B bb.0 (%ir-block.0) +;CHECK: {{[0-9]+}}B %0:gr64 = COPY killed $rdi +;CHECK: {{^}} PSEUDO_PROBE 5116412291814990879 +;CHECK: {{[0-9]+}}B MOV32mi +;CHECK: {{[0-9]+}}B RET 0 + +declare void @llvm.pseudoprobe(i64, i64, i32, i64) #0 + +attributes #0 = { inaccessiblememonly nounwind willreturn } \ No newline at end of file -- 2.7.4