From a923c9783e8bd8edd26e56df9eaacd872c43e20e Mon Sep 17 00:00:00 2001
From: John McCall <rjmccall@apple.com>
Date: Wed, 12 Dec 2012 22:21:47 +0000
Subject: [PATCH] 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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170034 91177308-0d34-0410-b5e6-96231b3b80d8
---
lib/CodeGen/CodeGenModule.cpp | 190 ++++++++++++++++++++--------------
test/CodeGen/exceptions.c | 9 ++
test/CodeGen/functions.c | 2 +-
3 files changed, 123 insertions(+), 78 deletions(-)
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 8a6f05368ff..d8da042d5c9 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -1785,99 +1785,131 @@ CodeGenModule::GetLLVMLinkageVarDefinition(const VarDecl *D,
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;
-
- 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())
+/// 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;
+ }
+
+ // Recognize calls to the function.
+ llvm::CallSite callSite(user);
+ if (!callSite) continue;
+ if (!callSite.isCallee(use)) continue;
- // 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;
+ // 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;
+
+ // 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;
}
// Add any parameter attributes.
- llvm::Attributes PAttrs = AttrList.getParamAttributes(ArgNo + 1);
- if (PAttrs.hasAttributes())
- AttrVec.push_back(llvm::AttributeWithIndex::get(ArgNo + 1, PAttrs));
+ llvm::Attributes pAttrs = oldAttrs.getParamAttributes(argNo + 1);
+ if (pAttrs.hasAttributes())
+ newAttrs.push_back(llvm::AttributeWithIndex::get(argNo + 1, pAttrs));
}
- if (DontTransform)
+ if (dontTransform)
continue;
- llvm::Attributes FnAttrs = AttrList.getFnAttributes();
- if (FnAttrs.hasAttributes())
- AttrVec.push_back(llvm::
+ llvm::Attributes fnAttrs = oldAttrs.getFnAttributes();
+ if (fnAttrs.hasAttributes())
+ newAttrs.push_back(llvm::
AttributeWithIndex::get(llvm::AttributeSet::FunctionIndex,
- FnAttrs));
+ fnAttrs));
// Okay, we can transform this. Create the new call instruction and copy
// over the required information.
- ArgList.append(CS.arg_begin(), CS.arg_begin() + ArgNo);
- llvm::CallInst *NewCall = llvm::CallInst::Create(NewFn, ArgList, "", CI);
- ArgList.clear();
- if (!NewCall->getType()->isVoidTy())
- NewCall->takeName(CI);
- NewCall->setAttributes(llvm::AttributeSet::get(OldFn->getContext(), AttrVec));
- NewCall->setCallingConv(CI->getCallingConv());
+ newArgs.append(callSite.arg_begin(), callSite.arg_begin() + argNo);
+
+ llvm::CallSite newCall;
+ if (callSite.isCall()) {
+ newCall = llvm::CallInst::Create(newFn, newArgs, "",
+ callSite.getInstruction());
+ } else {
+ llvm::InvokeInst *oldInvoke =
+ cast<llvm::InvokeInst>(callSite.getInstruction());
+ newCall = llvm::InvokeInst::Create(newFn,
+ oldInvoke->getNormalDest(),
+ oldInvoke->getUnwindDest(),
+ newArgs, "",
+ callSite.getInstruction());
+ }
+ newArgs.clear(); // for the next iteration
+
+ if (!newCall->getType()->isVoidTy())
+ newCall->takeName(callSite.getInstruction());
+ newCall.setAttributes(
+ llvm::AttributeSet::get(newFn->getContext(), newAttrs));
+ newCall.setCallingConv(callSite.getCallingConv());
// Finally, remove the old call, replacing any uses with the new one.
- if (!CI->use_empty())
- CI->replaceAllUsesWith(NewCall);
+ if (!callSite->use_empty())
+ callSite->replaceAllUsesWith(newCall.getInstruction());
// Copy debug location attached to CI.
- if (!CI->getDebugLoc().isUnknown())
- NewCall->setDebugLoc(CI->getDebugLoc());
- CI->eraseFromParent();
+ if (!callSite->getDebugLoc().isUnknown())
+ newCall->setDebugLoc(callSite->getDebugLoc());
+ callSite->eraseFromParent();
}
}
+/// 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.
+ if (!isa<llvm::Function>(Old)) return;
+
+ replaceUsesOfNonProtoConstant(Old, NewFn);
+}
+
void CodeGenModule::HandleCXXStaticMemberVarInstantiation(VarDecl *VD) {
TemplateSpecializationKind TSK = VD->getTemplateSpecializationKind();
// If we have a definition, this might be a deferred decl. If the
@@ -1921,10 +1953,14 @@ void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD) {
OldFn->setName(StringRef());
llvm::Function *NewFn = cast<llvm::Function>(GetAddrOfFunction(GD, Ty));
- // If this is an implementation of a function without a prototype, try to
- // replace any existing uses of the function (which may be calls) with uses
- // of the new function
- if (D->getType()->isFunctionNoProtoType()) {
+ // This might be an implementation of a function without a
+ // prototype, in which case, try to do special replacement of
+ // calls which match the new prototype. The really key thing here
+ // is that we also potentially drop arguments from the call site
+ // so as to make a direct call, which makes the inliner happier
+ // and suppresses a number of optimizer warnings (!) about
+ // dropping arguments.
+ if (!OldFn->use_empty()) {
ReplaceUsesOfNonProtoTypeWithRealFunction(OldFn, NewFn);
OldFn->removeDeadConstantUsers();
}
diff --git a/test/CodeGen/exceptions.c b/test/CodeGen/exceptions.c
index 20eb706a03b..311bc84e6a1 100644
--- a/test/CodeGen/exceptions.c
+++ b/test/CodeGen/exceptions.c
@@ -19,3 +19,12 @@ void test1() {
// CHECK-ARM: landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gcc_personality_sj0 to i8*)
// CHECK-ARM-NEXT: cleanup
}
+
+void test2_helper();
+void test2() {
+ __block int x = 10;
+ test2_helper(5, 6, 7);
+}
+void test2_helper(int x, int y) {
+}
+// CHECK: invoke void @test2_helper(i32 5, i32 6)
diff --git a/test/CodeGen/functions.c b/test/CodeGen/functions.c
index 28e4bd0c829..8241a3d1674 100644
--- a/test/CodeGen/functions.c
+++ b/test/CodeGen/functions.c
@@ -24,7 +24,7 @@ void f0() {}
void f1();
void f2(void) {
-// CHECK: call void bitcast (void ()* @f1 to void (i32, i32, i32)*)(i32 1, i32 2, i32 3)
+// CHECK: call void @f1()
f1(1, 2, 3);
}
// CHECK: define void @f1()
--
GitLab