SwitchInst refactoring.

The purpose of refactoring is to hide operand roles from SwitchInst user (programmer). If you want to play with operands directly, probably you will need lower level methods than SwitchInst ones (TerminatorInst or may be User). After this patch we can reorganize SwitchInst operands and successors as we want.

What was done:

1. Changed semantics of index inside the getCaseValue method:
getCaseValue(0) means "get first case", not a condition. Use getCondition() if you want to resolve the condition. I propose don't mix SwitchInst case indexing with low level indexing (TI successors indexing, User's operands indexing), since it may be dangerous.
2. By the same reason findCaseValue(ConstantInt*) returns actual number of case value. 0 means first case, not default. If there is no case with given value, ErrorIndex will returned.
3. Added getCaseSuccessor method. I propose to avoid usage of TerminatorInst::getSuccessor if you want to resolve case successor BB. Use getCaseSuccessor instead, since internal SwitchInst organization of operands/successors is hidden and may be changed in any moment.
4. Added resolveSuccessorIndex and resolveCaseIndex. The main purpose of these methods is to see how case successors are really mapped in TerminatorInst.
4.1 "resolveSuccessorIndex" was created if you need to level down from SwitchInst to TerminatorInst. It returns TerminatorInst's successor index for given case successor.
4.2 "resolveCaseIndex" converts low level successors index to case index that curresponds to the given successor.

Note: There are also related compatability fix patches for dragonegg, klee, llvm-gcc-4.0, llvm-gcc-4.2, safecode, clang.



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

include/llvm/Analysis/CFGPrinter.h

@@ -95,7 +95,8 @@ struct DOTGraphTraits<const Function*> : public DefaultDOTGraphTraits {
 
       std::string Str;
       raw_string_ostream OS(Str);
-      OS << SI->getCaseValue(SuccNo)->getValue();
+      unsigned Case = SI->resolveCaseIndex(SuccNo);
+      OS << SI->getCaseValue(Case)->getValue();
       return OS.str();
     }    
     return "";

include/llvm/Instructions.h

