Skip to content

Commit 3340b24

Browse files
authored
[DFAJumpThreading] Precompute value => successor mapping (llvm#162824)
Address some previous comments. Note that the value => successor mapping is invalid during DFA transformation unless we update it correctly.
1 parent 424fa83 commit 3340b24

File tree

1 file changed

+39
-23
lines changed

1 file changed

+39
-23
lines changed

llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -386,22 +386,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, const PathType &Path) {
386386
return OS;
387387
}
388388

389-
/// Helper to get the successor corresponding to a particular case value for
390-
/// a switch statement.
391-
static BasicBlock *getNextCaseSuccessor(SwitchInst *Switch,
392-
const APInt &NextState) {
393-
BasicBlock *NextCase = nullptr;
394-
for (auto Case : Switch->cases()) {
395-
if (Case.getCaseValue()->getValue() == NextState) {
396-
NextCase = Case.getCaseSuccessor();
397-
break;
398-
}
399-
}
400-
if (!NextCase)
401-
NextCase = Switch->getDefaultDest();
402-
return NextCase;
403-
}
404-
405389
namespace {
406390
/// ThreadingPath is a path in the control flow of a loop that can be threaded
407391
/// by cloning necessary basic blocks and replacing conditional branches with
@@ -835,19 +819,32 @@ struct AllSwitchPaths {
835819
TPaths = std::move(TempList);
836820
}
837821

822+
/// Fast helper to get the successor corresponding to a particular case value
823+
/// for a switch statement.
824+
BasicBlock *getNextCaseSuccessor(const APInt &NextState) {
825+
// Precompute the value => successor mapping
826+
if (CaseValToDest.empty()) {
827+
for (auto Case : Switch->cases()) {
828+
APInt CaseVal = Case.getCaseValue()->getValue();
829+
CaseValToDest[CaseVal] = Case.getCaseSuccessor();
830+
}
831+
}
832+
833+
auto SuccIt = CaseValToDest.find(NextState);
834+
return SuccIt == CaseValToDest.end() ? Switch->getDefaultDest()
835+
: SuccIt->second;
836+
}
837+
838838
// Two states are equivalent if they have the same switch destination.
839839
// Unify the states in different threading path if the states are equivalent.
840840
void unifyTPaths() {
841-
llvm::SmallDenseMap<BasicBlock *, APInt> DestToState;
841+
SmallDenseMap<BasicBlock *, APInt> DestToState;
842842
for (ThreadingPath &Path : TPaths) {
843843
APInt NextState = Path.getExitValue();
844-
BasicBlock *Dest = getNextCaseSuccessor(Switch, NextState);
845-
auto StateIt = DestToState.find(Dest);
846-
if (StateIt == DestToState.end()) {
847-
DestToState.insert({Dest, NextState});
844+
BasicBlock *Dest = getNextCaseSuccessor(NextState);
845+
auto [StateIt, Inserted] = DestToState.try_emplace(Dest, NextState);
846+
if (Inserted)
848847
continue;
849-
}
850-
851848
if (NextState != StateIt->second) {
852849
LLVM_DEBUG(dbgs() << "Next state in " << Path << " is equivalent to "
853850
<< StateIt->second << "\n");
@@ -861,6 +858,7 @@ struct AllSwitchPaths {
861858
BasicBlock *SwitchBlock;
862859
OptimizationRemarkEmitter *ORE;
863860
std::vector<ThreadingPath> TPaths;
861+
DenseMap<APInt, BasicBlock *> CaseValToDest;
864862
LoopInfo *LI;
865863
Loop *SwitchOuterLoop;
866864
};
@@ -1162,6 +1160,24 @@ struct TransformDFA {
11621160
SSAUpdate.RewriteAllUses(&DTU->getDomTree());
11631161
}
11641162

1163+
/// Helper to get the successor corresponding to a particular case value for
1164+
/// a switch statement.
1165+
/// TODO: Unify it with SwitchPaths->getNextCaseSuccessor(SwitchInst *Switch)
1166+
/// by updating cached value => successor mapping during threading.
1167+
static BasicBlock *getNextCaseSuccessor(SwitchInst *Switch,
1168+
const APInt &NextState) {
1169+
BasicBlock *NextCase = nullptr;
1170+
for (auto Case : Switch->cases()) {
1171+
if (Case.getCaseValue()->getValue() == NextState) {
1172+
NextCase = Case.getCaseSuccessor();
1173+
break;
1174+
}
1175+
}
1176+
if (!NextCase)
1177+
NextCase = Switch->getDefaultDest();
1178+
return NextCase;
1179+
}
1180+
11651181
/// Clones a basic block, and adds it to the CFG.
11661182
///
11671183
/// This function also includes updating phi nodes in the successors of the

0 commit comments

Comments
 (0)