Skip to content

Commit

Permalink
[BOLT] Fix non-determinism in shrink wrapping
Browse files Browse the repository at this point in the history
Summary:
Iterating over SmallPtrSet is non-deterministic. Change it to
SmallSetVector. Similarly, do not sort a vector of ProgramPoint when
computing the dominance frontier, as ProgramPoint uses the pointer value
to determine order. Use a SmallSetVector there too to avoid duplicates
instead of sorting + uniqueing.

(cherry picked from FBD14992085)
  • Loading branch information
rafaelauler authored and maksfb committed Apr 18, 2019
1 parent 433f3e3 commit 3b422ea
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 11 deletions.
14 changes: 6 additions & 8 deletions bolt/src/Passes/DominatorAnalysis.h
Expand Up @@ -39,30 +39,28 @@ class DominatorAnalysis
: InstrsDataflowAnalysis<DominatorAnalysis<Backward>, Backward>(BC, BF) {}
virtual ~DominatorAnalysis() {}

SmallVector<ProgramPoint, 4> getDominanceFrontierFor(const MCInst &Dom) {
SmallVector<ProgramPoint, 4> Result;
SmallSetVector<ProgramPoint, 4> getDominanceFrontierFor(const MCInst &Dom) {
SmallSetVector<ProgramPoint, 4> Result;
auto DomIdx = this->ExprToIdx[&Dom];
assert(!Backward && "Post-dom frontier not implemented");
for (auto &BB : this->Func) {
bool HasDominatedPred = false;
bool HasNonDominatedPred = false;
SmallVector<ProgramPoint, 4> Candidates;
SmallSetVector<ProgramPoint, 4> Candidates;
this->doForAllSuccsOrPreds(BB, [&](ProgramPoint P) {
if ((*this->getStateAt(P))[DomIdx]) {
Candidates.emplace_back(P);
Candidates.insert(P);
HasDominatedPred = true;
return;
}
HasNonDominatedPred = true;
});
if (HasDominatedPred && HasNonDominatedPred)
Result.append(Candidates.begin(), Candidates.end());
Result.insert(Candidates.begin(), Candidates.end());
if ((*this->getStateAt(ProgramPoint::getLastPointAt(BB)))[DomIdx] &&
BB.succ_begin() == BB.succ_end())
Result.emplace_back(ProgramPoint::getLastPointAt(BB));
Result.insert(ProgramPoint::getLastPointAt(BB));
}
std::sort(Result.begin(), Result.end());
Result.erase(std::unique(Result.begin(), Result.end()), Result.end());
return Result;
}

Expand Down
4 changes: 2 additions & 2 deletions bolt/src/Passes/ShrinkWrapping.cpp
Expand Up @@ -771,7 +771,7 @@ void ShrinkWrapping::pruneUnwantedCSRs() {
}

void ShrinkWrapping::computeSaveLocations() {
SavePos = std::vector<SmallPtrSet<MCInst *, 4>>(BC.MRI->getNumRegs());
SavePos = std::vector<SmallSetVector<MCInst *, 4>>(BC.MRI->getNumRegs());
auto &RI = Info.getReachingInsnsBackwards();
auto &DA = Info.getDominatorAnalysis();
auto &SPT = Info.getStackPointerTracking();
Expand Down Expand Up @@ -960,7 +960,7 @@ ShrinkWrapping::doRestorePlacement(MCInst *BestPosSave, unsigned CSR,
// In case of a critical edge, we need to create extra BBs to host restores
// into edges transitioning to the dominance frontier, otherwise we pull these
// restores to inside the dominated area.
Frontier = DA.getDominanceFrontierFor(*BestPosSave);
Frontier = DA.getDominanceFrontierFor(*BestPosSave).takeVector();
DEBUG({
dbgs() << "Dumping dominance frontier for ";
BC.printInstruction(dbgs(), *BestPosSave);
Expand Down
2 changes: 1 addition & 1 deletion bolt/src/Passes/ShrinkWrapping.h
Expand Up @@ -306,7 +306,7 @@ class ShrinkWrapping {
std::vector<int64_t> PopOffsetByReg;
std::vector<MCPhysReg> DomOrder;
CalleeSavedAnalysis CSA;
std::vector<SmallPtrSet<MCInst *, 4>> SavePos;
std::vector<SmallSetVector<MCInst *, 4>> SavePos;
std::vector<uint64_t> BestSaveCount;
std::vector<MCInst *> BestSavePos;

Expand Down

0 comments on commit 3b422ea

Please sign in to comment.