Skip to content

Commit

Permalink
[NFC][Regalloc] Ensure Query::interferingVRegs is accurate.
Browse files Browse the repository at this point in the history
To correctly use Query, one had to first call collectInterferingVRegs to
pre-cache the query result, then call interferingVRegs. Failing the
former, interferingVRegs could be stale. This did cause a bug which was
addressed in D98232, but the underlying usability issue of the Query API
wasn't.

This patch addresses the latter by making collectInterferingVRegs an
implementation detail, and having interferingVRegs play both roles. One
side-effect of this is that interferingVRegs is not const anymore.

Differential Revision: https://reviews.llvm.org/D112882
  • Loading branch information
mtrofin committed Nov 3, 2021
1 parent 1b108ab commit 34f4fe3
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 37 deletions.
29 changes: 14 additions & 15 deletions llvm/include/llvm/CodeGen/LiveIntervalUnion.h
Expand Up @@ -114,12 +114,19 @@ class LiveIntervalUnion {
const LiveRange *LR = nullptr;
LiveRange::const_iterator LRI; ///< current position in LR
ConstSegmentIter LiveUnionI; ///< current position in LiveUnion
Optional<SmallVector<LiveInterval *, 4>> InterferingVRegs;
SmallVector<LiveInterval *, 4> InterferingVRegs;
bool CheckedFirstInterference = false;
bool SeenAllInterferences = false;
unsigned Tag = 0;
unsigned UserTag = 0;

// Count the virtual registers in this union that interfere with this
// query's live virtual register, up to maxInterferingRegs.
unsigned collectInterferingVRegs(unsigned MaxInterferingRegs);

// Was this virtual register visited during collectInterferingVRegs?
bool isSeenInterference(LiveInterval *VirtReg) const;

public:
Query() = default;
Query(const LiveRange &LR, const LiveIntervalUnion &LIU)
Expand All @@ -131,7 +138,7 @@ class LiveIntervalUnion {
const LiveIntervalUnion &NewLiveUnion) {
LiveUnion = &NewLiveUnion;
LR = &NewLR;
InterferingVRegs = None;
InterferingVRegs.clear();
CheckedFirstInterference = false;
SeenAllInterferences = false;
Tag = NewLiveUnion.getTag();
Expand All @@ -151,20 +158,12 @@ class LiveIntervalUnion {
// Does this live virtual register interfere with the union?
bool checkInterference() { return collectInterferingVRegs(1); }

// Count the virtual registers in this union that interfere with this
// query's live virtual register, up to maxInterferingRegs.
unsigned collectInterferingVRegs(
unsigned MaxInterferingRegs = std::numeric_limits<unsigned>::max());

// Was this virtual register visited during collectInterferingVRegs?
bool isSeenInterference(LiveInterval *VirtReg) const;

// Did collectInterferingVRegs collect all interferences?
bool seenAllInterferences() const { return SeenAllInterferences; }

// Vector generated by collectInterferingVRegs.
const SmallVectorImpl<LiveInterval*> &interferingVRegs() const {
return *InterferingVRegs;
const SmallVectorImpl<LiveInterval *> &interferingVRegs(
unsigned MaxInterferingRegs = std::numeric_limits<unsigned>::max()) {
if (!SeenAllInterferences || MaxInterferingRegs < InterferingVRegs.size())
collectInterferingVRegs(MaxInterferingRegs);
return InterferingVRegs;
}
};

Expand Down
23 changes: 10 additions & 13 deletions llvm/lib/CodeGen/LiveIntervalUnion.cpp
Expand Up @@ -112,7 +112,7 @@ LiveInterval *LiveIntervalUnion::getOneVReg() const {
// Scan the vector of interfering virtual registers in this union. Assume it's
// quite small.
bool LiveIntervalUnion::Query::isSeenInterference(LiveInterval *VirtReg) const {
return is_contained(*InterferingVRegs, VirtReg);
return is_contained(InterferingVRegs, VirtReg);
}

// Collect virtual registers in this union that interfere with this
Expand All @@ -124,14 +124,11 @@ bool LiveIntervalUnion::Query::isSeenInterference(LiveInterval *VirtReg) const {
// 2. SeenAllInterferences == true: InterferingVRegs complete, iterators unused.
// 3. Iterators left at the last seen intersection.
//
unsigned LiveIntervalUnion::Query::
collectInterferingVRegs(unsigned MaxInterferingRegs) {
if (!InterferingVRegs)
InterferingVRegs.emplace();

unsigned
LiveIntervalUnion::Query::collectInterferingVRegs(unsigned MaxInterferingRegs) {
// Fast path return if we already have the desired information.
if (SeenAllInterferences || InterferingVRegs->size() >= MaxInterferingRegs)
return InterferingVRegs->size();
if (SeenAllInterferences || InterferingVRegs.size() >= MaxInterferingRegs)
return InterferingVRegs.size();

// Set up iterators on the first call.
if (!CheckedFirstInterference) {
Expand Down Expand Up @@ -160,14 +157,14 @@ collectInterferingVRegs(unsigned MaxInterferingRegs) {
LiveInterval *VReg = LiveUnionI.value();
if (VReg != RecentReg && !isSeenInterference(VReg)) {
RecentReg = VReg;
InterferingVRegs->push_back(VReg);
if (InterferingVRegs->size() >= MaxInterferingRegs)
return InterferingVRegs->size();
InterferingVRegs.push_back(VReg);
if (InterferingVRegs.size() >= MaxInterferingRegs)
return InterferingVRegs.size();
}
// This LiveUnion segment is no longer interesting.
if (!(++LiveUnionI).valid()) {
SeenAllInterferences = true;
return InterferingVRegs->size();
return InterferingVRegs.size();
}
}

Expand All @@ -188,7 +185,7 @@ collectInterferingVRegs(unsigned MaxInterferingRegs) {
LiveUnionI.advanceTo(LRI->start);
}
SeenAllInterferences = true;
return InterferingVRegs->size();
return InterferingVRegs.size();
}

void LiveIntervalUnion::Array::init(LiveIntervalUnion::Allocator &Alloc,
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/CodeGen/RegAllocBasic.cpp
Expand Up @@ -217,9 +217,7 @@ bool RABasic::spillInterferences(LiveInterval &VirtReg, MCRegister PhysReg,
// Collect interferences assigned to any alias of the physical register.
for (MCRegUnitIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
LiveIntervalUnion::Query &Q = Matrix->query(VirtReg, *Units);
Q.collectInterferingVRegs();
for (unsigned i = Q.interferingVRegs().size(); i; --i) {
LiveInterval *Intf = Q.interferingVRegs()[i - 1];
for (auto *Intf : reverse(Q.interferingVRegs())) {
if (!Intf->isSpillable() || Intf->weight() > VirtReg.weight())
return false;
Intfs.push_back(Intf);
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/CodeGen/RegAllocGreedy.cpp
Expand Up @@ -955,11 +955,12 @@ bool RAGreedy::canEvictInterference(
for (MCRegUnitIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
LiveIntervalUnion::Query &Q = Matrix->query(VirtReg, *Units);
// If there is 10 or more interferences, chances are one is heavier.
if (Q.collectInterferingVRegs(10) >= 10)
const auto &Interferences = Q.interferingVRegs(10);
if (Interferences.size() >= 10)
return false;

// Check if any interfering live range is heavier than MaxWeight.
for (LiveInterval *Intf : reverse(Q.interferingVRegs())) {
for (LiveInterval *Intf : reverse(Interferences)) {
assert(Register::isVirtualRegister(Intf->reg()) &&
"Only expecting virtual register interference from query");

Expand Down Expand Up @@ -1037,7 +1038,6 @@ bool RAGreedy::canEvictInterferenceInRange(const LiveInterval &VirtReg,

for (MCRegUnitIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
LiveIntervalUnion::Query &Q = Matrix->query(VirtReg, *Units);
Q.collectInterferingVRegs();

// Check if any interfering live range is heavier than MaxWeight.
for (const LiveInterval *Intf : reverse(Q.interferingVRegs())) {
Expand Down Expand Up @@ -1127,7 +1127,6 @@ void RAGreedy::evictInterference(LiveInterval &VirtReg, MCRegister PhysReg,
// should be fast, we may need to recalculate if when different physregs
// overlap the same register unit so we had different SubRanges queried
// against it.
Q.collectInterferingVRegs();
ArrayRef<LiveInterval*> IVR = Q.interferingVRegs();
Intfs.append(IVR.begin(), IVR.end());
}
Expand Down Expand Up @@ -2547,8 +2546,9 @@ bool RAGreedy::mayRecolorAllInterferences(
LiveIntervalUnion::Query &Q = Matrix->query(VirtReg, *Units);
// If there is LastChanceRecoloringMaxInterference or more interferences,
// chances are one would not be recolorable.
if (Q.collectInterferingVRegs(LastChanceRecoloringMaxInterference) >=
LastChanceRecoloringMaxInterference && !ExhaustiveSearch) {
if (Q.interferingVRegs(LastChanceRecoloringMaxInterference).size() >=
LastChanceRecoloringMaxInterference &&
!ExhaustiveSearch) {
LLVM_DEBUG(dbgs() << "Early abort: too many interferences.\n");
CutOffInfo |= CO_Interf;
return false;
Expand Down

0 comments on commit 34f4fe3

Please sign in to comment.