Replace remaining *Inst::Create() calls with llvm::Builder

Replace the remaining occurrences of `Inst::Create()` with
`llvm::Builder` to manage metadata automatically and to fold
instructions.

C++11 it and clang-format

5 files changed, 102 insertions, 85 deletions

diff --git a/lib/Core/ExternalDispatcher.cpp b/lib/Core/ExternalDispatcher.cpp
index 0a32e98d..c8ebf38a 100644
--- a/lib/Core/ExternalDispatcher.cpp
+++ b/lib/Core/ExternalDispatcher.cpp
@@ -12,6 +12,7 @@
 
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
@@ -306,12 +307,13 @@ Function *ExternalDispatcherImpl::createDispatcher(Function *target,
 
   BasicBlock *dBB = BasicBlock::Create(ctx, "entry", dispatcher);
 
+  llvm::IRBuilder<> Builder(dBB);
   // Get a Value* for &gTheArgsP, as an i64**.
-  Instruction *argI64sp = new IntToPtrInst(
+  auto argI64sp = Builder.CreateIntToPtr(
       ConstantInt::get(Type::getInt64Ty(ctx), (uintptr_t)(void *)&gTheArgsP),
       PointerType::getUnqual(PointerType::getUnqual(Type::getInt64Ty(ctx))),
-      "argsp", dBB);
-  Instruction *argI64s = new LoadInst(argI64sp, "args", dBB);
+      "argsp");
+  auto argI64s = Builder.CreateLoad(argI64sp, "args");
 
   // Get the target function type.
   FunctionType *FTy = cast<FunctionType>(
@@ -321,34 +323,33 @@ Function *ExternalDispatcherImpl::createDispatcher(Function *target,
   unsigned i = 0, idx = 2;
   for (CallSite::arg_iterator ai = cs.arg_begin(), ae = cs.arg_end(); ai != ae;
        ++ai, ++i) {
-    // Determine the type the argument will be passed as. This accomodates for
+    // Determine the type the argument will be passed as. This accommodates for
     // the corresponding code in Executor.cpp for handling calls to bitcasted
     // functions.
-    Type *argTy =
+    auto argTy =
         (i < FTy->getNumParams() ? FTy->getParamType(i) : (*ai)->getType());
-    Instruction *argI64p = GetElementPtrInst::Create(
-        KLEE_LLVM_GEP_TYPE(nullptr)
-        argI64s, ConstantInt::get(Type::getInt32Ty(ctx), idx), "", dBB);
+    auto argI64p =
+        Builder.CreateGEP(KLEE_LLVM_GEP_TYPE(nullptr) argI64s,
+                          ConstantInt::get(Type::getInt32Ty(ctx), idx));
 
-    Instruction *argp =
-        new BitCastInst(argI64p, PointerType::getUnqual(argTy), "", dBB);
-    args[i] = new LoadInst(argp, "", dBB);
+    auto argp = Builder.CreateBitCast(argI64p, PointerType::getUnqual(argTy));
+    args[i] = Builder.CreateLoad(argp);
 
     unsigned argSize = argTy->getPrimitiveSizeInBits();
     idx += ((!!argSize ? argSize : 64) + 63) / 64;
   }
 
-  Constant *dispatchTarget = module->getOrInsertFunction(
-      target->getName(), FTy, target->getAttributes());
-  Instruction *result = CallInst::Create(
-      dispatchTarget, llvm::ArrayRef<Value *>(args, args + i), "", dBB);
+  auto dispatchTarget = module->getOrInsertFunction(target->getName(), FTy,
+                                                    target->getAttributes());
+  auto result = Builder.CreateCall(dispatchTarget,
+                                   llvm::ArrayRef<Value *>(args, args + i));
   if (result->getType() != Type::getVoidTy(ctx)) {
-    Instruction *resp = new BitCastInst(
-        argI64s, PointerType::getUnqual(result->getType()), "", dBB);
-    new StoreInst(result, resp, dBB);
+    auto resp = Builder.CreateBitCast(
+        argI64s, PointerType::getUnqual(result->getType()));
+    Builder.CreateStore(result, resp);
   }
 
-  ReturnInst::Create(ctx, dBB);
+  Builder.CreateRetVoid();
 
   delete[] args;
 
diff --git a/lib/Module/IntrinsicCleaner.cpp b/lib/Module/IntrinsicCleaner.cpp
index 0f7eb223..c48952c2 100644
--- a/lib/Module/IntrinsicCleaner.cpp
+++ b/lib/Module/IntrinsicCleaner.cpp
@@ -64,40 +64,40 @@ bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b, Module &M) {
         // FIXME: This is much more target dependent than just the word size,
         // however this works for x86-32 and x86-64.
       case Intrinsic::vacopy: { // (dst, src) -> *((i8**) dst) = *((i8**) src)
+        llvm::IRBuilder<> Builder(ii);
         Value *dst = ii->getArgOperand(0);
         Value *src = ii->getArgOperand(1);
 
         if (WordSize == 4) {
           Type *i8pp = PointerType::getUnqual(
               PointerType::getUnqual(Type::getInt8Ty(ctx)));
-          Value *castedDst =
-              CastInst::CreatePointerCast(dst, i8pp, "vacopy.cast.dst", ii);
-          Value *castedSrc =
-              CastInst::CreatePointerCast(src, i8pp, "vacopy.cast.src", ii);
-          Value *load = new LoadInst(castedSrc, "vacopy.read", ii);
-          new StoreInst(load, castedDst, false, ii);
+          auto castedDst =
+              Builder.CreatePointerCast(dst, i8pp, "vacopy.cast.dst");
+          auto castedSrc =
+              Builder.CreatePointerCast(src, i8pp, "vacopy.cast.src");
+          auto load = Builder.CreateLoad(castedSrc, "vacopy.read");
+          Builder.CreateStore(load, castedDst, false /* isVolatile */);
         } else {
           assert(WordSize == 8 && "Invalid word size!");
           Type *i64p = PointerType::getUnqual(Type::getInt64Ty(ctx));
-          Value *pDst =
-              CastInst::CreatePointerCast(dst, i64p, "vacopy.cast.dst", ii);
-          Value *pSrc =
-              CastInst::CreatePointerCast(src, i64p, "vacopy.cast.src", ii);
-          Value *val = new LoadInst(pSrc, std::string(), ii);
-          new StoreInst(val, pDst, ii);
-          Value *off = ConstantInt::get(Type::getInt64Ty(ctx), 1);
-          pDst = GetElementPtrInst::Create(KLEE_LLVM_GEP_TYPE(nullptr)
-              pDst, off, std::string(), ii);
-          pSrc = GetElementPtrInst::Create(KLEE_LLVM_GEP_TYPE(nullptr)
-              pSrc, off, std::string(), ii);
-          val = new LoadInst(pSrc, std::string(), ii);
-          new StoreInst(val, pDst, ii);
-          pDst = GetElementPtrInst::Create(KLEE_LLVM_GEP_TYPE(nullptr)
-              pDst, off, std::string(), ii);
-          pSrc = GetElementPtrInst::Create(KLEE_LLVM_GEP_TYPE(nullptr)
-              pSrc, off, std::string(), ii);
-          val = new LoadInst(pSrc, std::string(), ii);
-          new StoreInst(val, pDst, ii);
+          auto pDst = Builder.CreatePointerCast(dst, i64p, "vacopy.cast.dst");
+          auto pSrc = Builder.CreatePointerCast(src, i64p, "vacopy.cast.src");
+          auto val = Builder.CreateLoad(pSrc, std::string());
+          Builder.CreateStore(val, pDst, ii);
+
+          auto off = ConstantInt::get(Type::getInt64Ty(ctx), 1);
+          pDst = Builder.CreateGEP(KLEE_LLVM_GEP_TYPE(nullptr) pDst, off,
+                                   std::string());
+          pSrc = Builder.CreateGEP(KLEE_LLVM_GEP_TYPE(nullptr) pSrc, off,
+                                   std::string());
+          val = Builder.CreateLoad(pSrc, std::string());
+          Builder.CreateStore(val, pDst);
+          pDst = Builder.CreateGEP(KLEE_LLVM_GEP_TYPE(nullptr) pDst, off,
+                                   std::string());
+          pSrc = Builder.CreateGEP(KLEE_LLVM_GEP_TYPE(nullptr) pSrc, off,
+                                   std::string());
+          val = Builder.CreateLoad(pSrc, std::string());
+          Builder.CreateStore(val, pDst);
         }
         ii->eraseFromParent();
         dirty = true;
@@ -207,15 +207,16 @@ bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b, Module &M) {
       }
 
       case Intrinsic::trap: {
-        // Intrisic instruction "llvm.trap" found. Directly lower it to
+        // Intrinsic instruction "llvm.trap" found. Directly lower it to
         // a call of the abort() function.
         Function *F = cast<Function>(
             M.getOrInsertFunction("abort", Type::getVoidTy(ctx), NULL));
         F->setDoesNotReturn();
         F->setDoesNotThrow();
 
-        CallInst::Create(F, Twine(), ii);
-        new UnreachableInst(ctx, ii);
+        llvm::IRBuilder<> Builder(ii);
+        Builder.CreateCall(F);
+        Builder.CreateUnreachable();
 
         ii->eraseFromParent();
 
diff --git a/lib/Module/KModule.cpp b/lib/Module/KModule.cpp
index cbcd395c..74d91a8f 100644
--- a/lib/Module/KModule.cpp
+++ b/lib/Module/KModule.cpp
@@ -22,11 +22,12 @@
 #include "klee/Internal/Support/ModuleUtil.h"
 
 #include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/IR/DataLayout.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ValueSymbolTable.h"
-#include "llvm/IR/DataLayout.h"
 
 #if LLVM_VERSION_CODE < LLVM_VERSION(3, 5)
 #include "llvm/Analysis/Verifier.h"
@@ -105,14 +106,15 @@ static Function *getStubFunctionForCtorList(Module *m,
 				  name,
                               m);
   BasicBlock *bb = BasicBlock::Create(m->getContext(), "entry", fn);
-  
+  llvm::IRBuilder<> Builder(bb);
+
   // From lli:
   // Should be an array of '{ int, void ()* }' structs.  The first value is
   // the init priority, which we ignore.
-  ConstantArray *arr = dyn_cast<ConstantArray>(gv->getInitializer());
+  auto arr = dyn_cast<ConstantArray>(gv->getInitializer());
   if (arr) {
     for (unsigned i=0; i<arr->getNumOperands(); i++) {
-      ConstantStruct *cs = cast<ConstantStruct>(arr->getOperand(i));
+      auto cs = cast<ConstantStruct>(arr->getOperand(i));
 #if LLVM_VERSION_CODE >= LLVM_VERSION(3, 5)
       // There is a third *optional* element in global_ctor elements (``i8
       // @data``).
@@ -121,21 +123,21 @@ static Function *getStubFunctionForCtorList(Module *m,
 #else
       assert(cs->getNumOperands()==2 && "unexpected element in ctor initializer list");
 #endif
-      Constant *fp = cs->getOperand(1);      
+      auto fp = cs->getOperand(1);
       if (!fp->isNullValue()) {
-        if (llvm::ConstantExpr *ce = dyn_cast<llvm::ConstantExpr>(fp))
+        if (auto ce = dyn_cast<llvm::ConstantExpr>(fp))
           fp = ce->getOperand(0);
 
-        if (Function *f = dyn_cast<Function>(fp)) {
-	  CallInst::Create(f, "", bb);
+        if (auto f = dyn_cast<Function>(fp)) {
+          Builder.CreateCall(f);
         } else {
           assert(0 && "unable to get function pointer from ctor initializer list");
         }
       }
     }
   }
-  
-  ReturnInst::Create(m->getContext(), bb);
+
+  Builder.CreateRetVoid();
 
   return fn;
 }
@@ -143,21 +145,30 @@ static Function *getStubFunctionForCtorList(Module *m,
 static void injectStaticConstructorsAndDestructors(Module *m) {
   GlobalVariable *ctors = m->getNamedGlobal("llvm.global_ctors");
   GlobalVariable *dtors = m->getNamedGlobal("llvm.global_dtors");
-  
-  if (ctors || dtors) {
-    Function *mainFn = m->getFunction("main");
-    if (!mainFn)
-      klee_error("Could not find main() function.");
-
-    if (ctors)
-    CallInst::Create(getStubFunctionForCtorList(m, ctors, "klee.ctor_stub"),
-		     "", &*(mainFn->begin()->begin()));
-    if (dtors) {
-      Function *dtorStub = getStubFunctionForCtorList(m, dtors, "klee.dtor_stub");
-      for (Function::iterator it = mainFn->begin(), ie = mainFn->end();
-           it != ie; ++it) {
-        if (isa<ReturnInst>(it->getTerminator()))
-	  CallInst::Create(dtorStub, "", it->getTerminator());
+
+  if (!ctors && !dtors)
+    return;
+
+  Function *mainFn = m->getFunction("main");
+  if (!mainFn)
+    klee_error("Could not find main() function.");
+
+  if (ctors) {
+#if LLVM_VERSION_CODE >= LLVM_VERSION(3, 8)
+    llvm::IRBuilder<> Builder(&*mainFn->begin()->begin());
+#else
+    llvm::IRBuilder<> Builder(mainFn->begin()->begin());
+#endif
+    Builder.CreateCall(getStubFunctionForCtorList(m, ctors, "klee.ctor_stub"));
+  }
+
+  if (dtors) {
+    Function *dtorStub = getStubFunctionForCtorList(m, dtors, "klee.dtor_stub");
+    for (Function::iterator it = mainFn->begin(), ie = mainFn->end(); it != ie;
+         ++it) {
+      if (isa<ReturnInst>(it->getTerminator())) {
+        llvm::IRBuilder<> Builder(it->getTerminator());
+        Builder.CreateCall(dtorStub);
       }
     }
   }
diff --git a/lib/Module/LowerSwitch.cpp b/lib/Module/LowerSwitch.cpp
index 05688521..4d80ddd8 100644
--- a/lib/Module/LowerSwitch.cpp
+++ b/lib/Module/LowerSwitch.cpp
@@ -16,6 +16,7 @@
 
 #include "Passes.h"
 #include "klee/Config/Version.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include <algorithm>
 
@@ -60,7 +61,8 @@ void LowerSwitchPass::switchConvert(CaseItr begin, CaseItr end,
 {
   BasicBlock *curHead = defaultBlock;
   Function *F = origBlock->getParent();
-  
+  llvm::IRBuilder<> Builder(defaultBlock);
+
   // iterate through all the cases, creating a new BasicBlock for each
   for (CaseItr it = begin; it < end; ++it) {
     BasicBlock *newBlock = BasicBlock::Create(F->getContext(), "NodeBlock");
@@ -70,10 +72,9 @@ void LowerSwitchPass::switchConvert(CaseItr begin, CaseItr end,
     Function::iterator FI = origBlock;
 #endif
     F->getBasicBlockList().insert(++FI, newBlock);
-    
-    ICmpInst *cmpInst = 
-      new ICmpInst(*newBlock, ICmpInst::ICMP_EQ, value, it->value, "case.cmp");
-    BranchInst::Create(it->block, curHead, cmpInst, newBlock);
+    Builder.SetInsertPoint(newBlock);
+    auto cmpValue = Builder.CreateICmpEQ(value, it->value, "case.cmp");
+    Builder.CreateCondBr(cmpValue, it->block, curHead);
 
     // If there were any PHI nodes in this successor, rewrite one entry
     // from origBlock to come from newBlock.
@@ -89,7 +90,8 @@ void LowerSwitchPass::switchConvert(CaseItr begin, CaseItr end,
   }
 
   // Branch to our shiny new if-then stuff...
-  BranchInst::Create(curHead, origBlock);
+  Builder.SetInsertPoint(origBlock);
+  Builder.CreateBr(curHead);
 }
 
 // processSwitchInst - Replace the specified switch instruction with a sequence
@@ -104,13 +106,14 @@ void LowerSwitchPass::processSwitchInst(SwitchInst *SI) {
   // Create a new, empty default block so that the new hierarchy of
   // if-then statements go to this and the PHI nodes are happy.
   BasicBlock* newDefault = BasicBlock::Create(F->getContext(), "newDefault");
+  llvm::IRBuilder<> Builder(newDefault);
 
 #if LLVM_VERSION_CODE >= LLVM_VERSION(3, 8)
   F->getBasicBlockList().insert(defaultBlock->getIterator(), newDefault);
 #else
   F->getBasicBlockList().insert(defaultBlock, newDefault);
 #endif
-  BranchInst::Create(defaultBlock, newDefault);
+  Builder.CreateBr(defaultBlock);
 
   // If there is an entry in any PHI nodes for the default edge, make sure
   // to update them as well.
diff --git a/tools/klee/main.cpp b/tools/klee/main.cpp
index c91ce75a..c655aa56 100644
--- a/tools/klee/main.cpp
+++ b/tools/klee/main.cpp
@@ -23,16 +23,17 @@
 #include "klee/Interpreter.h"
 #include "klee/Statistics.h"
 
+#include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/Constants.h"
-#include "llvm/IR/Type.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Type.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Bitcode/ReaderWriter.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -1045,6 +1046,7 @@ createLibCWrapper(std::vector<std::unique_ptr<llvm::Module>> &modules,
                        GlobalVariable::ExternalLinkage, intendedFunction,
                        libcMainFn->getParent());
   BasicBlock *bb = BasicBlock::Create(ctx, "entry", stub);
+  llvm::IRBuilder<> Builder(bb);
 
   std::vector<llvm::Value*> args;
   args.push_back(
@@ -1055,9 +1057,8 @@ createLibCWrapper(std::vector<std::unique_ptr<llvm::Module>> &modules,
   args.push_back(Constant::getNullValue(ft->getParamType(4))); // app_fini
   args.push_back(Constant::getNullValue(ft->getParamType(5))); // rtld_fini
   args.push_back(Constant::getNullValue(ft->getParamType(6))); // stack_end
-  CallInst::Create(libcMainFn, args, "", bb);
-
-  new UnreachableInst(ctx, bb);
+  Builder.CreateCall(libcMainFn, args);
+  Builder.CreateUnreachable();
 }
 
 static void