@@ -24,6 +24,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <iterator>
+#include <limits.h>
 
 namespace llvm {
 
@@ -2467,6 +2468,9 @@ class SwitchInst : public TerminatorInst {
 protected:
   virtual SwitchInst *clone_impl() const;
 public:
+
+  enum { ErrorIndex = UINT_MAX };
+
   static SwitchInst *Create(Value *Value, BasicBlock *Default,
                             unsigned NumCases, Instruction *InsertBefore = 0) {
     return new SwitchInst(Value, Default, NumCases, InsertBefore);
@@ -2488,34 +2492,62 @@ class SwitchInst : public TerminatorInst {
     return cast<BasicBlock>(getOperand(1));
   }
 
-  /// getNumCases - return the number of 'cases' in this switch instruction.
-  /// Note that case #0 is always the default case.
+  void setDefaultDest(BasicBlock *DefaultCase) {
+    setOperand(1, reinterpret_cast<Value*>(DefaultCase));
+  }
+
+  /// getNumCases - return the number of 'cases' in this switch instruction,
+  /// except the default case
   unsigned getNumCases() const {
-    return getNumOperands()/2;
+    return getNumOperands()/2 - 1;
   }
 
-  /// getCaseValue - Return the specified case value.  Note that case #0, the
-  /// default destination, does not have a case value.
+  /// getCaseValue - Return the specified case value. Note that case #0, means
+  /// first case, not a default case.
   ConstantInt *getCaseValue(unsigned i) {
-    assert(i && i < getNumCases() && "Illegal case value to get!");
-    return getSuccessorValue(i);
+    assert(i < getNumCases() && "Illegal case value to get!");
+    return reinterpret_cast<ConstantInt*>(getOperand(2 + i*2));
   }
 
-  /// getCaseValue - Return the specified case value.  Note that case #0, the
-  /// default destination, does not have a case value.
+  /// getCaseValue - Return the specified case value. Note that case #0, means
+  /// first case, not a default case.
   const ConstantInt *getCaseValue(unsigned i) const {
-    assert(i && i < getNumCases() && "Illegal case value to get!");
-    return getSuccessorValue(i);
+    assert(i < getNumCases() && "Illegal case value to get!");
+    return reinterpret_cast<const ConstantInt*>(getOperand(2 + i*2));
+  }
+
+  // setSuccessorValue - Updates the value associated with the specified
+  // case.
+  void setCaseValue(unsigned i, ConstantInt *CaseValue) {
+    assert(i < getNumCases() && "Case index # out of range!");
+    setOperand(2 + i*2, reinterpret_cast<Value*>(CaseValue));
   }
 
   /// findCaseValue - Search all of the case values for the specified constant.
   /// If it is explicitly handled, return the case number of it, otherwise
-  /// return 0 to indicate that it is handled by the default handler.
+  /// return ErrorIndex to indicate that it is handled by the default handler.
   unsigned findCaseValue(const ConstantInt *C) const {
-    for (unsigned i = 1, e = getNumCases(); i != e; ++i)
+    for (unsigned i = 0, e = getNumCases(); i != e; ++i)
       if (getCaseValue(i) == C)
         return i;
-    return 0;
+    return ErrorIndex;
+  }
+
+  /// resolveSuccessorIndex - Converts case index to index of its successor
+  /// index in TerminatorInst successors collection.
+  /// If CaseIndex == ErrorIndex, "default" successor will returned then. 
+  unsigned resolveSuccessorIndex(unsigned CaseIndex) const {
+    assert((CaseIndex == ErrorIndex || CaseIndex < getNumCases()) &&
+           "Case index # out of range!");
+    return CaseIndex != ErrorIndex ? CaseIndex + 1 : 0;
+  }
+
+  /// resolveCaseIndex - Converts index of successor in TerminatorInst
+  /// collection to index of case that corresponds to this successor.
+  unsigned resolveCaseIndex(unsigned SuccessorIndex) const {
+    assert(SuccessorIndex < getNumSuccessors() &&
+           "Successor index # out of range!");    
+    return SuccessorIndex != 0 ? SuccessorIndex - 1 : ErrorIndex; 
   }
 
   /// findCaseDest - Finds the unique case value for a given successor. Returns
@@ -2524,8 +2556,8 @@ class SwitchInst : public TerminatorInst {
     if (BB == getDefaultDest()) return NULL;
 
     ConstantInt *CI = NULL;
-    for (unsigned i = 1, e = getNumCases(); i != e; ++i) {
-      if (getSuccessor(i) == BB) {
+    for (unsigned i = 0, e = getNumCases(); i != e; ++i) {
+      if (getSuccessor(i + 1) == BB) {
         if (CI) return NULL;   // Multiple cases lead to BB.
         else CI = getCaseValue(i);
       }
@@ -2537,9 +2569,8 @@ class SwitchInst : public TerminatorInst {
   ///
   void addCase(ConstantInt *OnVal, BasicBlock *Dest);
 
-  /// removeCase - This method removes the specified successor from the switch
-  /// instruction.  Note that this cannot be used to remove the default
-  /// destination (successor #0). Also note that this operation may reorder the
+  /// removeCase - This method removes the specified case and its successor
+  /// from the switch instruction. Note that this operation may reorder the
   /// remaining cases at index idx and above.
   ///
   void removeCase(unsigned idx);
@@ -2554,6 +2585,22 @@ class SwitchInst : public TerminatorInst {
     setOperand(idx*2+1, (Value*)NewSucc);
   }
 
+  /// Resolves successor for idx-th case.
+  /// Use getCaseSuccessor instead of TerminatorInst::getSuccessor,
+  /// since internal SwitchInst organization of operands/successors is
+  /// hidden and may be changed in any moment.
+  BasicBlock *getCaseSuccessor(unsigned idx) const {
+    return getSuccessor(resolveSuccessorIndex(idx));
+  }
+
+  /// Set new successor for idx-th case.
+  /// Use setCaseSuccessor instead of TerminatorInst::setSuccessor,
+  /// since internal SwitchInst organization of operands/successors is
+  /// hidden and may be changed in any moment.
+  void setCaseSuccessor(unsigned idx, BasicBlock *NewSucc) {
+    setSuccessor(resolveSuccessorIndex(idx), NewSucc);
+  }
+
   // getSuccessorValue - Return the value associated with the specified
   // successor.
   ConstantInt *getSuccessorValue(unsigned idx) const {

lib/Analysis/LazyValueInfo.cpp

@@ -854,8 +854,8 @@ bool LazyValueInfoCache::getEdgeValue(Value *Val, BasicBlock *BBFrom,
       // BBFrom to BBTo.
       unsigned NumEdges = 0;
       ConstantInt *EdgeVal = 0;
-      for (unsigned i = 1, e = SI->getNumSuccessors(); i != e; ++i) {
-        if (SI->getSuccessor(i) != BBTo) continue;
+      for (unsigned i = 0, e = SI->getNumCases(); i != e; ++i) {
+        if (SI->getCaseSuccessor(i) != BBTo) continue;
         if (NumEdges++) break;
         EdgeVal = SI->getCaseValue(i);
       }

lib/Analysis/SparsePropagation.cpp

@@ -195,7 +195,8 @@ void SparseSolver::getFeasibleSuccessors(TerminatorInst &TI,
     return;
   }
 
-  Succs[SI.findCaseValue(cast<ConstantInt>(C))] = true;
+  unsigned CCase = SI.findCaseValue(cast<ConstantInt>(C));
+  Succs[SI.resolveSuccessorIndex(CCase)] = true;
 }
 
 

lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -1162,10 +1162,17 @@ static void WriteInstruction(const Instruction &I, unsigned InstID,
     }
     break;
   case Instruction::Switch:
-    Code = bitc::FUNC_CODE_INST_SWITCH;
-    Vals.push_back(VE.getTypeID(I.getOperand(0)->getType()));
-    for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i)
-      Vals.push_back(VE.getValueID(I.getOperand(i)));
+    {
+      Code = bitc::FUNC_CODE_INST_SWITCH;
+      SwitchInst &SI = cast<SwitchInst>(I);
+      Vals.push_back(VE.getTypeID(SI.getCondition()->getType()));
+      Vals.push_back(VE.getValueID(SI.getCondition()));
+      Vals.push_back(VE.getValueID(SI.getDefaultDest()));
+      for (unsigned i = 0, e = SI.getNumCases(); i != e; ++i) {
+        Vals.push_back(VE.getValueID(SI.getCaseValue(i)));
+        Vals.push_back(VE.getValueID(SI.getCaseSuccessor(i)));
+      }
+    }
     break;
   case Instruction::IndirectBr:
     Code = bitc::FUNC_CODE_INST_INDIRECTBR;

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

@@ -2209,7 +2209,7 @@ bool SelectionDAGBuilder::handleBTSplitSwitchCase(CaseRec& CR,
 
   CaseRange LHSR(CR.Range.first, Pivot);
   CaseRange RHSR(Pivot, CR.Range.second);
-  Constant *C = Pivot->Low;
+  const Constant *C = Pivot->Low;
   MachineBasicBlock *FalseBB = 0, *TrueBB = 0;
 
   // We know that we branch to the LHS if the Value being switched on is
@@ -2402,14 +2402,14 @@ size_t SelectionDAGBuilder::Clusterify(CaseVector& Cases,
 
   BranchProbabilityInfo *BPI = FuncInfo.BPI;
   // Start with "simple" cases
-  for (size_t i = 1; i < SI.getNumSuccessors(); ++i) {
-    BasicBlock *SuccBB = SI.getSuccessor(i);
+  for (size_t i = 0; i < SI.getNumCases(); ++i) {
+    BasicBlock *SuccBB = SI.getCaseSuccessor(i);
     MachineBasicBlock *SMBB = FuncInfo.MBBMap[SuccBB];
 
     uint32_t ExtraWeight = BPI ? BPI->getEdgeWeight(SI.getParent(), SuccBB) : 0;
 
-    Cases.push_back(Case(SI.getSuccessorValue(i),
-                         SI.getSuccessorValue(i),
+    Cases.push_back(Case(SI.getCaseValue(i),
+                         SI.getCaseValue(i),
                          SMBB, ExtraWeight));
   }
   std::sort(Cases.begin(), Cases.end(), CaseCmp());
@@ -2476,7 +2476,7 @@ void SelectionDAGBuilder::visitSwitch(const SwitchInst &SI) {
 
   // If there is only the default destination, branch to it if it is not the
   // next basic block.  Otherwise, just fall through.
-  if (SI.getNumCases() == 1) {
+  if (!SI.getNumCases()) {
     // Update machine-CFG edges.
 
     // If this is not a fall-through branch, emit the branch.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

@@ -130,13 +130,13 @@ class SelectionDAGBuilder {
   /// Case - A struct to record the Value for a switch case, and the
   /// case's target basic block.
   struct Case {
-    Constant* Low;
-    Constant* High;
+    const Constant *Low;
+    const Constant *High;
     MachineBasicBlock* BB;
     uint32_t ExtraWeight;
 
     Case() : Low(0), High(0), BB(0), ExtraWeight(0) { }
-    Case(Constant* low, Constant* high, MachineBasicBlock* bb,
+    Case(const Constant *low, const Constant *high, MachineBasicBlock *bb,
          uint32_t extraweight) : Low(low), High(high), BB(bb),
          ExtraWeight(extraweight) { }
 

lib/ExecutionEngine/Interpreter/Execution.cpp

@@ -670,10 +670,10 @@ void Interpreter::visitSwitchInst(SwitchInst &I) {
   BasicBlock *Dest = 0;
   unsigned NumCases = I.getNumCases();
   // Skip the first item since that's the default case.
-  for (unsigned i = 1; i < NumCases; ++i) {
+  for (unsigned i = 0; i < NumCases; ++i) {
     GenericValue CaseVal = getOperandValue(I.getCaseValue(i), SF);
     if (executeICMP_EQ(CondVal, CaseVal, ElTy).IntVal != 0) {
-      Dest = cast<BasicBlock>(I.getSuccessor(i));
+      Dest = cast<BasicBlock>(I.getCaseSuccessor(i));
       break;
     }
   }

lib/Target/CBackend/CBackend.cpp

@@ -2449,9 +2449,9 @@ void CWriter::visitSwitchInst(SwitchInst &SI) {
 
   unsigned NumCases = SI.getNumCases();
   // Skip the first item since that's the default case.
-  for (unsigned i = 1; i < NumCases; ++i) {
+  for (unsigned i = 0; i < NumCases; ++i) {
     ConstantInt* CaseVal = SI.getCaseValue(i);
-    BasicBlock* Succ = SI.getSuccessor(i);
+    BasicBlock* Succ = SI.getCaseSuccessor(i);
     Out << "  case ";
     writeOperand(CaseVal);
     Out << ":\n";

lib/Target/CppBackend/CPPBackend.cpp

@@ -1115,9 +1115,9 @@ void CppWriter::printInstruction(const Instruction *I,
         << SI->getNumCases() << ", " << bbname << ");";
     nl(Out);
     unsigned NumCases = SI->getNumCases();
-    for (unsigned i = 1; i < NumCases; ++i) {
+    for (unsigned i = 0; i < NumCases; ++i) {
       const ConstantInt* CaseVal = SI->getCaseValue(i);
-      const BasicBlock* BB = SI->getSuccessor(i);
+      const BasicBlock *BB = SI->getCaseSuccessor(i);
       Out << iName << "->addCase("
           << getOpName(CaseVal) << ", "
           << getOpName(BB) << ");";

lib/Transforms/IPO/GlobalOpt.cpp

@@ -2455,7 +2455,8 @@ static bool EvaluateFunction(Function *F, Constant *&RetVal,
         ConstantInt *Val =
           dyn_cast<ConstantInt>(getVal(Values, SI->getCondition()));
         if (!Val) return false;  // Cannot determine.
-        NewBB = SI->getSuccessor(SI->findCaseValue(Val));
+        unsigned ValTISucc = SI->resolveSuccessorIndex(SI->findCaseValue(Val));
+        NewBB = SI->getSuccessor(ValTISucc);
       } else if (IndirectBrInst *IBI = dyn_cast<IndirectBrInst>(CurInst)) {
         Value *Val = getVal(Values, IBI->getAddress())->stripPointerCasts();
         if (BlockAddress *BA = dyn_cast<BlockAddress>(Val))

lib/Transforms/InstCombine/InstructionCombining.cpp

@@ -1251,13 +1251,13 @@ Instruction *InstCombiner::visitSwitchInst(SwitchInst &SI) {
         // change 'switch (X+4) case 1:' into 'switch (X) case -3'
         unsigned NumCases = SI.getNumCases();
         // Skip the first item since that's the default case.
-        for (unsigned i = 1; i < NumCases; ++i) {
+        for (unsigned i = 0; i < NumCases; ++i) {
           ConstantInt* CaseVal = SI.getCaseValue(i);
           Constant* NewCaseVal = ConstantExpr::getSub(cast<Constant>(CaseVal),
                                                       AddRHS);
           assert(isa<ConstantInt>(NewCaseVal) &&
                  "Result of expression should be constant");
-          SI.setSuccessorValue(i, cast<ConstantInt>(NewCaseVal));
+          SI.setCaseValue(i, cast<ConstantInt>(NewCaseVal));
         }
         SI.setCondition(I->getOperand(0));
         Worklist.Add(I);
@@ -1877,15 +1877,15 @@ static bool AddReachableCodeToWorklist(BasicBlock *BB,
     } else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
       if (ConstantInt *Cond = dyn_cast<ConstantInt>(SI->getCondition())) {
         // See if this is an explicit destination.
-        for (unsigned i = 1, e = SI->getNumSuccessors(); i != e; ++i)
+        for (unsigned i = 0, e = SI->getNumCases(); i != e; ++i)
           if (SI->getCaseValue(i) == Cond) {
-            BasicBlock *ReachableBB = SI->getSuccessor(i);
+            BasicBlock *ReachableBB = SI->getCaseSuccessor(i);
             Worklist.push_back(ReachableBB);
             continue;
           }
 
         // Otherwise it is the default destination.
-        Worklist.push_back(SI->getSuccessor(0));
+        Worklist.push_back(SI->getDefaultDest());
         continue;
       }
     }

lib/Transforms/Scalar/GVN.cpp

@@ -2085,8 +2085,8 @@ bool GVN::processInstruction(Instruction *I) {
     Value *SwitchCond = SI->getCondition();
     BasicBlock *Parent = SI->getParent();
     bool Changed = false;
-    for (unsigned i = 1, e = SI->getNumCases(); i != e; ++i) {
-      BasicBlock *Dst = SI->getSuccessor(i);
+    for (unsigned i = 0, e = SI->getNumCases(); i != e; ++i) {
+      BasicBlock *Dst = SI->getCaseSuccessor(i);
       if (isOnlyReachableViaThisEdge(Parent, Dst, DT))
         Changed |= propagateEquality(SwitchCond, SI->getCaseValue(i), Dst);
     }

lib/Transforms/Scalar/JumpThreading.cpp

@@ -1086,9 +1086,10 @@ bool JumpThreading::ProcessThreadableEdges(Value *Cond, BasicBlock *BB,
       DestBB = 0;
     else if (BranchInst *BI = dyn_cast<BranchInst>(BB->getTerminator()))
       DestBB = BI->getSuccessor(cast<ConstantInt>(Val)->isZero());
-    else if (SwitchInst *SI = dyn_cast<SwitchInst>(BB->getTerminator()))
-      DestBB = SI->getSuccessor(SI->findCaseValue(cast<ConstantInt>(Val)));
-    else {
+    else if (SwitchInst *SI = dyn_cast<SwitchInst>(BB->getTerminator())) {
+      unsigned ValCase = SI->findCaseValue(cast<ConstantInt>(Val));
+      DestBB = SI->getSuccessor(SI->resolveSuccessorIndex(ValCase));
+    } else {
       assert(isa<IndirectBrInst>(BB->getTerminator())
               && "Unexpected terminator");
       DestBB = cast<BlockAddress>(Val)->getBasicBlock();