[cfe-commits] r170034 - in /cfe/trunk: lib/CodeGen/CodeGenModule.cpp test/CodeGen/exceptions.c test/CodeGen/functions.c

[Eli Friedman]

On Wed, Dec 12, 2012 at 2:21 PM, John McCall <rjmccall at apple.com> wrote:
> Author: rjmccall
> Date: Wed Dec 12 16:21:47 2012
> New Revision: 170034
>
> URL: http://llvm.org/viewvc/llvm-project?rev=170034&view=rev
> Log:
> Rewrite calls to bitcast unprototyped functions when emitting a definition.
>
> My variadics patch, r169588, changed these calls to typically be
> bitcasts rather than calls to a supposedly variadic function.
> This totally subverted a hack where we intentionally dropped
> excess arguments from such calls in order to appease the inliner
> and a "warning" from the optimizer.  This patch extends the hack
> to also work with bitcasts, as well as teaching it to rewrite
> invokes.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>     cfe/trunk/test/CodeGen/exceptions.c
>     cfe/trunk/test/CodeGen/functions.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=170034&r1=170033&r2=170034&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Dec 12 16:21:47 2012
> @@ -1785,99 +1785,131 @@
>    return llvm::GlobalVariable::ExternalLinkage;
>  }
>
> -/// ReplaceUsesOfNonProtoTypeWithRealFunction - This function is called when we
> -/// implement a function with no prototype, e.g. "int foo() {}".  If there are
> -/// existing call uses of the old function in the module, this adjusts them to
> -/// call the new function directly.
> -///
> -/// This is not just a cleanup: the always_inline pass requires direct calls to
> -/// functions to be able to inline them.  If there is a bitcast in the way, it
> -/// won't inline them.  Instcombine normally deletes these calls, but it isn't
> -/// run at -O0.
> -static void ReplaceUsesOfNonProtoTypeWithRealFunction(llvm::GlobalValue *Old,
> -                                                      llvm::Function *NewFn) {
> -  // If we're redefining a global as a function, don't transform it.
> -  llvm::Function *OldFn = dyn_cast<llvm::Function>(Old);
> -  if (OldFn == 0) return;
> -
> -  llvm::Type *NewRetTy = NewFn->getReturnType();
> -  SmallVector<llvm::Value*, 4> ArgList;
> +/// Replace the uses of a function that was declared with a non-proto type.
> +/// We want to silently drop extra arguments from call sites
> +static void replaceUsesOfNonProtoConstant(llvm::Constant *old,
> +                                          llvm::Function *newFn) {
> +  // Fast path.
> +  if (old->use_empty()) return;
> +
> +  llvm::Type *newRetTy = newFn->getReturnType();
> +  SmallVector<llvm::Value*, 4> newArgs;
> +
> +  for (llvm::Value::use_iterator ui = old->use_begin(), ue = old->use_end();
> +         ui != ue; ) {
> +    llvm::Value::use_iterator use = ui++; // Increment before the use is erased.
> +    llvm::User *user = *use;
> +
> +    // Recognize and replace uses of bitcasts.  Most calls to
> +    // unprototyped functions will use bitcasts.
> +    if (llvm::ConstantExpr *bitcast = dyn_cast<llvm::ConstantExpr>(user)) {
> +      if (bitcast->getOpcode() == llvm::Instruction::BitCast)
> +        replaceUsesOfNonProtoConstant(bitcast, newFn);
> +      continue;
> +    }
>
> -  for (llvm::Value::use_iterator UI = OldFn->use_begin(), E = OldFn->use_end();
> -       UI != E; ) {
> -    // TODO: Do invokes ever occur in C code?  If so, we should handle them too.
> -    llvm::Value::use_iterator I = UI++; // Increment before the CI is erased.
> -    llvm::CallInst *CI = dyn_cast<llvm::CallInst>(*I);
> -    if (!CI) continue; // FIXME: when we allow Invoke, just do CallSite CS(*I)
> -    llvm::CallSite CS(CI);
> -    if (!CI || !CS.isCallee(I)) continue;
> -
> -    // If the return types don't match exactly, and if the call isn't dead, then
> -    // we can't transform this call.
> -    if (CI->getType() != NewRetTy && !CI->use_empty())
> +    // Recognize calls to the function.
> +    llvm::CallSite callSite(user);
> +    if (!callSite) continue;
> +    if (!callSite.isCallee(use)) continue;
> +
> +    // If the return types don't match exactly, then we can't
> +    // transform this call unless it's dead.
> +    if (callSite->getType() != newRetTy && !callSite->use_empty())
>        continue;

We can handle the case where both types are pointer types, but the
pointee is different.  (This might be important if the return type is
a pointer to an incomplete function type.)

> -    // Get the attribute list.
> -    llvm::SmallVector<llvm::AttributeWithIndex, 8> AttrVec;
> -    llvm::AttributeSet AttrList = CI->getAttributes();
> -
> -    // Get any return attributes.
> -    llvm::Attributes RAttrs = AttrList.getRetAttributes();
> -
> -    // Add the return attributes.
> -    if (RAttrs.hasAttributes())
> -      AttrVec.push_back(llvm::
> -                        AttributeWithIndex::get(llvm::AttributeSet::ReturnIndex,
> -                                                RAttrs));
> -
> -    // If the function was passed too few arguments, don't transform.  If extra
> -    // arguments were passed, we silently drop them.  If any of the types
> -    // mismatch, we don't transform.
> -    unsigned ArgNo = 0;
> -    bool DontTransform = false;
> -    for (llvm::Function::arg_iterator AI = NewFn->arg_begin(),
> -         E = NewFn->arg_end(); AI != E; ++AI, ++ArgNo) {
> -      if (CS.arg_size() == ArgNo ||
> -          CS.getArgument(ArgNo)->getType() != AI->getType()) {
> -        DontTransform = true;
> +    // Get the call site's attribute list.
> +    llvm::SmallVector<llvm::AttributeWithIndex, 8> newAttrs;
> +    llvm::AttributeSet oldAttrs = callSite.getAttributes();
> +
> +    // Collect any return attributes from the call.
> +    llvm::Attributes returnAttrs = oldAttrs.getRetAttributes();
> +    if (returnAttrs.hasAttributes())
> +      newAttrs.push_back(llvm::AttributeWithIndex::get(
> +                                llvm::AttributeSet::ReturnIndex, returnAttrs));
> +
> +    // If the function was passed too few arguments, don't transform.
> +    unsigned newNumArgs = newFn->arg_size();
> +    if (callSite.arg_size() < newNumArgs) continue;
> +
> +    // If extra arguments were passed, we silently drop them.
> +    // If any of the types mismatch, we don't transform.
> +    unsigned argNo = 0;
> +    bool dontTransform = false;
> +    for (llvm::Function::arg_iterator ai = newFn->arg_begin(),
> +           ae = newFn->arg_end(); ai != ae; ++ai, ++argNo) {
> +      if (callSite.getArgument(argNo)->getType() != ai->getType()) {
> +        dontTransform = true;
>          break;
>        }

Same here.

-Eli