[opaque pointer types] Add a FunctionCallee wrapper type, and use it.

The FunctionCallee type is effectively a {FunctionType*,Value*} pair,
and is a useful convenience to enable code to continue passing the
result of getOrInsertFunction() through to EmitCall, even once pointer
types lose their pointee-type.

Then:
- update the CallInst/InvokeInst instruction creation functions to
  take a Callee,
- modify getOrInsertFunction to return FunctionCallee, and
- update all callers appropriately.

One area of particular note is the change to the sanitizer
code. Previously, they had been casting the result of
`getOrInsertFunction` to a `Function*` via
`checkSanitizerInterfaceFunction`, and storing that. That would report
an error if someone had already inserted a function declaraction with
a mismatching signature.

However, in general, LLVM allows for such mismatches, as
`getOrInsertFunction` will automatically insert a bitcast if
needed. As part of this cleanup, cause the sanitizer code to do the
same. (It will call its functions using the expected signature,
however they may have been declared.)

Finally, in a small number of locations, callers of
`getOrInsertFunction` actually were expecting/requiring that a brand
new function was being created. In such cases, I've switched them to
Function::Create instead.

Differential Revision: https://reviews.llvm.org/D57315

llvm-svn: 352791

Author: jyknight

clang/lib/CodeGen/CGExpr.cpp

