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.



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@158997 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Instructions.h

@@ -2442,10 +2442,31 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BranchInst, Value)
 class SwitchInst : public TerminatorInst {
   void *operator new(size_t, unsigned);  // DO NOT IMPLEMENT
   unsigned ReservedSpace;
+  // Operands format:
   // Operand[0]    = Value to switch on
   // Operand[1]    = Default basic block destination
   // Operand[2n  ] = Value to match
   // Operand[2n+1] = BasicBlock to go to on match
+  
+  // Store case values separately from operands list. We needn't User-Use
+  // concept here, since it is just a case value, it will always constant,
+  // and case value couldn't reused with another instructions/values.
+  // Additionally:
+  // It allows us to use custom type for case values that is not inherited
+  // from Value. Since case value is a complex type that implements
+  // the subset of integers, we needn't extract sub-constants within
+  // slow getAggregateElement method.
+  // For case values we will use std::list to by two reasons:
+  // 1. It allows to add/remove cases without whole collection reallocation.
+  // 2. In most of cases we needn't random access.
+  // Currently case values are also stored in Operands List, but it will moved
+  // out in future commits.
+  typedef std::list<IntegersSubset> Subsets;
+  typedef Subsets::iterator SubsetsIt;
+  typedef Subsets::const_iterator SubsetsConstIt;
+  
+  Subsets TheSubsets;
+  
   SwitchInst(const SwitchInst &SI);
   void init(Value *Value, BasicBlock *Default, unsigned NumReserved);
   void growOperands();
@@ -2470,12 +2491,20 @@ protected:
   virtual SwitchInst *clone_impl() const;
 public:
   
-  template <class SwitchInstTy, class ConstantIntTy, class BasicBlockTy> 
+  // FIXME: Currently there are a lot of unclean template parameters,
+  // we need to make refactoring in future.
+  // All these parameters are used to implement both iterator and const_iterator
+  // without code duplication.
+  // SwitchInstTy may be "const SwitchInst" or "SwitchInst"
+  // ConstantIntTy may be "const ConstantInt" or "ConstantInt"
+  // SubsetsItTy may be SubsetsConstIt or SubsetsIt
+  // BasicBlockTy may be "const BasicBlock" or "BasicBlock"
+  template <class SwitchInstTy, class ConstantIntTy,
+            class SubsetsItTy, class BasicBlockTy> 
     class CaseIteratorT;
 
-  typedef CaseIteratorT<const SwitchInst, const ConstantInt, const BasicBlock>
-      ConstCaseIt;
-  
+  typedef CaseIteratorT<const SwitchInst, const ConstantInt,
+                        SubsetsConstIt, const BasicBlock> ConstCaseIt;
   class CaseIt;
   
   // -2
@@ -2516,23 +2545,23 @@ public:
   /// Returns a read/write iterator that points to the first
   /// case in SwitchInst.
   CaseIt case_begin() {
-    return CaseIt(this, 0);
+    return CaseIt(this, 0, TheSubsets.begin());
   }
   /// Returns a read-only iterator that points to the first
   /// case in the SwitchInst.
   ConstCaseIt case_begin() const {
-    return ConstCaseIt(this, 0);
+    return ConstCaseIt(this, 0, TheSubsets.begin());
   }
   
   /// Returns a read/write iterator that points one past the last
   /// in the SwitchInst.
   CaseIt case_end() {
-    return CaseIt(this, getNumCases());
+    return CaseIt(this, getNumCases(), TheSubsets.end());
   }
   /// Returns a read-only iterator that points one past the last
   /// in the SwitchInst.
   ConstCaseIt case_end() const {
-    return ConstCaseIt(this, getNumCases());
+    return ConstCaseIt(this, getNumCases(), TheSubsets.end());
   }
   /// Returns an iterator that points to the default case.
   /// Note: this iterator allows to resolve successor only. Attempt
@@ -2540,10 +2569,10 @@ public:
   /// Also note, that increment and decrement also causes an assertion and
   /// makes iterator invalid. 
   CaseIt case_default() {
-    return CaseIt(this, DefaultPseudoIndex);
+    return CaseIt(this, DefaultPseudoIndex, TheSubsets.end());
   }
   ConstCaseIt case_default() const {
-    return ConstCaseIt(this, DefaultPseudoIndex);
+    return ConstCaseIt(this, DefaultPseudoIndex, TheSubsets.end());
   }
   
   /// findCaseValue - Search all of the case values for the specified constant.
@@ -2597,7 +2626,7 @@ public:
   /// Note:
   /// This action invalidates iterators for all cases following the one removed,
   /// including the case_end() iterator.
-  void removeCase(CaseIt i);
+  void removeCase(CaseIt& i);
 
   unsigned getNumSuccessors() const { return getNumOperands()/2; }
   BasicBlock *getSuccessor(unsigned idx) const {
@@ -2622,24 +2651,38 @@ public:
   
   // Case iterators definition.
 
-  template <class SwitchInstTy, class ConstantIntTy, class BasicBlockTy> 
+  template <class SwitchInstTy, class ConstantIntTy,
+            class SubsetsItTy, class BasicBlockTy> 
   class CaseIteratorT {
   protected:
     
     SwitchInstTy *SI;
-    unsigned Index;
-    
-  public:
-    
-    typedef CaseIteratorT<SwitchInstTy, ConstantIntTy, BasicBlockTy> Self;
+    unsigned long Index;
+    SubsetsItTy SubsetIt;
     
     /// Initializes case iterator for given SwitchInst and for given
     /// case number.    
-    CaseIteratorT(SwitchInstTy *SI, unsigned CaseNum) {
+    friend class SwitchInst;
+    CaseIteratorT(SwitchInstTy *SI, unsigned SuccessorIndex,
+                  SubsetsItTy CaseValueIt) {
       this->SI = SI;
-      Index = CaseNum;
+      Index = SuccessorIndex;
+      this->SubsetIt = CaseValueIt;
     }
     
+  public:
+    typedef typename SubsetsItTy::reference IntegersSubsetRef;
+    typedef CaseIteratorT<SwitchInstTy, ConstantIntTy,
+                          SubsetsItTy, BasicBlockTy> Self;
+    
+    CaseIteratorT(SwitchInstTy *SI, unsigned CaseNum) {
+          this->SI = SI;
+          Index = CaseNum;
+          SubsetIt = SI->TheSubsets.begin();
+          std::advance(SubsetIt, CaseNum);
+        }
+        
+    
     /// Initializes case iterator for given SwitchInst and for given
     /// TerminatorInst's successor index.
     static Self fromSuccessorIndex(SwitchInstTy *SI, unsigned SuccessorIndex) {
@@ -2654,19 +2697,18 @@ public:
     /// @Deprecated
     ConstantIntTy *getCaseValue() {
       assert(Index < SI->getNumCases() && "Index out the number of cases.");
-      IntegersSubset CaseRanges =
-          reinterpret_cast<Constant*>(SI->getOperand(2 + Index*2));
-      IntegersSubset::Range R = CaseRanges.getItem(0);
+      IntegersSubsetRef CaseRanges = *SubsetIt;
       
       // FIXME: Currently we work with ConstantInt based cases.
       // So return CaseValue as ConstantInt.
-      return R.getLow().toConstantInt();
+      return CaseRanges.getSingleNumber(0).toConstantInt();
     }
 
     /// Resolves case value for current case.
+//    IntegersSubsetRef getCaseValueEx() {
     IntegersSubset getCaseValueEx() {
       assert(Index < SI->getNumCases() && "Index out the number of cases.");
-      return reinterpret_cast<Constant*>(SI->getOperand(2 + Index*2));
+      return *SubsetIt;
     }
     
     /// Resolves successor for current case.
@@ -2689,9 +2731,14 @@ public:
     
     Self operator++() {
       // Check index correctness after increment.
-      // Note: Index == getNumCases() means end().      
-      assert(Index+1 <= SI->getNumCases() && "Index out the number of cases.");
+      // Note: Index == getNumCases() means end().
+      unsigned NumCases = SI->getNumCases();
+      assert(Index+1 <= NumCases && "Index out the number of cases.");
       ++Index;
+      if (Index == 0)
+        SubsetIt = SI->TheSubsets.begin();
+      else
+        ++SubsetIt;
       return *this;
     }
     Self operator++(int) {
@@ -2703,9 +2750,18 @@ public:
       // Check index correctness after decrement.
       // Note: Index == getNumCases() means end().
       // Also allow "-1" iterator here. That will became valid after ++.
-      assert((Index == 0 || Index-1 <= SI->getNumCases()) &&
+      unsigned NumCases = SI->getNumCases();
+      assert((Index == 0 || Index-1 <= NumCases) &&
              "Index out the number of cases.");
       --Index;
+      if (Index == NumCases) {
+        SubsetIt = SI->TheSubsets.end();
+        return *this;
+      }
+        
+      if (Index != -1UL)
+        --SubsetIt;
+      
       return *this;
     }
     Self operator--(int) {
@@ -2723,14 +2779,25 @@ public:
     }
   };
 
-  class CaseIt : public CaseIteratorT<SwitchInst, ConstantInt, BasicBlock> {
+  class CaseIt : public CaseIteratorT<SwitchInst, ConstantInt,
+                                      SubsetsIt, BasicBlock> {
+    typedef CaseIteratorT<SwitchInst, ConstantInt, SubsetsIt, BasicBlock>
+      ParentTy;
     
-    typedef CaseIteratorT<SwitchInst, ConstantInt, BasicBlock> ParentTy;
+  protected:
+    friend class SwitchInst;
+    CaseIt(SwitchInst *SI, unsigned CaseNum, SubsetsIt SubsetIt) :
+      ParentTy(SI, CaseNum, SubsetIt) {}
+    
+    void updateCaseValueOperand(IntegersSubset& V) {
+      SI->setOperand(2 + Index*2, reinterpret_cast<Value*>((Constant*)V));      
+    }
   
   public:
+
+    CaseIt(SwitchInst *SI, unsigned CaseNum) : ParentTy(SI, CaseNum) {}    
     
     CaseIt(const ParentTy& Src) : ParentTy(Src) {}
-    CaseIt(SwitchInst *SI, unsigned CaseNum) : ParentTy(SI, CaseNum) {}
 
     /// Sets the new value for current case.    
     /// @Deprecated.
@@ -2740,14 +2807,15 @@ public:
       // FIXME: Currently we work with ConstantInt based cases.
       // So inititalize IntItem container directly from ConstantInt.
       Mapping.add(IntItem::fromConstantInt(V));
-      SI->setOperand(2 + Index*2,
-          reinterpret_cast<Value*>((Constant*)Mapping.getCase()));
+      *SubsetIt = Mapping.getCase();
+      updateCaseValueOperand(*SubsetIt);
     }
     
     /// Sets the new value for current case.
     void setValueEx(IntegersSubset& V) {
       assert(Index < SI->getNumCases() && "Index out the number of cases.");
-      SI->setOperand(2 + Index*2, reinterpret_cast<Value*>((Constant*)V));      
+      *SubsetIt = V;
+      updateCaseValueOperand(*SubsetIt);   
     }
     
     /// Sets the new successor for current case.

include/llvm/Support/IntegersSubset.h

@@ -40,26 +40,26 @@ namespace llvm {
 
 #define INT_ITEM_DEFINE_COMPARISON(op,func) \
   bool operator op (const APInt& RHS) const { \
-    return ConstantIntVal->getValue().func(RHS); \
+    return getAPIntValue().func(RHS); \
   }
   
 #define INT_ITEM_DEFINE_UNARY_OP(op) \
   IntItem operator op () const { \
-    APInt res = op(ConstantIntVal->getValue()); \
+    APInt res = op(getAPIntValue()); \
     Constant *NewVal = ConstantInt::get(ConstantIntVal->getContext(), res); \
     return IntItem(cast<ConstantInt>(NewVal)); \
   }
   
 #define INT_ITEM_DEFINE_BINARY_OP(op) \
   IntItem operator op (const APInt& RHS) const { \
-    APInt res = ConstantIntVal->getValue() op RHS; \
+    APInt res = getAPIntValue() op RHS; \
     Constant *NewVal = ConstantInt::get(ConstantIntVal->getContext(), res); \
     return IntItem(cast<ConstantInt>(NewVal)); \
   }
   
 #define INT_ITEM_DEFINE_ASSIGNMENT_BY_OP(op) \
   IntItem& operator op (const APInt& RHS) {\
-    APInt res = ConstantIntVal->getValue();\
+    APInt res = getAPIntValue();\
     res op RHS; \
     Constant *NewVal = ConstantInt::get(ConstantIntVal->getContext(), res); \
     ConstantIntVal = cast<ConstantInt>(NewVal); \
@@ -68,7 +68,7 @@ namespace llvm {
   
 #define INT_ITEM_DEFINE_PREINCDEC(op) \
     IntItem& operator op () { \
-      APInt res = ConstantIntVal->getValue(); \
+      APInt res = getAPIntValue(); \
       op(res); \
       Constant *NewVal = ConstantInt::get(ConstantIntVal->getContext(), res); \
       ConstantIntVal = cast<ConstantInt>(NewVal); \
@@ -77,7 +77,7 @@ namespace llvm {
 
 #define INT_ITEM_DEFINE_POSTINCDEC(op) \
     IntItem& operator op (int) { \
-      APInt res = ConstantIntVal->getValue();\
+      APInt res = getAPIntValue();\
       op(res); \
       Constant *NewVal = ConstantInt::get(ConstantIntVal->getContext(), res); \
       OldConstantIntVal = ConstantIntVal; \
@@ -87,18 +87,24 @@ namespace llvm {
   
 #define INT_ITEM_DEFINE_OP_STANDARD_INT(RetTy, op, IntTy) \
   RetTy operator op (IntTy RHS) const { \
-    return (*this) op APInt(ConstantIntVal->getValue().getBitWidth(), RHS); \
+    return (*this) op APInt(getAPIntValue().getBitWidth(), RHS); \
   }  
 
 class IntItem {
   ConstantInt *ConstantIntVal;
-  IntItem(const ConstantInt *V) : ConstantIntVal(const_cast<ConstantInt*>(V)) {}
+  const APInt* APIntVal;
+  IntItem(const ConstantInt *V) :
+    ConstantIntVal(const_cast<ConstantInt*>(V)),
+    APIntVal(&ConstantIntVal->getValue()){}
+  const APInt& getAPIntValue() const {
+    return *APIntVal;
+  }
 public:
   
   IntItem() {}
   
   operator const APInt&() const {
-    return (const APInt&)ConstantIntVal->getValue();
+    return getAPIntValue();
   }  
   
   // Propagate APInt operators.
@@ -137,7 +143,7 @@ public:
   
   // Special case for <<=
   IntItem& operator <<= (unsigned RHS) {
-    APInt res = ConstantIntVal->getValue();
+    APInt res = getAPIntValue();
     res <<= RHS;
     Constant *NewVal = ConstantInt::get(ConstantIntVal->getContext(), res);
     ConstantIntVal = cast<ConstantInt>(NewVal);
@@ -278,9 +284,9 @@ public:
   // In short, for more compact memory consumption we can store flat
   // numbers collection, and define range as pair of indices.
   // In that case we can safe some memory on 32 bit machines.
-  typedef std::list<IntTy> FlatCollectionTy;
+  typedef std::vector<IntTy> FlatCollectionTy;
   typedef std::pair<IntTy*, IntTy*> RangeLinkTy;
-  typedef SmallVector<RangeLinkTy, 64> RangeLinksTy;
+  typedef std::vector<RangeLinkTy> RangeLinksTy;
   typedef typename RangeLinksTy::const_iterator RangeLinksConstIt;
   
   typedef IntegersSubsetGeneric<IntTy> self;
@@ -290,21 +296,33 @@ protected:
   FlatCollectionTy FlatCollection;
   RangeLinksTy RangeLinks;
   
+  bool IsSingleNumber;
+  bool IsSingleNumbersOnly;
+  
 public:
   
   template<class RangesCollectionTy>
   explicit IntegersSubsetGeneric(const RangesCollectionTy& Links) {
     assert(Links.size() && "Empty ranges are not allowed.");
+    
+    // In case of big set of single numbers consumes additional RAM space,
+    // but allows to avoid additional reallocation.
+    FlatCollection.reserve(Links.size() * 2);
+    RangeLinks.reserve(Links.size());        
+    IsSingleNumbersOnly = true;
     for (typename RangesCollectionTy::const_iterator i = Links.begin(),
          e = Links.end(); i != e; ++i) {
       RangeLinkTy RangeLink;
       FlatCollection.push_back(i->getLow());
       RangeLink.first = &FlatCollection.back();
-      if (i->getLow() != i->getHigh())
+      if (i->getLow() != i->getHigh()) {
         FlatCollection.push_back(i->getHigh());
+        IsSingleNumbersOnly = false;
+      }
       RangeLink.second = &FlatCollection.back();
       RangeLinks.push_back(RangeLink);
     }
+    IsSingleNumber = IsSingleNumbersOnly && RangeLinks.size() == 1;
   }
   
   IntegersSubsetGeneric(const self& RHS) {
@@ -314,6 +332,8 @@ public:
   self& operator=(const self& RHS) {
     FlatCollection.clear();
     RangeLinks.clear();
+    FlatCollection.reserve(RHS.RangeLinks.size() * 2);
+    RangeLinks.reserve(RHS.RangeLinks.size());
     for (RangeLinksConstIt i = RHS.RangeLinks.begin(), e = RHS.RangeLinks.end();
          i != e; ++i) {
       RangeLinkTy RangeLink;
@@ -324,6 +344,8 @@ public:
       RangeLink.second = &FlatCollection.back();
       RangeLinks.push_back(RangeLink);
     }
+    IsSingleNumber = RHS.IsSingleNumber;
+    IsSingleNumbersOnly = RHS.IsSingleNumbersOnly;
     return *this;
   }
   
@@ -333,6 +355,13 @@ public:
   /// true if it equals to one of contained values or belongs to the one of
   /// contained ranges.
   bool isSatisfies(const IntTy &CheckingVal) const {
+    if (IsSingleNumber)
+      return FlatCollection.front() == CheckingVal;
+    if (IsSingleNumbersOnly)
+      return std::find(FlatCollection.begin(),
+                       FlatCollection.end(),
+                       CheckingVal) != FlatCollection.end();
+    
     for (unsigned i = 0, e = getNumItems(); i < e; ++i) {
       if (RangeLinks[i].first == RangeLinks[i].second) {
         if (*RangeLinks[i].first == CheckingVal)
@@ -360,8 +389,12 @@ public:
   
   /// Returns true if whole subset contains single element.
   bool isSingleNumber() const {
-    return RangeLinks.size() == 1 &&
-           RangeLinks[0].first == RangeLinks[0].second;
+    return IsSingleNumber;
+  }
+  
+  /// Returns true if whole subset contains only single numbers, no ranges.
+  bool isSingleNumbersOnly() const {
+    return IsSingleNumbersOnly;
   }
 
   /// Does the same like getItem(idx).isSingleNumber(), but
@@ -385,7 +418,7 @@ public:
     }
     return sz.getZExtValue();    
   }
-  
+    
   /// Allows to access single value even if it belongs to some range.
   /// Ranges set is considered as flat numbers collection.
   /// [<1>, <4,8>] is considered as [1,4,5,6,7,8] 
@@ -409,6 +442,14 @@ public:
     assert(0 && "Index exceeds high border.");
     return sz;    
   }
+  
+  /// Does the same as getSingleValue, but works only if subset contains
+  /// single numbers only.
+  const IntTy& getSingleNumber(unsigned idx) const {
+    assert(IsSingleNumbersOnly && "This method works properly if subset "
+                                  "contains single numbers only.");
+    return FlatCollection[idx];
+  }
 };  
 
 //===----------------------------------------------------------------------===//
@@ -455,9 +496,9 @@ class IntegersSubset : public IntegersSubsetGeneric<IntItem> {
   }
   
 public:
-  
-  IntegersSubset(Constant *C) : ParentTy(rangesFromConstant(C)),
-                                Holder(C) {}
+
+  explicit IntegersSubset(Constant *C) : ParentTy(rangesFromConstant(C)),
+                          Holder(C) {}
   
   template<class RangesCollectionTy>
   explicit IntegersSubset(const RangesCollectionTy& Src) : ParentTy(Src) {

lib/Transforms/Utils/CodeExtractor.cpp

@@ -664,7 +664,8 @@ emitCallAndSwitchStatement(Function *newFunction, BasicBlock *codeReplacer,
     TheSwitch->setCondition(call);
     TheSwitch->setDefaultDest(TheSwitch->getSuccessor(NumExitBlocks));
     // Remove redundant case
-    TheSwitch->removeCase(SwitchInst::CaseIt(TheSwitch, NumExitBlocks-1));
+    SwitchInst::CaseIt ToBeRemoved(TheSwitch, NumExitBlocks-1);
+    TheSwitch->removeCase(ToBeRemoved);
     break;
   }
 }

lib/VMCore/Instructions.cpp

@@ -3158,6 +3158,7 @@ SwitchInst::SwitchInst(const SwitchInst &SI)
     OL[i] = InOL[i];
     OL[i+1] = InOL[i+1];
   }
+  TheSubsets = SI.TheSubsets;
   SubclassOptionalData = SI.SubclassOptionalData;
 }
 
@@ -3186,14 +3187,17 @@ void SwitchInst::addCase(IntegersSubset& OnVal, BasicBlock *Dest) {
   // Initialize some new operands.
   assert(OpNo+1 < ReservedSpace && "Growing didn't work!");
   NumOperands = OpNo+2;
-  CaseIt Case(this, NewCaseIdx);
-  Case.setValueEx(OnVal);
+
+  SubsetsIt TheSubsetsIt = TheSubsets.insert(TheSubsets.end(), OnVal);
+  
+  CaseIt Case(this, NewCaseIdx, TheSubsetsIt);
+  Case.updateCaseValueOperand(OnVal);
   Case.setSuccessor(Dest);
 }
 
 /// removeCase - This method removes the specified case and its successor
 /// from the switch instruction.
-void SwitchInst::removeCase(CaseIt i) {
+void SwitchInst::removeCase(CaseIt& i) {
   unsigned idx = i.getCaseIndex();
   
   assert(2 + idx*2 < getNumOperands() && "Case index out of range!!!");
@@ -3210,6 +3214,16 @@ void SwitchInst::removeCase(CaseIt i) {
   // Nuke the last value.
   OL[NumOps-2].set(0);
   OL[NumOps-2+1].set(0);
+
+  // Do the same with TheCases collection:
+  if (i.SubsetIt != --TheSubsets.end()) {
+    *i.SubsetIt = TheSubsets.back();
+    TheSubsets.pop_back();
+  } else {
+    TheSubsets.pop_back();
+    i.SubsetIt = TheSubsets.end();
+  }
+  
   NumOperands = NumOps-2;
 }