Skip to content

Commit 2288724

Browse files
dianqkdianqk
committed
Address feedback
1 parent 9864e17 commit 2288724

File tree

1 file changed

+83
-70
lines changed

1 file changed

+83
-70
lines changed

‎llvm/lib/Transforms/Utils/SimplifyCFG.cpp‎

Lines changed: 83 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -5718,22 +5718,35 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
57185718
return Changed;
57195719
}
57205720

5721-
static bool casesAreContiguous(Value *Condition,
5722-
SmallVectorImpl<ConstantInt *> &Cases,
5723-
ConstantInt *&ContiguousCasesMin,
5724-
ConstantInt *&ContiguousCasesMax,
5725-
bool &IsWrapping) {
5721+
struct ContiguousCasesResult {
5722+
ConstantInt *Min;
5723+
ConstantInt *Max;
5724+
BasicBlock *Dest;
5725+
BasicBlock *OtherDest;
5726+
SmallVectorImpl<ConstantInt *> *Cases;
5727+
SmallVectorImpl<ConstantInt *> *OtherCases;
5728+
};
5729+
5730+
static std::optional<ContiguousCasesResult>
5731+
findContiguousCases(Value *Condition, SmallVectorImpl<ConstantInt *> &Cases,
5732+
SmallVectorImpl<ConstantInt *> &OtherCases,
5733+
BasicBlock *Dest, BasicBlock *OtherDest) {
57265734
assert(Cases.size() >= 1);
57275735

57285736
array_pod_sort(Cases.begin(), Cases.end(), constantIntSortPredicate);
5729-
auto Min = Cases.back()->getValue();
5730-
auto Max = Cases.front()->getValue();
5731-
auto Offset = Max - Min;
5732-
auto ContiguousOffset = Cases.size() - 1;
5737+
APInt Min = Cases.back()->getValue();
5738+
APInt Max = Cases.front()->getValue();
5739+
APInt Offset = Max - Min;
5740+
size_t ContiguousOffset = Cases.size() - 1;
57335741
if (Offset == ContiguousOffset) {
5734-
ContiguousCasesMin = Cases.back();
5735-
ContiguousCasesMax = Cases.front();
5736-
return true;
5742+
return ContiguousCasesResult{
5743+
/*Min=*/Cases.back(),
5744+
/*Max=*/Cases.front(),
5745+
/*Dest=*/Dest,
5746+
/*OtherDest=*/OtherDest,
5747+
/*Cases=*/&Cases,
5748+
/*OtherCases=*/&OtherCases,
5749+
};
57375750
}
57385751
ConstantRange CR = computeConstantRange(Condition, /*ForSigned=*/false);
57395752
// If this is a wrapping contiguous range, that is, [Min, OtherMin] +
@@ -5747,20 +5760,24 @@ static bool casesAreContiguous(Value *Condition,
57475760
return L->getValue() != R->getValue() + 1;
57485761
});
57495762
if (It == Cases.end())
5750-
return false;
5751-
auto *OtherMax = *It;
5752-
auto *OtherMin = *(It + 1);
5763+
return std::nullopt;
5764+
auto [OtherMax, OtherMin] = std::make_pair(*It, *std::next(It));
57535765
if ((Max - OtherMax->getValue()) + (OtherMin->getValue() - Min) ==
57545766
Cases.size() - 2) {
5755-
ContiguousCasesMin = cast<ConstantInt>(
5756-
ConstantInt::get(OtherMin->getType(), OtherMin->getValue() + 1));
5757-
ContiguousCasesMax = cast<ConstantInt>(
5758-
ConstantInt::get(OtherMax->getType(), OtherMax->getValue() - 1));
5759-
IsWrapping = true;
5760-
return true;
5767+
return ContiguousCasesResult{
5768+
/*Min=*/cast<ConstantInt>(
5769+
ConstantInt::get(OtherMin->getType(), OtherMin->getValue() + 1)),
5770+
/*Max=*/
5771+
cast<ConstantInt>(
5772+
ConstantInt::get(OtherMax->getType(), OtherMax->getValue() - 1)),
5773+
/*Dest=*/OtherDest,
5774+
/*OtherDest=*/Dest,
5775+
/*Cases=*/&OtherCases,
5776+
/*OtherCases=*/&Cases,
5777+
};
57615778
}
57625779
}
5763-
return false;
5780+
return std::nullopt;
57645781
}
57655782

