Revert r230921, "Revert some changes that were made to fix PR20680.", for now.
[oota-llvm.git] / lib / Transforms / Scalar / RewriteStatepointsForGC.cpp
index baa49e3472c8effd622e139239cb2c839abdb807..15f09a5fe4aea76a68745bc6eb31d115255cdba0 100644 (file)
@@ -95,8 +95,8 @@ namespace {
 // Generally, after the execution of a full findBasePointer call, only the
 // base relation will remain.  Internally, we add a mixture of the two
 // types, then update all the second type to the first type
-typedef std::map<Value *, Value *> DefiningValueMapTy;
-typedef std::set<llvm::Value *> StatepointLiveSetTy;
+typedef DenseMap<Value *, Value *> DefiningValueMapTy;
+typedef DenseSet<llvm::Value *> StatepointLiveSetTy;
 
 struct PartiallyConstructedSafepointRecord {
   /// The set of values known to be live accross this safepoint
@@ -150,17 +150,16 @@ static bool isLiveGCReferenceAt(Value &V, Instruction *loc, DominatorTree &DT,
 static bool isAggWhichContainsGCPtrType(Type *Ty) {
   if (VectorType *VT = dyn_cast<VectorType>(Ty))
     return isGCPointerType(VT->getScalarType());
-  else if (ArrayType *AT = dyn_cast<ArrayType>(Ty)) {
+  if (ArrayType *AT = dyn_cast<ArrayType>(Ty))
     return isGCPointerType(AT->getElementType()) ||
            isAggWhichContainsGCPtrType(AT->getElementType());
-  } else if (StructType *ST = dyn_cast<StructType>(Ty)) {
-    bool UnsupportedType = false;
-    for (Type *SubType : ST->subtypes())
-      UnsupportedType |=
-          isGCPointerType(SubType) || isAggWhichContainsGCPtrType(SubType);
-    return UnsupportedType;
-  } else
-    return false;
+  if (StructType *ST = dyn_cast<StructType>(Ty))
+    return std::any_of(ST->subtypes().begin(), ST->subtypes().end(),
+                       [](Type *SubType) {
+                         return isGCPointerType(SubType) ||
+                                isAggWhichContainsGCPtrType(SubType);
+                       });
+  return false;
 }
 #endif
 
@@ -177,7 +176,7 @@ static bool isAggWhichContainsGCPtrType(Type *Ty) {
 //  postconditions: populates liveValues as discussed above
 static void findLiveGCValuesAtInst(Instruction *term, BasicBlock *pred,
                                    DominatorTree &DT, LoopInfo *LI,
-                                   std::set<llvm::Value *> &liveValues) {
+                                   StatepointLiveSetTy &liveValues) {
   liveValues.clear();
 
   assert(isa<CallInst>(term) || isa<InvokeInst>(term) || term->isTerminator());
@@ -277,7 +276,7 @@ analyzeParsePointLiveness(DominatorTree &DT, const CallSite &CS,
   Instruction *inst = CS.getInstruction();
 
   BasicBlock *BB = inst->getParent();
-  std::set<Value *> liveset;
+  StatepointLiveSetTy liveset;
   findLiveGCValuesAtInst(inst, BB, DT, nullptr, liveset);
 
   if (PrintLiveSet) {
@@ -301,7 +300,7 @@ analyzeParsePointLiveness(DominatorTree &DT, const CallSite &CS,
 }
 
 /// True iff this value is the null pointer constant (of any pointer type)
-static bool isNullConstant(Value *V) {
+static bool LLVM_ATTRIBUTE_UNUSED isNullConstant(Value *V) {
   return isa<Constant>(V) && isa<PointerType>(V->getType()) &&
          cast<Constant>(V)->isNullValue();
 }
@@ -379,12 +378,9 @@ static Value *findBaseDefiningValue(Value *I) {
     Value *def = CI->stripPointerCasts();
     assert(def->getType()->isPointerTy() &&
            "Base for pointer must be another pointer");
-    if (isa<CastInst>(def)) {
-      // If we find a cast instruction here, it means we've found a cast
-      // which is not simply a pointer cast (i.e. an inttoptr).  We don't
-      // know how to handle int->ptr conversion.
-      llvm_unreachable("Can not find the base pointers for an inttoptr cast");
-    }
+    // If we find a cast instruction here, it means we've found a cast which is
+    // not simply a pointer cast (i.e. an inttoptr).  We don't know how to
+    // handle int->ptr conversion.
     assert(!isa<CastInst>(def) && "shouldn't find another cast here");
     return findBaseDefiningValue(def);
   }
@@ -492,13 +488,8 @@ static Value *findBaseDefiningValue(Value *I) {
   if (SelectInst *select = dyn_cast<SelectInst>(I)) {
     return select;
   }
-  if (PHINode *phi = dyn_cast<PHINode>(I)) {
-    return phi;
-  }
 
-  errs() << "unknown type: " << *I << "\n";
-  llvm_unreachable("unknown type");
-  return nullptr;
+  return cast<PHINode>(I);
 }
 
 /// Returns the base defining value for this value.
@@ -584,7 +575,7 @@ private:
   Value *base; // non null only if status == base
 };
 
-typedef std::map<Value *, PhiState> ConflictStateMapTy;
+typedef DenseMap<Value *, PhiState> ConflictStateMapTy;
 // Values of type PhiState form a lattice, and this is a helper
 // class that implementes the meet operation.  The meat of the meet
 // operation is implemented in MeetPhiStates::pureMeet
@@ -635,18 +626,18 @@ private:
 
     case PhiState::Base:
       assert(stateA.getBase() && "can't be null");
-      if (stateB.isUnknown()) {
+      if (stateB.isUnknown())
         return stateA;
-      } else if (stateB.isBase()) {
+
+      if (stateB.isBase()) {
         if (stateA.getBase() == stateB.getBase()) {
           assert(stateA == stateB && "equality broken!");
           return stateA;
         }
         return PhiState(PhiState::Conflict);
-      } else {
-        assert(stateB.isConflict() && "only three states!");
-        return PhiState(PhiState::Conflict);
       }
+      assert(stateB.isConflict() && "only three states!");
+      return PhiState(PhiState::Conflict);
 
     case PhiState::Conflict:
       return stateA;
@@ -693,18 +684,24 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
   states[def] = PhiState();
   // Recursively fill in all phis & selects reachable from the initial one
   // for which we don't already know a definite base value for
-  // PERF: Yes, this is as horribly inefficient as it looks.
+  // TODO: This should be rewritten with a worklist
   bool done = false;
   while (!done) {
     done = true;
+    // Since we're adding elements to 'states' as we run, we can't keep
+    // iterators into the set.
+    SmallVector<Value*, 16> Keys;
+    Keys.reserve(states.size());
     for (auto Pair : states) {
-      Value *v = Pair.first;
+      Value *V = Pair.first;
+      Keys.push_back(V);
+    }
+    for (Value *v : Keys) {
       assert(!isKnownBaseResult(v) && "why did it get added?");
       if (PHINode *phi = dyn_cast<PHINode>(v)) {
-        unsigned NumPHIValues = phi->getNumIncomingValues();
-        assert(NumPHIValues > 0 && "zero input phis are illegal");
-        for (unsigned i = 0; i != NumPHIValues; ++i) {
-          Value *InVal = phi->getIncomingValue(i);
+        assert(phi->getNumIncomingValues() > 0 &&
+               "zero input phis are illegal");
+        for (Value *InVal : phi->incoming_values()) {
           Value *local = findBaseOrBDV(InVal, cache);
           if (!isKnownBaseResult(local) && states.find(local) == states.end()) {
             states[local] = PhiState();
@@ -740,26 +737,22 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
   // have reached conflict state.  The current version seems too conservative.
 
   bool progress = true;
-  size_t oldSize = 0;
   while (progress) {
-    oldSize = states.size();
+#ifndef NDEBUG
+    size_t oldSize = states.size();
+#endif
     progress = false;
+    // We're only changing keys in this loop, thus safe to keep iterators
     for (auto Pair : states) {
       MeetPhiStates calculateMeet(states);
       Value *v = Pair.first;
       assert(!isKnownBaseResult(v) && "why did it get added?");
-      assert(isa<SelectInst>(v) || isa<PHINode>(v));
       if (SelectInst *select = dyn_cast<SelectInst>(v)) {
         calculateMeet.meetWith(findBaseOrBDV(select->getTrueValue(), cache));
         calculateMeet.meetWith(findBaseOrBDV(select->getFalseValue(), cache));
-      } else if (PHINode *phi = dyn_cast<PHINode>(v)) {
-        for (unsigned i = 0; i < phi->getNumIncomingValues(); i++) {
-          calculateMeet.meetWith(
-              findBaseOrBDV(phi->getIncomingValue(i), cache));
-        }
-      } else {
-        llvm_unreachable("no such state expected");
-      }
+      } else
+        for (Value *Val : cast<PHINode>(v)->incoming_values())
+          calculateMeet.meetWith(findBaseOrBDV(Val, cache));
 
       PhiState oldState = states[v];
       PhiState newState = calculateMeet.getResult();
@@ -784,46 +777,58 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
   }
 
   // Insert Phis for all conflicts
+  // We want to keep naming deterministic in the loop that follows, so
+  // sort the keys before iteration.  This is useful in allowing us to
+  // write stable tests. Note that there is no invalidation issue here.
+  SmallVector<Value*, 16> Keys;
+  Keys.reserve(states.size());
   for (auto Pair : states) {
-    Instruction *v = cast<Instruction>(Pair.first);
-    PhiState state = Pair.second;
+    Value *V = Pair.first;
+    Keys.push_back(V);
+  }
+  std::sort(Keys.begin(), Keys.end(), order_by_name);
+  // TODO: adjust naming patterns to avoid this order of iteration dependency
+  for (Value *V : Keys) {
+    Instruction *v = cast<Instruction>(V);
+    PhiState state = states[V];
     assert(!isKnownBaseResult(v) && "why did it get added?");
     assert(!state.isUnknown() && "Optimistic algorithm didn't complete!");
-    if (state.isConflict()) {
-      if (isa<PHINode>(v)) {
-        int num_preds =
-            std::distance(pred_begin(v->getParent()), pred_end(v->getParent()));
-        assert(num_preds > 0 && "how did we reach here");
-        PHINode *phi = PHINode::Create(v->getType(), num_preds, "base_phi", v);
-        NewInsertedDefs.insert(phi);
-        // Add metadata marking this as a base value
-        auto *const_1 = ConstantInt::get(
-            Type::getInt32Ty(
-                v->getParent()->getParent()->getParent()->getContext()),
-            1);
-        auto MDConst = ConstantAsMetadata::get(const_1);
-        MDNode *md = MDNode::get(
-            v->getParent()->getParent()->getParent()->getContext(), MDConst);
-        phi->setMetadata("is_base_value", md);
-        states[v] = PhiState(PhiState::Conflict, phi);
-      } else if (SelectInst *sel = dyn_cast<SelectInst>(v)) {
-        // The undef will be replaced later
-        UndefValue *undef = UndefValue::get(sel->getType());
-        SelectInst *basesel = SelectInst::Create(sel->getCondition(), undef,
-                                                 undef, "base_select", sel);
-        NewInsertedDefs.insert(basesel);
-        // Add metadata marking this as a base value
-        auto *const_1 = ConstantInt::get(
-            Type::getInt32Ty(
-                v->getParent()->getParent()->getParent()->getContext()),
-            1);
-        auto MDConst = ConstantAsMetadata::get(const_1);
-        MDNode *md = MDNode::get(
-            v->getParent()->getParent()->getParent()->getContext(), MDConst);
-        basesel->setMetadata("is_base_value", md);
-        states[v] = PhiState(PhiState::Conflict, basesel);
-      } else
-        llvm_unreachable("unknown conflict type");
+    if (!state.isConflict())
+      continue;
+    
+    if (isa<PHINode>(v)) {
+      int num_preds =
+          std::distance(pred_begin(v->getParent()), pred_end(v->getParent()));
+      assert(num_preds > 0 && "how did we reach here");
+      PHINode *phi = PHINode::Create(v->getType(), num_preds, "base_phi", v);
+      NewInsertedDefs.insert(phi);
+      // Add metadata marking this as a base value
+      auto *const_1 = ConstantInt::get(
+          Type::getInt32Ty(
+              v->getParent()->getParent()->getParent()->getContext()),
+          1);
+      auto MDConst = ConstantAsMetadata::get(const_1);
+      MDNode *md = MDNode::get(
+          v->getParent()->getParent()->getParent()->getContext(), MDConst);
+      phi->setMetadata("is_base_value", md);
+      states[v] = PhiState(PhiState::Conflict, phi);
+    } else {
+      SelectInst *sel = cast<SelectInst>(v);
+      // The undef will be replaced later
+      UndefValue *undef = UndefValue::get(sel->getType());
+      SelectInst *basesel = SelectInst::Create(sel->getCondition(), undef,
+                                               undef, "base_select", sel);
+      NewInsertedDefs.insert(basesel);
+      // Add metadata marking this as a base value
+      auto *const_1 = ConstantInt::get(
+          Type::getInt32Ty(
+              v->getParent()->getParent()->getParent()->getContext()),
+          1);
+      auto MDConst = ConstantAsMetadata::get(const_1);
+      MDNode *md = MDNode::get(
+          v->getParent()->getParent()->getParent()->getContext(), MDConst);
+      basesel->setMetadata("is_base_value", md);
+      states[v] = PhiState(PhiState::Conflict, basesel);
     }
   }
 
@@ -834,97 +839,98 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
 
     assert(!isKnownBaseResult(v) && "why did it get added?");
     assert(!state.isUnknown() && "Optimistic algorithm didn't complete!");
-    if (state.isConflict()) {
-      if (PHINode *basephi = dyn_cast<PHINode>(state.getBase())) {
-        PHINode *phi = cast<PHINode>(v);
-        unsigned NumPHIValues = phi->getNumIncomingValues();
-        for (unsigned i = 0; i < NumPHIValues; i++) {
-          Value *InVal = phi->getIncomingValue(i);
-          BasicBlock *InBB = phi->getIncomingBlock(i);
-
-          // If we've already seen InBB, add the same incoming value
-          // we added for it earlier.  The IR verifier requires phi
-          // nodes with multiple entries from the same basic block
-          // to have the same incoming value for each of those
-          // entries.  If we don't do this check here and basephi
-          // has a different type than base, we'll end up adding two
-          // bitcasts (and hence two distinct values) as incoming
-          // values for the same basic block.
-
-          int blockIndex = basephi->getBasicBlockIndex(InBB);
-          if (blockIndex != -1) {
-            Value *oldBase = basephi->getIncomingValue(blockIndex);
-            basephi->addIncoming(oldBase, InBB);
+    if (!state.isConflict())
+      continue;
+    
+    if (PHINode *basephi = dyn_cast<PHINode>(state.getBase())) {
+      PHINode *phi = cast<PHINode>(v);
+      unsigned NumPHIValues = phi->getNumIncomingValues();
+      for (unsigned i = 0; i < NumPHIValues; i++) {
+        Value *InVal = phi->getIncomingValue(i);
+        BasicBlock *InBB = phi->getIncomingBlock(i);
+
+        // If we've already seen InBB, add the same incoming value
+        // we added for it earlier.  The IR verifier requires phi
+        // nodes with multiple entries from the same basic block
+        // to have the same incoming value for each of those
+        // entries.  If we don't do this check here and basephi
+        // has a different type than base, we'll end up adding two
+        // bitcasts (and hence two distinct values) as incoming
+        // values for the same basic block.
+
+        int blockIndex = basephi->getBasicBlockIndex(InBB);
+        if (blockIndex != -1) {
+          Value *oldBase = basephi->getIncomingValue(blockIndex);
+          basephi->addIncoming(oldBase, InBB);
 #ifndef NDEBUG
-            Value *base = findBaseOrBDV(InVal, cache);
-            if (!isKnownBaseResult(base)) {
-              // Either conflict or base.
-              assert(states.count(base));
-              base = states[base].getBase();
-              assert(base != nullptr && "unknown PhiState!");
-              assert(NewInsertedDefs.count(base) &&
-                     "should have already added this in a prev. iteration!");
-            }
-
-            // In essense this assert states: the only way two
-            // values incoming from the same basic block may be
-            // different is by being different bitcasts of the same
-            // value.  A cleanup that remains TODO is changing
-            // findBaseOrBDV to return an llvm::Value of the correct
-            // type (and still remain pure).  This will remove the
-            // need to add bitcasts.
-            assert(base->stripPointerCasts() == oldBase->stripPointerCasts() &&
-                   "sanity -- findBaseOrBDV should be pure!");
-#endif
-            continue;
-          }
-
-          // Find either the defining value for the PHI or the normal base for
-          // a non-phi node
           Value *base = findBaseOrBDV(InVal, cache);
           if (!isKnownBaseResult(base)) {
             // Either conflict or base.
             assert(states.count(base));
             base = states[base].getBase();
             assert(base != nullptr && "unknown PhiState!");
+            assert(NewInsertedDefs.count(base) &&
+                   "should have already added this in a prev. iteration!");
           }
-          assert(base && "can't be null");
-          // Must use original input BB since base may not be Instruction
-          // The cast is needed since base traversal may strip away bitcasts
-          if (base->getType() != basephi->getType()) {
-            base = new BitCastInst(base, basephi->getType(), "cast",
-                                   InBB->getTerminator());
-            NewInsertedDefs.insert(base);
-          }
-          basephi->addIncoming(base, InBB);
+
+          // In essense this assert states: the only way two
+          // values incoming from the same basic block may be
+          // different is by being different bitcasts of the same
+          // value.  A cleanup that remains TODO is changing
+          // findBaseOrBDV to return an llvm::Value of the correct
+          // type (and still remain pure).  This will remove the
+          // need to add bitcasts.
+          assert(base->stripPointerCasts() == oldBase->stripPointerCasts() &&
+                 "sanity -- findBaseOrBDV should be pure!");
+#endif
+          continue;
         }
-        assert(basephi->getNumIncomingValues() == NumPHIValues);
-      } else if (SelectInst *basesel = dyn_cast<SelectInst>(state.getBase())) {
-        SelectInst *sel = cast<SelectInst>(v);
-        // Operand 1 & 2 are true, false path respectively. TODO: refactor to
-        // something more safe and less hacky.
-        for (int i = 1; i <= 2; i++) {
-          Value *InVal = sel->getOperand(i);
-          // Find either the defining value for the PHI or the normal base for
-          // a non-phi node
-          Value *base = findBaseOrBDV(InVal, cache);
-          if (!isKnownBaseResult(base)) {
-            // Either conflict or base.
-            assert(states.count(base));
-            base = states[base].getBase();
-            assert(base != nullptr && "unknown PhiState!");
-          }
-          assert(base && "can't be null");
-          // Must use original input BB since base may not be Instruction
-          // The cast is needed since base traversal may strip away bitcasts
-          if (base->getType() != basesel->getType()) {
-            base = new BitCastInst(base, basesel->getType(), "cast", basesel);
-            NewInsertedDefs.insert(base);
-          }
-          basesel->setOperand(i, base);
+
+        // Find either the defining value for the PHI or the normal base for
+        // a non-phi node
+        Value *base = findBaseOrBDV(InVal, cache);
+        if (!isKnownBaseResult(base)) {
+          // Either conflict or base.
+          assert(states.count(base));
+          base = states[base].getBase();
+          assert(base != nullptr && "unknown PhiState!");
         }
-      } else
-        llvm_unreachable("unexpected conflict type");
+        assert(base && "can't be null");
+        // Must use original input BB since base may not be Instruction
+        // The cast is needed since base traversal may strip away bitcasts
+        if (base->getType() != basephi->getType()) {
+          base = new BitCastInst(base, basephi->getType(), "cast",
+                                 InBB->getTerminator());
+          NewInsertedDefs.insert(base);
+        }
+        basephi->addIncoming(base, InBB);
+      }
+      assert(basephi->getNumIncomingValues() == NumPHIValues);
+    } else {
+      SelectInst *basesel = cast<SelectInst>(state.getBase());
+      SelectInst *sel = cast<SelectInst>(v);
+      // Operand 1 & 2 are true, false path respectively. TODO: refactor to
+      // something more safe and less hacky.
+      for (int i = 1; i <= 2; i++) {
+        Value *InVal = sel->getOperand(i);
+        // Find either the defining value for the PHI or the normal base for
+        // a non-phi node
+        Value *base = findBaseOrBDV(InVal, cache);
+        if (!isKnownBaseResult(base)) {
+          // Either conflict or base.
+          assert(states.count(base));
+          base = states[base].getBase();
+          assert(base != nullptr && "unknown PhiState!");
+        }
+        assert(base && "can't be null");
+        // Must use original input BB since base may not be Instruction
+        // The cast is needed since base traversal may strip away bitcasts
+        if (base->getType() != basesel->getType()) {
+          base = new BitCastInst(base, basesel->getType(), "cast", basesel);
+          NewInsertedDefs.insert(base);
+        }
+        basesel->setOperand(i, base);
+      }
     }
   }
 
@@ -976,11 +982,17 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
 // post condition: PointerToBase contains one (derived, base) pair for every
 // pointer in live.  Note that derived can be equal to base if the original
 // pointer was a base pointer.
-static void findBasePointers(const std::set<llvm::Value *> &live,
+static void findBasePointers(const StatepointLiveSetTy &live,
                              DenseMap<llvm::Value *, llvm::Value *> &PointerToBase,
                              DominatorTree *DT, DefiningValueMapTy &DVCache,
                              DenseSet<llvm::Value *> &NewInsertedDefs) {
-  for (Value *ptr : live) {
+  // For the naming of values inserted to be deterministic - which makes for
+  // much cleaner and more stable tests - we need to assign an order to the
+  // live values.  DenseSets do not provide a deterministic order across runs.
+  SmallVector<Value*, 64> Temp;
+  Temp.insert(Temp.end(), live.begin(), live.end());
+  std::sort(Temp.begin(), Temp.end(), order_by_name);
+  for (Value *ptr : Temp) {
     Value *base = findBasePointer(ptr, DVCache, NewInsertedDefs);
     assert(base && "failed to find base pointer");
     PointerToBase[ptr] = base;
@@ -989,13 +1001,13 @@ static void findBasePointers(const std::set<llvm::Value *> &live,
                           cast<Instruction>(ptr)->getParent())) &&
            "The base we found better dominate the derived pointer");
 
-    if (isNullConstant(base))
-      // If you see this trip and like to live really dangerously, the code
-      // should be correct, just with idioms the verifier can't handle.  You
-      // can try disabling the verifier at your own substaintial risk.
-      llvm_unreachable("the relocation code needs adjustment to handle the"
-                       "relocation of a null pointer constant without causing"
-                       "false positives in the safepoint ir verifier.");
+    // If you see this trip and like to live really dangerously, the code should
+    // be correct, just with idioms the verifier can't handle.  You can try
+    // disabling the verifier at your own substaintial risk.
+    assert(!isNullConstant(base) && "the relocation code needs adjustment to "
+                                    "handle the relocation of a null pointer "
+                                    "constant without causing false positives "
+                                    "in the safepoint ir verifier.");
   }
 }
 
@@ -1009,10 +1021,19 @@ static void findBasePointers(DominatorTree &DT, DefiningValueMapTy &DVCache,
   findBasePointers(result.liveset, PointerToBase, &DT, DVCache, NewInsertedDefs);
 
   if (PrintBasePointers) {
+    // Note: Need to print these in a stable order since this is checked in
+    // some tests.
     errs() << "Base Pairs (w/o Relocation):\n";
+    SmallVector<Value*, 64> Temp;
+    Temp.reserve(PointerToBase.size());
     for (auto Pair : PointerToBase) {
-      errs() << " derived %" << Pair.first->getName() << " base %"
-             << Pair.second->getName() << "\n";
+      Temp.push_back(Pair.first);
+    }
+    std::sort(Temp.begin(), Temp.end(), order_by_name);
+    for (Value *Ptr : Temp) {
+      Value *Base = PointerToBase[Ptr];
+      errs() << " derived %" << Ptr->getName() << " base %"
+             << Base->getName() << "\n";
     }
   }
 
@@ -1023,7 +1044,7 @@ static void findBasePointers(DominatorTree &DT, DefiningValueMapTy &DVCache,
 /// Check for liveness of items in the insert defs and add them to the live
 /// and base pointer sets
 static void fixupLiveness(DominatorTree &DT, const CallSite &CS,
-                          const std::set<Value *> &allInsertedDefs,
+                          const DenseSet<Value *> &allInsertedDefs,
                           PartiallyConstructedSafepointRecord &result) {
   Instruction *inst = CS.getInstruction();
 
@@ -1064,12 +1085,12 @@ static void fixupLiveness(DominatorTree &DT, const CallSite &CS,
 
 static void fixupLiveReferences(
     Function &F, DominatorTree &DT, Pass *P,
-    const std::set<llvm::Value *> &allInsertedDefs,
-    std::vector<CallSite> &toUpdate,
-    std::vector<struct PartiallyConstructedSafepointRecord> &records) {
+    const DenseSet<llvm::Value *> &allInsertedDefs,
+    ArrayRef<CallSite> toUpdate,
+    MutableArrayRef<struct PartiallyConstructedSafepointRecord> records) {
   for (size_t i = 0; i < records.size(); i++) {
     struct PartiallyConstructedSafepointRecord &info = records[i];
-    CallSite &CS = toUpdate[i];
+    const CallSite &CS = toUpdate[i];
     fixupLiveness(DT, CS, allInsertedDefs, info);
   }
 }
@@ -1097,7 +1118,7 @@ static BasicBlock *normalizeBBForInvokeSafepoint(BasicBlock *BB,
   return ret;
 }
 
-static int find_index(const SmallVectorImpl<Value *> &livevec, Value *val) {
+static int find_index(ArrayRef<Value *> livevec, Value *val) {
   auto itr = std::find(livevec.begin(), livevec.end(), val);
   assert(livevec.end() != itr);
   size_t index = std::distance(livevec.begin(), itr);
@@ -1147,14 +1168,13 @@ static AttributeSet legalizeCallAttributes(AttributeSet AS) {
 ///   statepointToken - statepoint instruction to which relocates should be
 ///   bound.
 ///   Builder - Llvm IR builder to be used to construct new calls.
-/// Returns array with newly created relocates.
-static std::vector<llvm::Instruction *>
-CreateGCRelocates(const SmallVectorImpl<llvm::Value *> &liveVariables,
-                  const int liveStart,
-                  const SmallVectorImpl<llvm::Value *> &basePtrs,
-                  Instruction *statepointToken, IRBuilder<> Builder) {
+void CreateGCRelocates(ArrayRef<llvm::Value *> liveVariables,
+                       const int liveStart,
+                       ArrayRef<llvm::Value *> basePtrs,
+                       Instruction *statepointToken, IRBuilder<> Builder) {
 
-  std::vector<llvm::Instruction *> newDefs;
+  SmallVector<Instruction *, 64> NewDefs;
+  NewDefs.reserve(liveVariables.size());
 
   Module *M = statepointToken->getParent()->getParent()->getParent();
 
@@ -1163,7 +1183,7 @@ CreateGCRelocates(const SmallVectorImpl<llvm::Value *> &liveVariables,
     // combination.  This results is some blow up the function declarations in
     // the IR, but removes the need for argument bitcasts which shrinks the IR
     // greatly and makes it much more readable.
-    std::vector<Type *> types;                    // one per 'any' type
+    SmallVector<Type *, 1> types;                    // one per 'any' type
     types.push_back(liveVariables[i]->getType()); // result type
     Value *gc_relocate_decl = Intrinsic::getDeclaration(
         M, Intrinsic::experimental_gc_relocate, types);
@@ -1185,12 +1205,10 @@ CreateGCRelocates(const SmallVectorImpl<llvm::Value *> &liveVariables,
     // fake call.
     cast<CallInst>(reloc)->setCallingConv(CallingConv::Cold);
 
-    newDefs.push_back(cast<Instruction>(reloc));
+    NewDefs.push_back(cast<Instruction>(reloc));
   }
-  assert(newDefs.size() == liveVariables.size() &&
+  assert(NewDefs.size() == liveVariables.size() &&
          "missing or extra redefinition at safepoint");
-
-  return newDefs;
 }
 
 static void
@@ -1223,7 +1241,7 @@ makeStatepointExplicitImpl(const CallSite &CS, /* to replace */
   IRBuilder<> Builder(insertBefore);
   // Copy all of the arguments from the original statepoint - this includes the
   // target, call args, and deopt args
-  std::vector<llvm::Value *> args;
+  SmallVector<llvm::Value *, 64> args;
   args.insert(args.end(), CS.arg_begin(), CS.arg_end());
   // TODO: Clear the 'needs rewrite' flag
 
@@ -1261,7 +1279,7 @@ makeStatepointExplicitImpl(const CallSite &CS, /* to replace */
     Builder.SetInsertPoint(IP);
     Builder.SetCurrentDebugLocation(IP->getDebugLoc());
 
-  } else if (CS.isInvoke()) {
+  } else {
     InvokeInst *toReplace = cast<InvokeInst>(CS.getInstruction());
 
     // Insert the new invoke into the old block.  We'll remove the old one in a
@@ -1312,8 +1330,6 @@ makeStatepointExplicitImpl(const CallSite &CS, /* to replace */
 
     // gc relocates will be generated later as if it were regular call
     // statepoint
-  } else {
-    llvm_unreachable("unexpect type of CallSite");
   }
   assert(token);
 
@@ -1321,15 +1337,13 @@ makeStatepointExplicitImpl(const CallSite &CS, /* to replace */
   token->takeName(CS.getInstruction());
 
   // The GCResult is already inserted, we just need to find it
-  /* scope */ {
-    Instruction *toReplace = CS.getInstruction();
-    assert((toReplace->hasNUses(0) || toReplace->hasNUses(1)) &&
-           "only valid use before rewrite is gc.result");
-    if (toReplace->hasOneUse()) {
-      Instruction *GCResult = cast<Instruction>(*toReplace->user_begin());
-      assert(isGCResult(GCResult));
-    }
-  }
+#ifndef NDEBUG
+  Instruction *toReplace = CS.getInstruction();
+  assert((toReplace->hasNUses(0) || toReplace->hasNUses(1)) &&
+         "only valid use before rewrite is gc.result");
+  assert(!toReplace->hasOneUse() ||
+         isGCResult(cast<Instruction>(*toReplace->user_begin())));
+#endif
 
   // Update the gc.result of the original statepoint (if any) to use the newly
   // inserted statepoint.  This is safe to do here since the token can't be
@@ -1444,8 +1458,8 @@ insertRelocationStores(iterator_range<Value::user_iterator> gcRelocs,
 
 /// do all the relocation update via allocas and mem2reg
 static void relocationViaAlloca(
-    Function &F, DominatorTree &DT, const std::vector<Value *> &live,
-    const std::vector<struct PartiallyConstructedSafepointRecord> &records) {
+    Function &F, DominatorTree &DT, ArrayRef<Value *> live,
+    ArrayRef<struct PartiallyConstructedSafepointRecord> records) {
 #ifndef NDEBUG
   int initialAllocaNum = 0;
 
@@ -1529,12 +1543,11 @@ static void relocationViaAlloca(
     if (auto II = dyn_cast<InvokeInst>(Statepoint)) {
       InsertClobbersAt(II->getNormalDest()->getFirstInsertionPt());
       InsertClobbersAt(II->getUnwindDest()->getFirstInsertionPt());
-    } else if (auto CI = dyn_cast<CallInst>(Statepoint)) {
-      BasicBlock::iterator Next(CI);
+    } else {
+      BasicBlock::iterator Next(cast<CallInst>(Statepoint));
       Next++;
       InsertClobbersAt(Next);
-    } else
-      llvm_unreachable("illegal statepoint instruction type?");
+    }
 #endif
   }
   // update use with load allocas and add store for gc_relocated
@@ -1613,14 +1626,16 @@ static void relocationViaAlloca(
 /// Implement a unique function which doesn't require we sort the input
 /// vector.  Doing so has the effect of changing the output of a couple of
 /// tests in ways which make them less useful in testing fused safepoints.
-template <typename T> static void unique_unsorted(std::vector<T> &vec) {
-  DenseSet<T> seen;
-  std::vector<T> tmp;
-  vec.reserve(vec.size());
-  std::swap(tmp, vec);
-  for (auto V : tmp) {
-    if (seen.insert(V).second) {
-      vec.push_back(V);
+template <typename T> static void unique_unsorted(SmallVectorImpl<T> &Vec) {
+  DenseSet<T> Seen;
+  SmallVector<T, 128> TempVec;
+  TempVec.reserve(Vec.size());
+  for (auto Element : Vec)
+    TempVec.push_back(Element);
+  Vec.clear();
+  for (auto V : TempVec) {
+    if (Seen.insert(V).second) {
+      Vec.push_back(V);
     }
   }
 }
@@ -1635,7 +1650,7 @@ static Function *getUseHolder(Module &M) {
 /// Insert holders so that each Value is obviously live through the entire
 /// liftetime of the call.
 static void insertUseHolderAfter(CallSite &CS, const ArrayRef<Value *> Values,
-                                 std::vector<CallInst *> &holders) {
+                                 SmallVectorImpl<CallInst *> &holders) {
   Module *M = CS.getInstruction()->getParent()->getParent()->getParent();
   Function *Func = getUseHolder(*M);
   if (CS.isCall()) {
@@ -1659,16 +1674,16 @@ static void insertUseHolderAfter(CallSite &CS, const ArrayRef<Value *> Values,
 }
 
 static void findLiveReferences(
-    Function &F, DominatorTree &DT, Pass *P, std::vector<CallSite> &toUpdate,
-    std::vector<struct PartiallyConstructedSafepointRecord> &records) {
+    Function &F, DominatorTree &DT, Pass *P, ArrayRef<CallSite> toUpdate,
+    MutableArrayRef<struct PartiallyConstructedSafepointRecord> records) {
   for (size_t i = 0; i < records.size(); i++) {
     struct PartiallyConstructedSafepointRecord &info = records[i];
-    CallSite &CS = toUpdate[i];
+    const CallSite &CS = toUpdate[i];
     analyzeParsePointLiveness(DT, CS, info);
   }
 }
 
-static void addBasesAsLiveValues(std::set<Value *> &liveset,
+static void addBasesAsLiveValues(StatepointLiveSetTy &liveset,
                                  DenseMap<Value *, Value *> &PointerToBase) {
   // Identify any base pointers which are used in this safepoint, but not
   // themselves relocated.  We need to relocate them so that later inserted
@@ -1698,7 +1713,7 @@ static void addBasesAsLiveValues(std::set<Value *> &liveset,
 }
 
 static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P,
-                              std::vector<CallSite> &toUpdate) {
+                              SmallVectorImpl<CallSite> &toUpdate) {
 #ifndef NDEBUG
   // sanity check the input
   std::set<CallSite> uniqued;
@@ -1714,7 +1729,7 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P,
 
   // A list of dummy calls added to the IR to keep various values obviously
   // live in the IR.  We'll remove all of these when done.
-  std::vector<CallInst *> holders;
+  SmallVector<CallInst *, 64> holders;
 
   // Insert a dummy call with all of the arguments to the vm_state we'll need
   // for the actual safepoint insertion.  This ensures reference arguments in
@@ -1733,7 +1748,7 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P,
     insertUseHolderAfter(CS, DeoptValues, holders);
   }
 
-  std::vector<struct PartiallyConstructedSafepointRecord> records;
+  SmallVector<struct PartiallyConstructedSafepointRecord, 64> records;
   records.reserve(toUpdate.size());
   for (size_t i = 0; i < toUpdate.size(); i++) {
     struct PartiallyConstructedSafepointRecord info;
@@ -1768,7 +1783,7 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P,
   //   gep a + 1
   //   safepoint 2
   //   br loop
-  std::set<llvm::Value *> allInsertedDefs;
+  DenseSet<llvm::Value *> allInsertedDefs;
   for (size_t i = 0; i < records.size(); i++) {
     struct PartiallyConstructedSafepointRecord &info = records[i];
     allInsertedDefs.insert(info.NewInsertedDefs.begin(),
@@ -1856,7 +1871,7 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P,
   }
 
   // Do all the fixups of the original live variables to their relocated selves
-  std::vector<Value *> live;
+  SmallVector<Value *, 128> live;
   for (size_t i = 0; i < records.size(); i++) {
     struct PartiallyConstructedSafepointRecord &info = records[i];
     // We can't simply save the live set from the original insertion.  One of
@@ -1903,12 +1918,11 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F) {
     return false;
 
   // Gather all the statepoints which need rewritten.
-  std::vector<CallSite> ParsePointNeeded;
-  for (inst_iterator itr = inst_begin(F), end = inst_end(F); itr != end;
-       itr++) {
+  SmallVector<CallSite, 64> ParsePointNeeded;
+  for (Instruction &I : inst_range(F)) {
     // TODO: only the ones with the flag set!
-    if (isStatepoint(*itr))
-      ParsePointNeeded.push_back(CallSite(&*itr));
+    if (isStatepoint(I))
+      ParsePointNeeded.push_back(CallSite(&I));
   }
 
   // Return early if no work to do.