@@ -3056,7 +3056,7 @@ void CodeGenFunction::EmitCfiSlowPathCheck(
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
-  llvm::Constant *SlowPathFn;
+  llvm::FunctionCallee SlowPathFn;
   if (WithDiag) {
     llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
     auto *InfoPtr =
@@ -3078,7 +3078,8 @@ void CodeGenFunction::EmitCfiSlowPathCheck(
     CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
-  CGM.setDSOLocal(cast<llvm::GlobalValue>(SlowPathFn->stripPointerCasts()));
+  CGM.setDSOLocal(
+      cast<llvm::GlobalValue>(SlowPathFn.getCallee()->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);

llvm/docs/ProgrammersManual.rst

@@ -3491,11 +3491,17 @@ Important Public Members of the ``Module`` class
   Look up the specified function in the ``Module`` SymbolTable_.  If it does not
   exist, return ``null``.
 
-* ``Function *getOrInsertFunction(const std::string &Name, const FunctionType
-  *T)``
-
-  Look up the specified function in the ``Module`` SymbolTable_.  If it does not
-  exist, add an external declaration for the function and return it.
+* ``FunctionCallee getOrInsertFunction(const std::string &Name,
+  const FunctionType *T)``
+
+  Look up the specified function in the ``Module`` SymbolTable_.  If
+  it does not exist, add an external declaration for the function and
+  return it. Note that the function signature already present may not
+  match the requested signature. Thus, in order to enable the common
+  usage of passing the result directly to EmitCall, the return type is
+  a struct of ``{FunctionType *T, Constant *FunctionPtr}``, rather
+  than simply the ``Function*`` with potentially an unexpected
+  signature.
 
 * ``std::string getTypeName(const Type *Ty)``
 

llvm/examples/BrainF/BrainF.cpp

@@ -72,19 +72,17 @@ void BrainF::header(LLVMContext& C) {
                                                     Tys);
 
   //declare i32 @getchar()
-  getchar_func = cast<Function>(module->
-    getOrInsertFunction("getchar", IntegerType::getInt32Ty(C)));
+  getchar_func =
+      module->getOrInsertFunction("getchar", IntegerType::getInt32Ty(C));
 
   //declare i32 @putchar(i32)
-  putchar_func = cast<Function>(module->
-    getOrInsertFunction("putchar", IntegerType::getInt32Ty(C),
-                        IntegerType::getInt32Ty(C)));
+  putchar_func = module->getOrInsertFunction(
+      "putchar", IntegerType::getInt32Ty(C), IntegerType::getInt32Ty(C));
 
   //Function header
 
   //define void @brainf()
-  brainf_func = cast<Function>(module->
-    getOrInsertFunction("brainf", Type::getVoidTy(C)));
+  brainf_func = module->getOrInsertFunction("brainf", Type::getVoidTy(C));
 
   builder = new IRBuilder<>(BasicBlock::Create(C, label, brainf_func));
 
@@ -153,9 +151,9 @@ void BrainF::header(LLVMContext& C) {
       "aberrormsg");
 
     //declare i32 @puts(i8 *)
-    Function *puts_func = cast<Function>(module->
-      getOrInsertFunction("puts", IntegerType::getInt32Ty(C),
-                      PointerType::getUnqual(IntegerType::getInt8Ty(C))));
+    FunctionCallee puts_func = module->getOrInsertFunction(
+        "puts", IntegerType::getInt32Ty(C),
+        PointerType::getUnqual(IntegerType::getInt8Ty(C)));
 
     //brainf.aberror:
     aberrorbb = BasicBlock::Create(C, label, brainf_func);

llvm/examples/BrainF/BrainF.h

@@ -78,9 +78,9 @@ class BrainF {
     CompileFlags comflag;
     std::istream *in;
     Module *module;
-    Function *brainf_func;
-    Function *getchar_func;
-    Function *putchar_func;
+    FunctionCallee brainf_func;
+    FunctionCallee getchar_func;
+    FunctionCallee putchar_func;
     Value *ptr_arr;
     Value *ptr_arrmax;
     BasicBlock *endbb;

llvm/examples/BrainF/BrainFDriver.cpp

@@ -72,11 +72,13 @@ JIT("jit", cl::desc("Run program Just-In-Time"));
 //Add main function so can be fully compiled
 void addMainFunction(Module *mod) {
   //define i32 @main(i32 %argc, i8 **%argv)
-  Function *main_func = cast<Function>(mod->
-    getOrInsertFunction("main", IntegerType::getInt32Ty(mod->getContext()),
-                        IntegerType::getInt32Ty(mod->getContext()),
-                        PointerType::getUnqual(PointerType::getUnqual(
-                          IntegerType::getInt8Ty(mod->getContext())))));
+  FunctionType *main_func_fty = FunctionType::get(
+      Type::getInt32Ty(mod->getContext()),
+      {Type::getInt32Ty(mod->getContext()),
+       Type::getInt8Ty(mod->getContext())->getPointerTo()->getPointerTo()});
+  Function *main_func =
+      Function::create(main_func_fty, Function::ExternalLinkage, "main", mod);
+
   {
     Function::arg_iterator args = main_func->arg_begin();
     Value *arg_0 = &*args++;

llvm/examples/Fibonacci/fibonacci.cpp

@@ -51,9 +51,10 @@ using namespace llvm;
 static Function *CreateFibFunction(Module *M, LLVMContext &Context) {
   // Create the fib function and insert it into module M. This function is said
   // to return an int and take an int parameter.
+  FunctionType *FibFTy = FunctionType::get(Type::getInt32Ty(Context),
+                                           {Type::getInt32Ty(Context)}, false);
   Function *FibF =
-    cast<Function>(M->getOrInsertFunction("fib", Type::getInt32Ty(Context),
-                                          Type::getInt32Ty(Context)));
+      Function::Create(FibFTy, Function::ExternalLinkage, "fib", M);
 
   // Add a basic block to the function.
   BasicBlock *BB = BasicBlock::Create(Context, "EntryBlock", FibF);

llvm/examples/HowToUseJIT/HowToUseJIT.cpp

@@ -69,8 +69,9 @@ int main() {
   // Create the add1 function entry and insert this entry into module M.  The
   // function will have a return type of "int" and take an argument of "int".
   Function *Add1F =
-    cast<Function>(M->getOrInsertFunction("add1", Type::getInt32Ty(Context),
-                                          Type::getInt32Ty(Context)));
+      Function::Create(FunctionType::get(Type::getInt32Ty(Context),
+                                         {Type::getInt32Ty(Context)}, false),
+                       Function::ExternalLinkage, "add1", M);
 
   // Add a basic block to the function. As before, it automatically inserts
   // because of the last argument.
@@ -99,7 +100,8 @@ int main() {
   // Now we're going to create function `foo', which returns an int and takes no
   // arguments.
   Function *FooF =
-    cast<Function>(M->getOrInsertFunction("foo", Type::getInt32Ty(Context)));
+      Function::Create(FunctionType::get(Type::getInt32Ty(Context), {}, false),
+                       Function::ExternalLinkage, "foo", M);
 
   // Add a basic block to the FooF function.
   BB = BasicBlock::Create(Context, "EntryBlock", FooF);

llvm/examples/ParallelJIT/ParallelJIT.cpp

@@ -49,11 +49,10 @@ using namespace llvm;
 static Function* createAdd1(Module *M) {
   // Create the add1 function entry and insert this entry into module M.  The
   // function will have a return type of "int" and take an argument of "int".
-  // The '0' terminates the list of argument types.
   Function *Add1F =
-    cast<Function>(M->getOrInsertFunction("add1",
-                                          Type::getInt32Ty(M->getContext()),
-                                          Type::getInt32Ty(M->getContext())));
+      Function::Create(FunctionType::get(Type::getInt32Ty(Context),
+                                         {Type::getInt32Ty(Context)}, false),
+                       Function::ExternalLinkage, "add1", M);
 
   // Add a basic block to the function. As before, it automatically inserts
   // because of the last argument.
@@ -80,10 +79,10 @@ static Function* createAdd1(Module *M) {
 static Function *CreateFibFunction(Module *M) {
   // Create the fib function and insert it into module M.  This function is said
   // to return an int and take an int parameter.
-  Function *FibF = 
-    cast<Function>(M->getOrInsertFunction("fib",
-                                          Type::getInt32Ty(M->getContext()),
-                                          Type::getInt32Ty(M->getContext())));
+  FunctionType *FibFTy = FunctionType::get(Type::getInt32Ty(Context),
+                                           {Type::getInt32Ty(Context)}, false);
+  Function *FibF =
+      Function::Create(FibFTy, Function::ExternalLinkage, "fib", M);
 
   // Add a basic block to the function.
   BasicBlock *BB = BasicBlock::Create(M->getContext(), "EntryBlock", FibF);

llvm/include/llvm/CodeGen/IntrinsicLowering.h

@@ -30,10 +30,6 @@ class IntrinsicLowering {
 public:
   explicit IntrinsicLowering(const DataLayout &DL) : DL(DL), Warned(false) {}
 
-  /// Add all of the prototypes that might be needed by an intrinsic lowering
-  /// implementation to be inserted into the module specified.
-  void AddPrototypes(Module &M);
-
   /// Replace a call to the specified intrinsic function.
   /// If an intrinsic function must be implemented by the code generator
   /// (such as va_start), this function should print a message and abort.

llvm/include/llvm/IR/DerivedTypes.h

@@ -157,6 +157,39 @@ unsigned Type::getFunctionNumParams() const {
   return cast<FunctionType>(this)->getNumParams();
 }
 
+/// A handy container for a FunctionType+Callee-pointer pair, which can be
+/// passed around as a single entity. This assists in replacing the use of
+/// PointerType::getElementType() to access the function's type, since that's
+/// slated for removal as part of the [opaque pointer types] project.
+class FunctionCallee {
+public:
+  // Allow implicit conversion from types which have a getFunctionType member
+  // (e.g. Function and InlineAsm).
+  template <typename T,
+            typename U = std::enable_if<&T::getFunctionType != nullptr, void>>
+  FunctionCallee(T *Fn)
+      : FnTy(Fn ? Fn->getFunctionType() : nullptr), Callee(Fn) {}
+
+  FunctionCallee(FunctionType *FnTy, Value *Callee)
+      : FnTy(FnTy), Callee(Callee) {
+    assert((FnTy == nullptr) == (Callee == nullptr));
+  }
+
+  FunctionCallee(std::nullptr_t) {}
+
+  FunctionCallee() = default;
+
+  FunctionType *getFunctionType() { return FnTy; }
+
+  Value *getCallee() { return Callee; }
+
+  explicit operator bool() { return Callee; }
+
+private:
+  FunctionType *FnTy = nullptr;
+  Value *Callee = nullptr;
+};
+
 /// Common super class of ArrayType, StructType and VectorType.
 class CompositeType : public Type {
 protected:

llvm/include/llvm/IR/IRBuilder.h

@@ -905,20 +905,20 @@ class IRBuilder : public IRBuilderBase, public Inserter {
                   Name);
   }
 
-  InvokeInst *CreateInvoke(Function *Callee, BasicBlock *NormalDest,
+  InvokeInst *CreateInvoke(FunctionCallee Callee, BasicBlock *NormalDest,
                            BasicBlock *UnwindDest, ArrayRef<Value *> Args,
                            ArrayRef<OperandBundleDef> OpBundles,
                            const Twine &Name = "") {
-    return CreateInvoke(Callee->getFunctionType(), Callee, NormalDest,
-                        UnwindDest, Args, OpBundles, Name);
+    return CreateInvoke(Callee.getFunctionType(), Callee.getCallee(),
+                        NormalDest, UnwindDest, Args, OpBundles, Name);
   }
 
-  InvokeInst *CreateInvoke(Function *Callee, BasicBlock *NormalDest,
+  InvokeInst *CreateInvoke(FunctionCallee Callee, BasicBlock *NormalDest,
                            BasicBlock *UnwindDest,
                            ArrayRef<Value *> Args = None,
                            const Twine &Name = "") {
-    return CreateInvoke(Callee->getFunctionType(), Callee, NormalDest,
-                        UnwindDest, Args, Name);
+    return CreateInvoke(Callee.getFunctionType(), Callee.getCallee(),
+                        NormalDest, UnwindDest, Args, Name);
   }
 
   // Deprecated [opaque pointer types]
@@ -1988,16 +1988,17 @@ class IRBuilder : public IRBuilderBase, public Inserter {
     return Insert(CI, Name);
   }
 
-  CallInst *CreateCall(Function *Callee, ArrayRef<Value *> Args = None,
+  CallInst *CreateCall(FunctionCallee Callee, ArrayRef<Value *> Args = None,
                        const Twine &Name = "", MDNode *FPMathTag = nullptr) {
-    return CreateCall(Callee->getFunctionType(), Callee, Args, Name, FPMathTag);
+    return CreateCall(Callee.getFunctionType(), Callee.getCallee(), Args, Name,
+                      FPMathTag);
   }
 
-  CallInst *CreateCall(Function *Callee, ArrayRef<Value *> Args,
+  CallInst *CreateCall(FunctionCallee Callee, ArrayRef<Value *> Args,
                        ArrayRef<OperandBundleDef> OpBundles,
                        const Twine &Name = "", MDNode *FPMathTag = nullptr) {
-    return CreateCall(Callee->getFunctionType(), Callee, Args, OpBundles, Name,
-                      FPMathTag);
+    return CreateCall(Callee.getFunctionType(), Callee.getCallee(), Args,
+                      OpBundles, Name, FPMathTag);
   }
 
   // Deprecated [opaque pointer types]

llvm/include/llvm/IR/InstrTypes.h

@@ -1232,6 +1232,11 @@ class CallBase : public Instruction {
         Fn);
   }
 
+  /// Sets the function called, including updating the function type.
+  void setCalledFunction(FunctionCallee Fn) {
+    setCalledFunction(Fn.getFunctionType(), Fn.getCallee());
+  }
+
   /// Sets the function called, including updating to the specified function
   /// type.
   void setCalledFunction(FunctionType *FTy, Value *Fn) {

llvm/include/llvm/IR/Instructions.h

@@ -1543,25 +1543,44 @@ class CallInst : public CallBase {
         CallInst(Ty, Func, Args, Bundles, NameStr, InsertAtEnd);
   }
 
-  static CallInst *Create(Function *Func, const Twine &NameStr = "",
+  static CallInst *Create(FunctionCallee Func, const Twine &NameStr = "",
                           Instruction *InsertBefore = nullptr) {
-    return Create(Func->getFunctionType(), Func, NameStr, InsertBefore);
+    return Create(Func.getFunctionType(), Func.getCallee(), NameStr,
+                  InsertBefore);
   }
 
-  static CallInst *Create(Function *Func, ArrayRef<Value *> Args,
+  static CallInst *Create(FunctionCallee Func, ArrayRef<Value *> Args,
+                          ArrayRef<OperandBundleDef> Bundles = None,
                           const Twine &NameStr = "",
                           Instruction *InsertBefore = nullptr) {
-    return Create(Func->getFunctionType(), Func, Args, NameStr, InsertBefore);
+    return Create(Func.getFunctionType(), Func.getCallee(), Args, Bundles,
+                  NameStr, InsertBefore);
+  }
+
+  static CallInst *Create(FunctionCallee Func, ArrayRef<Value *> Args,
+                          const Twine &NameStr,
+                          Instruction *InsertBefore = nullptr) {
+    return Create(Func.getFunctionType(), Func.getCallee(), Args, NameStr,
+                  InsertBefore);
   }
 
-  static CallInst *Create(Function *Func, const Twine &NameStr,
+  static CallInst *Create(FunctionCallee Func, const Twine &NameStr,
                           BasicBlock *InsertAtEnd) {
-    return Create(Func->getFunctionType(), Func, NameStr, InsertAtEnd);
+    return Create(Func.getFunctionType(), Func.getCallee(), NameStr,
+                  InsertAtEnd);
   }
 
-  static CallInst *Create(Function *Func, ArrayRef<Value *> Args,
+  static CallInst *Create(FunctionCallee Func, ArrayRef<Value *> Args,
                           const Twine &NameStr, BasicBlock *InsertAtEnd) {
-    return Create(Func->getFunctionType(), Func, Args, NameStr, InsertAtEnd);
+    return Create(Func.getFunctionType(), Func.getCallee(), Args, NameStr,
+                  InsertAtEnd);
+  }
+
+  static CallInst *Create(FunctionCallee Func, ArrayRef<Value *> Args,
+                          ArrayRef<OperandBundleDef> Bundles,
+                          const Twine &NameStr, BasicBlock *InsertAtEnd) {
+    return Create(Func.getFunctionType(), Func.getCallee(), Args, Bundles,
+                  NameStr, InsertAtEnd);
   }
 
   // Deprecated [opaque pointer types]
@@ -3704,36 +3723,36 @@ class InvokeInst : public CallBase {
                    NameStr, InsertAtEnd);
   }
 
-  static InvokeInst *Create(Function *Func, BasicBlock *IfNormal,
+  static InvokeInst *Create(FunctionCallee Func, BasicBlock *IfNormal,
                             BasicBlock *IfException, ArrayRef<Value *> Args,
                             const Twine &NameStr,
                             Instruction *InsertBefore = nullptr) {
-    return Create(Func->getFunctionType(), Func, IfNormal, IfException, Args,
-                  None, NameStr, InsertBefore);
+    return Create(Func.getFunctionType(), Func.getCallee(), IfNormal,
+                  IfException, Args, None, NameStr, InsertBefore);
   }
 
-  static InvokeInst *Create(Function *Func, BasicBlock *IfNormal,
+  static InvokeInst *Create(FunctionCallee Func, BasicBlock *IfNormal,
                             BasicBlock *IfException, ArrayRef<Value *> Args,
                             ArrayRef<OperandBundleDef> Bundles = None,
                             const Twine &NameStr = "",
                             Instruction *InsertBefore = nullptr) {
-    return Create(Func->getFunctionType(), Func, IfNormal, IfException, Args,
-                  Bundles, NameStr, InsertBefore);
+    return Create(Func.getFunctionType(), Func.getCallee(), IfNormal,
+                  IfException, Args, Bundles, NameStr, InsertBefore);
   }
 
-  static InvokeInst *Create(Function *Func, BasicBlock *IfNormal,
+  static InvokeInst *Create(FunctionCallee Func, BasicBlock *IfNormal,
                             BasicBlock *IfException, ArrayRef<Value *> Args,
                             const Twine &NameStr, BasicBlock *InsertAtEnd) {
-    return Create(Func->getFunctionType(), Func, IfNormal, IfException, Args,
-                  NameStr, InsertAtEnd);
+    return Create(Func.getFunctionType(), Func.getCallee(), IfNormal,
+                  IfException, Args, NameStr, InsertAtEnd);
   }
 
-  static InvokeInst *Create(Function *Func, BasicBlock *IfNormal,
+  static InvokeInst *Create(FunctionCallee Func, BasicBlock *IfNormal,
                             BasicBlock *IfException, ArrayRef<Value *> Args,
                             ArrayRef<OperandBundleDef> Bundles,
                             const Twine &NameStr, BasicBlock *InsertAtEnd) {
-    return Create(Func->getFunctionType(), Func, IfNormal, IfException, Args,
-                  Bundles, NameStr, InsertAtEnd);
+    return Create(Func.getFunctionType(), Func.getCallee(), IfNormal,
+                  IfException, Args, Bundles, NameStr, InsertAtEnd);
   }
 
   // Deprecated [opaque pointer types]