Skip to content

Commit a6c8cc3

Browse files
committed
Fixed r158979.
Original message: Performance optimizations: - SwitchInst: case values stored separately from Operands List. It allows to make faster access to individual case value numbers or ranges. - Optimized IntItem, added APInt value caching. - Optimized IntegersSubsetGeneric: added optimizations for cases when subset is single number or when subset consists from single numbers only. llvm-svn: 158997
1 parent c3b8111 commit a6c8cc3

File tree

4 files changed

+180
-56
lines changed

4 files changed

+180
-56
lines changed

llvm/include/llvm/Instructions.h

Lines changed: 101 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,10 +2442,31 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BranchInst, Value)
24422442
class SwitchInst : public TerminatorInst {
24432443
void *operator new(size_t, unsigned); // DO NOT IMPLEMENT
24442444
unsigned ReservedSpace;
2445+
// Operands format:
24452446
// Operand[0] = Value to switch on
24462447
// Operand[1] = Default basic block destination
24472448
// Operand[2n ] = Value to match
24482449
// Operand[2n+1] = BasicBlock to go to on match
2450+
2451+
// Store case values separately from operands list. We needn't User-Use
2452+
// concept here, since it is just a case value, it will always constant,
2453+
// and case value couldn't reused with another instructions/values.
2454+
// Additionally:
2455+
// It allows us to use custom type for case values that is not inherited
2456+
// from Value. Since case value is a complex type that implements
2457+
// the subset of integers, we needn't extract sub-constants within
2458+
// slow getAggregateElement method.
2459+
// For case values we will use std::list to by two reasons:
2460+
// 1. It allows to add/remove cases without whole collection reallocation.
2461+
// 2. In most of cases we needn't random access.
2462+
// Currently case values are also stored in Operands List, but it will moved
2463+
// out in future commits.
2464+
typedef std::list<IntegersSubset> Subsets;
2465+
typedef Subsets::iterator SubsetsIt;
2466+
typedef Subsets::const_iterator SubsetsConstIt;
2467+
2468+
Subsets TheSubsets;
2469+
24492470
SwitchInst(const SwitchInst &SI);
24502471
void init(Value *Value, BasicBlock *Default, unsigned NumReserved);
24512472
void growOperands();
@@ -2470,12 +2491,20 @@ class SwitchInst : public TerminatorInst {
24702491
virtual SwitchInst *clone_impl() const;
24712492
public:
24722493

2473-
template <class SwitchInstTy, class ConstantIntTy, class BasicBlockTy>
2494+
// FIXME: Currently there are a lot of unclean template parameters,
2495+
// we need to make refactoring in future.
2496+
// All these parameters are used to implement both iterator and const_iterator
2497+
// without code duplication.
2498+
// SwitchInstTy may be "const SwitchInst" or "SwitchInst"
2499+
// ConstantIntTy may be "const ConstantInt" or "ConstantInt"
2500+
// SubsetsItTy may be SubsetsConstIt or SubsetsIt
2501+
// BasicBlockTy may be "const BasicBlock" or "BasicBlock"
2502+
template <class SwitchInstTy, class ConstantIntTy,
2503+
class SubsetsItTy, class BasicBlockTy>
24742504
class CaseIteratorT;
24752505

2476-
typedef CaseIteratorT<const SwitchInst, const ConstantInt, const BasicBlock>
2477-
ConstCaseIt;
2478-
2506+
typedef CaseIteratorT<const SwitchInst, const ConstantInt,
2507+
SubsetsConstIt, const BasicBlock> ConstCaseIt;
24792508
class CaseIt;
24802509

24812510
// -2
@@ -2516,34 +2545,34 @@ class SwitchInst : public TerminatorInst {
25162545
/// Returns a read/write iterator that points to the first
25172546
/// case in SwitchInst.
25182547
CaseIt case_begin() {
2519-
return CaseIt(this, 0);
2548+
return CaseIt(this, 0, TheSubsets.begin());
25202549
}
25212550
/// Returns a read-only iterator that points to the first
25222551
/// case in the SwitchInst.
25232552
ConstCaseIt case_begin() const {
2524-
return ConstCaseIt(this, 0);
2553+
return ConstCaseIt(this, 0, TheSubsets.begin());
25252554
}
25262555

25272556
/// Returns a read/write iterator that points one past the last
25282557
/// in the SwitchInst.
25292558
CaseIt case_end() {
2530-
return CaseIt(this, getNumCases());
2559+
return CaseIt(this, getNumCases(), TheSubsets.end());
25312560
}
25322561
/// Returns a read-only iterator that points one past the last
25332562
/// in the SwitchInst.
25342563
ConstCaseIt case_end() const {
2535-
return ConstCaseIt(this, getNumCases());
2564+
return ConstCaseIt(this, getNumCases(), TheSubsets.end());
25362565
}
25372566
/// Returns an iterator that points to the default case.
25382567
/// Note: this iterator allows to resolve successor only. Attempt
25392568
/// to resolve case value causes an assertion.
25402569
/// Also note, that increment and decrement also causes an assertion and
25412570
/// makes iterator invalid.
25422571
CaseIt case_default() {
2543-
return CaseIt(this, DefaultPseudoIndex);
2572+
return CaseIt(this, DefaultPseudoIndex, TheSubsets.end());
25442573
}
25452574
ConstCaseIt case_default() const {
2546-
return ConstCaseIt(this, DefaultPseudoIndex);
2575+
return ConstCaseIt(this, DefaultPseudoIndex, TheSubsets.end());
25472576
}
25482577

25492578
/// findCaseValue - Search all of the case values for the specified constant.
@@ -2597,7 +2626,7 @@ class SwitchInst : public TerminatorInst {
25972626
/// Note:
25982627
/// This action invalidates iterators for all cases following the one removed,
25992628
/// including the case_end() iterator.
2600-
void removeCase(CaseIt i);
2629+
void removeCase(CaseIt& i);
26012630

26022631
unsigned getNumSuccessors() const { return getNumOperands()/2; }
26032632
BasicBlock *getSuccessor(unsigned idx) const {
@@ -2622,24 +2651,38 @@ class SwitchInst : public TerminatorInst {
26222651

26232652
// Case iterators definition.
26242653

2625-
template <class SwitchInstTy, class ConstantIntTy, class BasicBlockTy>
2654+
template <class SwitchInstTy, class ConstantIntTy,
2655+
class SubsetsItTy, class BasicBlockTy>
26262656
class CaseIteratorT {
26272657
protected:
26282658

26292659
SwitchInstTy *SI;
2630-
unsigned Index;
2631-
2632-
public:
2633-
2634-
typedef CaseIteratorT<SwitchInstTy, ConstantIntTy, BasicBlockTy> Self;
2660+
unsigned long Index;
2661+
SubsetsItTy SubsetIt;
26352662

26362663
/// Initializes case iterator for given SwitchInst and for given
26372664
/// case number.
2638-
CaseIteratorT(SwitchInstTy *SI, unsigned CaseNum) {
2665+
friend class SwitchInst;
2666+
CaseIteratorT(SwitchInstTy *SI, unsigned SuccessorIndex,
2667+
SubsetsItTy CaseValueIt) {
26392668
this->SI = SI;
2640-
Index = CaseNum;
2669+
Index = SuccessorIndex;
2670+
this->SubsetIt = CaseValueIt;
26412671
}
26422672

2673+
public:
2674+
typedef typename SubsetsItTy::reference IntegersSubsetRef;
2675+
typedef CaseIteratorT<SwitchInstTy, ConstantIntTy,
2676+
SubsetsItTy, BasicBlockTy> Self;
2677+
2678+
CaseIteratorT(SwitchInstTy *SI, unsigned CaseNum) {
2679+
this->SI = SI;
2680+
Index = CaseNum;
2681+
SubsetIt = SI->TheSubsets.begin();
2682+
std::advance(SubsetIt, CaseNum);
2683+
}
2684+
2685+
26432686
/// Initializes case iterator for given SwitchInst and for given
26442687
/// TerminatorInst's successor index.
26452688
static Self fromSuccessorIndex(SwitchInstTy *SI, unsigned SuccessorIndex) {
@@ -2654,19 +2697,18 @@ class SwitchInst : public TerminatorInst {
26542697
/// @Deprecated
26552698
ConstantIntTy *getCaseValue() {
26562699
assert(Index < SI->getNumCases() && "Index out the number of cases.");
2657-
IntegersSubset CaseRanges =
2658-
reinterpret_cast<Constant*>(SI->getOperand(2 + Index*2));
2659-
IntegersSubset::Range R = CaseRanges.getItem(0);
2700+
IntegersSubsetRef CaseRanges = *SubsetIt;
26602701

26612702
// FIXME: Currently we work with ConstantInt based cases.
26622703
// So return CaseValue as ConstantInt.
2663-
return R.getLow().toConstantInt();
2704+
return CaseRanges.getSingleNumber(0).toConstantInt();
26642705
}
26652706

26662707
/// Resolves case value for current case.
2708+
// IntegersSubsetRef getCaseValueEx() {
26672709
IntegersSubset getCaseValueEx() {
26682710
assert(Index < SI->getNumCases() && "Index out the number of cases.");
2669-
return reinterpret_cast<Constant*>(SI->getOperand(2 + Index*2));
2711+
return *SubsetIt;
26702712
}
26712713

26722714
/// Resolves successor for current case.
@@ -2689,9 +2731,14 @@ class SwitchInst : public TerminatorInst {
26892731

26902732
Self operator++() {
26912733
// Check index correctness after increment.
2692-
// Note: Index == getNumCases() means end().
2693-
assert(Index+1 <= SI->getNumCases() && "Index out the number of cases.");
2734+
// Note: Index == getNumCases() means end().
2735+
unsigned NumCases = SI->getNumCases();
2736+
assert(Index+1 <= NumCases && "Index out the number of cases.");
26942737
++Index;
2738+
if (Index == 0)
2739+
SubsetIt = SI->TheSubsets.begin();
2740+
else
2741+
++SubsetIt;
26952742
return *this;
26962743
}
26972744
Self operator++(int) {
@@ -2703,9 +2750,18 @@ class SwitchInst : public TerminatorInst {
27032750
// Check index correctness after decrement.
27042751
// Note: Index == getNumCases() means end().
27052752
// Also allow "-1" iterator here. That will became valid after ++.
2706-
assert((Index == 0 || Index-1 <= SI->getNumCases()) &&
2753+
unsigned NumCases = SI->getNumCases();
2754+
assert((Index == 0 || Index-1 <= NumCases) &&
27072755
"Index out the number of cases.");
27082756
--Index;
2757+
if (Index == NumCases) {
2758+
SubsetIt = SI->TheSubsets.end();
2759+
return *this;
2760+
}
2761+
2762+
if (Index != -1UL)
2763+
--SubsetIt;
2764+
27092765
return *this;
27102766
}
27112767
Self operator--(int) {
@@ -2723,14 +2779,25 @@ class SwitchInst : public TerminatorInst {
27232779
}
27242780
};
27252781

2726-
class CaseIt : public CaseIteratorT<SwitchInst, ConstantInt, BasicBlock> {
2782+
class CaseIt : public CaseIteratorT<SwitchInst, ConstantInt,
2783+
SubsetsIt, BasicBlock> {
2784+
typedef CaseIteratorT<SwitchInst, ConstantInt, SubsetsIt, BasicBlock>
2785+
ParentTy;
2786+
2787+
protected:
2788+
friend class SwitchInst;
2789+
CaseIt(SwitchInst *SI, unsigned CaseNum, SubsetsIt SubsetIt) :
2790+
ParentTy(SI, CaseNum, SubsetIt) {}
27272791

2728-
typedef CaseIteratorT<SwitchInst, ConstantInt, BasicBlock> ParentTy;
2792+
void updateCaseValueOperand(IntegersSubset& V) {
2793+
SI->setOperand(2 + Index*2, reinterpret_cast<Value*>((Constant*)V));
2794+
}
27292795

27302796
public:
2797+
2798+
CaseIt(SwitchInst *SI, unsigned CaseNum) : ParentTy(SI, CaseNum) {}
27312799

27322800
CaseIt(const ParentTy& Src) : ParentTy(Src) {}
2733-
CaseIt(SwitchInst *SI, unsigned CaseNum) : ParentTy(SI, CaseNum) {}
27342801

27352802
/// Sets the new value for current case.
27362803
/// @Deprecated.
@@ -2740,14 +2807,15 @@ class SwitchInst : public TerminatorInst {
27402807
// FIXME: Currently we work with ConstantInt based cases.
27412808
// So inititalize IntItem container directly from ConstantInt.
27422809
Mapping.add(IntItem::fromConstantInt(V));
2743-
SI->setOperand(2 + Index*2,
2744-
reinterpret_cast<Value*>((Constant*)Mapping.getCase()));
2810+
*SubsetIt = Mapping.getCase();
2811+
updateCaseValueOperand(*SubsetIt);
27452812
}
27462813

27472814
/// Sets the new value for current case.
27482815
void setValueEx(IntegersSubset& V) {
27492816
assert(Index < SI->getNumCases() && "Index out the number of cases.");
2750-
SI->setOperand(2 + Index*2, reinterpret_cast<Value*>((Constant*)V));
2817+
*SubsetIt = V;
2818+
updateCaseValueOperand(*SubsetIt);
27512819
}
27522820

27532821
/// Sets the new successor for current case.

0 commit comments

Comments
 (0)