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

[John McCall]

On Dec 12, 2012, at 2:40 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 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.)

I agree, we could.  I'm not really motivated to extend this hack indefinitely, though.

John.