57665783
static void createUnreachableSwitchDefault(SwitchInst *Switch,
@@ -5797,7 +5814,6 @@ bool SimplifyCFGOpt::turnSwitchRangeIntoICmp(SwitchInst *SI,
57975814
bool HasDefault = !SI->defaultDestUnreachable();
57985815

57995816
auto *BB = SI->getParent();
5800-
58015817
// Partition the cases into two sets with different destinations.
58025818
BasicBlock *DestA = HasDefault ? SI->getDefaultDest() : nullptr;
58035819
BasicBlock *DestB = nullptr;
@@ -5831,67 +5847,64 @@ bool SimplifyCFGOpt::turnSwitchRangeIntoICmp(SwitchInst *SI,
58315847
assert(!CasesA.empty() || HasDefault);
58325848

58335849
// Figure out if one of the sets of cases form a contiguous range.
5834-
ConstantInt *ContiguousCasesMin = nullptr;
5835-
ConstantInt *ContiguousCasesMax = nullptr;
5836-
BasicBlock *ContiguousDest = nullptr;
5837-
BasicBlock *OtherDest = nullptr;
5838-
bool IsWrapping = false;
5839-
SmallVectorImpl<ConstantInt *> *ContiguousCases = &CasesA;
5840-
SmallVectorImpl<ConstantInt *> *OtherCases = &CasesB;
5850+
std::optional<ContiguousCasesResult> ContiguousCases;
58415851

58425852
// Only one icmp is needed when there is only one case.
5843-
if (!HasDefault && CasesA.size() == 1) {
5844-
ContiguousCasesMax = CasesA[0];
5845-
ContiguousCasesMin = CasesA[0];
5846-
ContiguousDest = DestA;
5847-
OtherDest = DestB;
5848-
} else if (CasesB.size() == 1) {
5849-
ContiguousCasesMax = CasesB[0];
5850-
ContiguousCasesMin = CasesB[0];
5851-
ContiguousDest = DestB;
5852-
OtherDest = DestA;
5853-
std::swap(ContiguousCases, OtherCases);
5854-
}
5853+
if (!HasDefault && CasesA.size() == 1)
5854+
ContiguousCases = ContiguousCasesResult{
5855+
/*Min=*/CasesA[0],
5856+
/*Max=*/CasesA[0],
5857+
/*Dest=*/DestA,
5858+
/*OtherDest=*/DestB,
5859+
/*Cases=*/&CasesA,
5860+
/*OtherCases=*/&CasesB,
5861+
};
5862+
else if (CasesB.size() == 1)
5863+
ContiguousCases = ContiguousCasesResult{
5864+
/*Min=*/CasesB[0],
5865+
/*Max=*/CasesB[0],
5866+
/*Dest=*/DestB,
5867+
/*OtherDest=*/DestA,
5868+
/*Cases=*/&CasesB,
5869+
/*OtherCases=*/&CasesA,
5870+
};
5871+
58555872
// Correctness: Cases to the default destination cannot be contiguous cases.
5856-
else if (!HasDefault && !CasesA.empty() &&
5857-
casesAreContiguous(SI->getCondition(), CasesA, ContiguousCasesMin,
5858-
ContiguousCasesMax, IsWrapping)) {
5859-
ContiguousDest = DestA;
5860-
OtherDest = DestB;
5861-
} else if (casesAreContiguous(SI->getCondition(), CasesB, ContiguousCasesMin,
5862-
ContiguousCasesMax, IsWrapping)) {
5863-
ContiguousDest = DestB;
5864-
OtherDest = DestA;
5865-
std::swap(ContiguousCases, OtherCases);
5866-
} else
5873+
if (!ContiguousCases && !HasDefault && !CasesA.empty())
5874+
if (auto Result = findContiguousCases(SI->getCondition(), CasesA, CasesB,
5875+
DestA, DestB))
5876+
ContiguousCases = *Result;
5877+
5878+
if (!ContiguousCases)
5879+
if (auto Result = findContiguousCases(SI->getCondition(), CasesB, CasesA,
5880+
DestB, DestA))
5881+
ContiguousCases = *Result;
5882+
5883+
if (!ContiguousCases)
58675884
return false;
58685885

5869-
if (IsWrapping) {
5870-
std::swap(ContiguousDest, OtherDest);
5871-
std::swap(ContiguousCases, OtherCases);
5872-
}
5886+
auto [Min, Max, Dest, OtherDest, Cases, OtherCases] = *ContiguousCases;
58735887

58745888
// Start building the compare and branch.
58755889

5876-
Constant *Offset = ConstantExpr::getNeg(ContiguousCasesMin);
5890+
Constant *Offset = ConstantExpr::getNeg(Min);
58775891
Constant *NumCases = ConstantInt::get(Offset->getType(),
5878-
ContiguousCasesMax->getValue() -
5879-
ContiguousCasesMin->getValue() + 1);
5892+
Max->getValue() - Min->getValue() + 1);
58805893
BranchInst *NewBI;
58815894
if (NumCases->isOneValue()) {
5882-
assert(ContiguousCasesMax->getValue() == ContiguousCasesMin->getValue());
5883-
Value *Cmp = Builder.CreateICmpEQ(SI->getCondition(), ContiguousCasesMin);
5884-
NewBI = Builder.CreateCondBr(Cmp, ContiguousDest, OtherDest);
5895+
assert(Max->getValue() == Min->getValue());
5896+
Value *Cmp = Builder.CreateICmpEQ(SI->getCondition(), Min);
5897+
NewBI = Builder.CreateCondBr(Cmp, Dest, OtherDest);
58855898
}
58865899
// If NumCases overflowed, then all possible values jump to the successor.
5887-
else if (NumCases->isNullValue() && !ContiguousCases->empty()) {
5888-
NewBI = Builder.CreateBr(ContiguousDest);
5900+
else if (NumCases->isNullValue() && !Cases->empty()) {
5901+
NewBI = Builder.CreateBr(Dest);
58895902
} else {
58905903
Value *Sub = SI->getCondition();
58915904
if (!Offset->isNullValue())
58925905
Sub = Builder.CreateAdd(Sub, Offset, Sub->getName() + ".off");
58935906
Value *Cmp = Builder.CreateICmpULT(Sub, NumCases, "switch");
5894-
NewBI = Builder.CreateCondBr(Cmp, ContiguousDest, OtherDest);
5907+
NewBI = Builder.CreateCondBr(Cmp, Dest, OtherDest);
58955908
}
58965909

58975910
// Update weight for the newly-created conditional branch.
@@ -5902,7 +5915,7 @@ bool SimplifyCFGOpt::turnSwitchRangeIntoICmp(SwitchInst *SI,
59025915
uint64_t TrueWeight = 0;
59035916
uint64_t FalseWeight = 0;
59045917
for (size_t I = 0, E = Weights.size(); I != E; ++I) {
5905-
if (SI->getSuccessor(I) == ContiguousDest)
5918+
if (SI->getSuccessor(I) == Dest)
59065919
TrueWeight += Weights[I];
59075920
else
59085921
FalseWeight += Weights[I];
@@ -5917,9 +5930,9 @@ bool SimplifyCFGOpt::turnSwitchRangeIntoICmp(SwitchInst *SI,
59175930
}
59185931

59195932
// Prune obsolete incoming values off the successors' PHI nodes.
5920-
for (auto BBI = ContiguousDest->begin(); isa<PHINode>(BBI); ++BBI) {
5921-
unsigned PreviousEdges = ContiguousCases->size();
5922-
if (ContiguousDest == SI->getDefaultDest())
5933+
for (auto BBI = Dest->begin(); isa<PHINode>(BBI); ++BBI) {
5934+
unsigned PreviousEdges = Cases->size();
5935+
if (Dest == SI->getDefaultDest())
59235936
++PreviousEdges;
59245937
for (unsigned I = 0, E = PreviousEdges - 1; I != E; ++I)
59255938
cast<PHINode>(BBI)->removeIncomingValue(SI->getParent());

0 commit comments

Comments
 (0)