Skip to content

Commit dfeb797

Browse files
committed
[PGO][CHR] Speed up following long use-def chains.
Summary: Avoid visiting an instruction more than once by using a map. Reviewers: davidxl Reviewed By: davidxl Subscribers: llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D62262 llvm-svn: 361416
1 parent adea0b6 commit dfeb797

File tree

2 files changed

+336
-9
lines changed

2 files changed

+336
-9
lines changed

llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -546,19 +546,25 @@ static std::set<Value *> getBaseValues(Value *V,
546546
static bool
547547
checkHoistValue(Value *V, Instruction *InsertPoint, DominatorTree &DT,
548548
DenseSet<Instruction *> &Unhoistables,
549-
DenseSet<Instruction *> *HoistStops) {
549+
DenseSet<Instruction *> *HoistStops,
550+
DenseMap<Instruction *, bool> &Visited) {
550551
assert(InsertPoint && "Null InsertPoint");
551552
if (auto *I = dyn_cast<Instruction>(V)) {
553+
if (Visited.count(I)) {
554+
return Visited[I];
555+
}
552556
assert(DT.getNode(I->getParent()) && "DT must contain I's parent block");
553557
assert(DT.getNode(InsertPoint->getParent()) && "DT must contain Destination");
554558
if (Unhoistables.count(I)) {
555559
// Don't hoist if they are not to be hoisted.
560+
Visited[I] = false;
556561
return false;
557562
}
558563
if (DT.dominates(I, InsertPoint)) {
559564
// We are already above the insert point. Stop here.
560565
if (HoistStops)
561566
HoistStops->insert(I);
567+
Visited[I] = true;
562568
return true;
563569
}
564570
// We aren't not above the insert point, check if we can hoist it above the
@@ -568,7 +574,8 @@ checkHoistValue(Value *V, Instruction *InsertPoint, DominatorTree &DT,
568574
DenseSet<Instruction *> OpsHoistStops;
569575
bool AllOpsHoisted = true;
570576
for (Value *Op : I->operands()) {
571-
if (!checkHoistValue(Op, InsertPoint, DT, Unhoistables, &OpsHoistStops)) {
577+
if (!checkHoistValue(Op, InsertPoint, DT, Unhoistables, &OpsHoistStops,
578+
Visited)) {
572579
AllOpsHoisted = false;
573580
break;
574581
}
@@ -577,9 +584,11 @@ checkHoistValue(Value *V, Instruction *InsertPoint, DominatorTree &DT,
577584
CHR_DEBUG(dbgs() << "checkHoistValue " << *I << "\n");
578585
if (HoistStops)
579586
HoistStops->insert(OpsHoistStops.begin(), OpsHoistStops.end());
587+
Visited[I] = true;
580588
return true;
581589
}
582590
}
591+
Visited[I] = false;
583592
return false;
584593
}
585594
// Non-instructions are considered hoistable.
@@ -892,8 +901,9 @@ void CHR::checkScopeHoistable(CHRScope *Scope) {
892901
++it;
893902
continue;
894903
}
904+
DenseMap<Instruction *, bool> Visited;
895905
bool IsHoistable = checkHoistValue(SI->getCondition(), InsertPoint,
896-
DT, Unhoistables, nullptr);
906+
DT, Unhoistables, nullptr, Visited);
897907
if (!IsHoistable) {
898908
CHR_DEBUG(dbgs() << "Dropping select " << *SI << "\n");
899909
ORE.emit([&]() {
@@ -912,8 +922,9 @@ void CHR::checkScopeHoistable(CHRScope *Scope) {
912922
InsertPoint = getBranchInsertPoint(RI);
913923
CHR_DEBUG(dbgs() << "InsertPoint " << *InsertPoint << "\n");
914924
if (RI.HasBranch && InsertPoint != Branch) {
925+
DenseMap<Instruction *, bool> Visited;
915926
bool IsHoistable = checkHoistValue(Branch->getCondition(), InsertPoint,
916-
DT, Unhoistables, nullptr);
927+
DT, Unhoistables, nullptr, Visited);
917928
if (!IsHoistable) {
918929
// If the branch isn't hoistable, drop the selects in the entry
919930
// block, preferring the branch, which makes the branch the hoist
@@ -944,15 +955,17 @@ void CHR::checkScopeHoistable(CHRScope *Scope) {
944955
if (RI.HasBranch) {
945956
assert(!DT.dominates(Branch, InsertPoint) &&
946957
"Branch can't be already above the hoist point");
958+
DenseMap<Instruction *, bool> Visited;
947959
assert(checkHoistValue(Branch->getCondition(), InsertPoint,
948-
DT, Unhoistables, nullptr) &&
960+
DT, Unhoistables, nullptr, Visited) &&
949961
"checkHoistValue for branch");
950962
}
951963
for (auto *SI : Selects) {
952964
assert(!DT.dominates(SI, InsertPoint) &&
953965
"SI can't be already above the hoist point");
966+
DenseMap<Instruction *, bool> Visited;
954967
assert(checkHoistValue(SI->getCondition(), InsertPoint, DT,
955-
Unhoistables, nullptr) &&
968+
Unhoistables, nullptr, Visited) &&
956969
"checkHoistValue for selects");
957970
}
958971
CHR_DEBUG(dbgs() << "Result\n");
@@ -1053,7 +1066,8 @@ static bool shouldSplit(Instruction *InsertPoint,
10531066
assert(InsertPoint && "Null InsertPoint");
10541067
// If any of Bases isn't hoistable to the hoist point, split.
10551068
for (Value *V : ConditionValues) {
1056-
if (!checkHoistValue(V, InsertPoint, DT, Unhoistables, nullptr)) {
1069+
DenseMap<Instruction *, bool> Visited;
1070+
if (!checkHoistValue(V, InsertPoint, DT, Unhoistables, nullptr, Visited)) {
10571071
CHR_DEBUG(dbgs() << "Split. checkHoistValue false " << *V << "\n");
10581072
return true; // Not hoistable, split.
10591073
}
@@ -1382,8 +1396,9 @@ void CHR::setCHRRegions(CHRScope *Scope, CHRScope *OutermostScope) {
13821396
"Must be truthy or falsy");
13831397
auto *BI = cast<BranchInst>(R->getEntry()->getTerminator());
13841398
// Note checkHoistValue fills in HoistStops.
1399+
DenseMap<Instruction *, bool> Visited;
13851400
bool IsHoistable = checkHoistValue(BI->getCondition(), InsertPoint, DT,
1386-
Unhoistables, &HoistStops);
1401+
Unhoistables, &HoistStops, Visited);
13871402
assert(IsHoistable && "Must be hoistable");
13881403
(void)(IsHoistable); // Unused in release build
13891404
IsHoisted = true;
@@ -1393,8 +1408,9 @@ void CHR::setCHRRegions(CHRScope *Scope, CHRScope *OutermostScope) {
13931408
OutermostScope->FalseBiasedSelects.count(SI) > 0) &&
13941409
"Must be true or false biased");
13951410
// Note checkHoistValue fills in HoistStops.
1411+
DenseMap<Instruction *, bool> Visited;
13961412
bool IsHoistable = checkHoistValue(SI->getCondition(), InsertPoint, DT,
1397-
Unhoistables, &HoistStops);
1413+
Unhoistables, &HoistStops, Visited);
13981414
assert(IsHoistable && "Must be hoistable");
13991415
(void)(IsHoistable); // Unused in release build
14001416
IsHoisted = true;

0 commit comments

Comments
 (0)