[llvm-commits] [llvm] r113190 - /llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
Nick Lewycky
nicholas at mxc.ca
Mon Sep 6 18:42:10 PDT 2010
Author: nicholas
Date: Mon Sep 6 20:42:10 2010
New Revision: 113190
URL: http://llvm.org/viewvc/llvm-project?rev=113190&view=rev
Log:
Fix major bug in thunk detection. Also verify the calling convention.
Switch from isWeakForLinker to mayBeOverridden which is more accurate.
Add more statistics and debugging info. Add comments. Move static function
outside anonymous namespace.
Modified:
llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=113190&r1=113189&r2=113190&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Mon Sep 6 20:42:10 2010
@@ -67,9 +67,12 @@
using namespace llvm;
STATISTIC(NumFunctionsMerged, "Number of functions merged");
+STATISTIC(NumThunksWritten, "Number of thunks generated");
+STATISTIC(NumDoubleWeak, "Number of new functions created");
-namespace {
-
+/// ProfileFunction - Creates a hash-code for the function which is the same
+/// for any two functions that will compare equal, without looking at the
+/// instructions inside the function.
static unsigned ProfileFunction(const Function *F) {
const FunctionType *FTy = F->getFunctionType();
@@ -84,6 +87,8 @@
return ID.ComputeHash();
}
+namespace {
+
class ComparableFunction {
public:
static const ComparableFunction EmptyKey;
@@ -117,7 +122,7 @@
const ComparableFunction ComparableFunction::TombstoneKey =
ComparableFunction(1);
-} // anonymous namespace
+}
namespace llvm {
template <>
@@ -508,14 +513,14 @@
return false;
assert(F1->arg_size() == F2->arg_size() &&
- "Identical functions have a different number of args.");
+ "Identically typed functions have different numbers of args!");
// Visit the arguments so that they get enumerated in the order they're
// passed in.
for (Function::const_arg_iterator f1i = F1->arg_begin(),
f2i = F2->arg_begin(), f1e = F1->arg_end(); f1i != f1e; ++f1i, ++f2i) {
if (!Enumerate(f1i, f2i))
- llvm_unreachable("Arguments repeat");
+ llvm_unreachable("Arguments repeat!");
}
// We do a CFG-ordered walk since the actual ordering of the blocks in the
@@ -567,7 +572,7 @@
}
}
- // If G was internal then we may have replaced all uses if G with F. If so,
+ // If G was internal then we may have replaced all uses of G with F. If so,
// stop here and delete G. There's no need for a thunk.
if (G->hasLocalLinkage() && G->use_empty()) {
G->eraseFromParent();
@@ -601,13 +606,16 @@
NewG->takeName(G);
G->replaceAllUsesWith(NewG);
G->eraseFromParent();
+
+ DEBUG(dbgs() << "WriteThunk: " << NewG->getName() << '\n');
+ ++NumThunksWritten;
}
/// MergeTwoFunctions - Merge two equivalent functions. Upon completion,
/// Function G is deleted.
void MergeFunctions::MergeTwoFunctions(Function *F, Function *G) const {
- if (F->isWeakForLinker()) {
- assert(G->isWeakForLinker());
+ if (F->mayBeOverridden()) {
+ assert(G->mayBeOverridden());
// Make them both thunks to the same internal function.
Function *H = Function::Create(F->getFunctionType(), F->getLinkage(), "",
@@ -623,6 +631,8 @@
F->setAlignment(MaxAlignment);
F->setLinkage(GlobalValue::InternalLinkage);
+
+ ++NumDoubleWeak;
} else {
WriteThunk(F, G);
}
@@ -640,8 +650,8 @@
const ComparableFunction &OldF = *Result.first;
// Never thunk a strong function to a weak function.
- assert(!OldF.getFunc()->isWeakForLinker() ||
- NewF.getFunc()->isWeakForLinker());
+ assert(!OldF.getFunc()->mayBeOverridden() ||
+ NewF.getFunc()->mayBeOverridden());
DEBUG(dbgs() << " " << OldF.getFunc()->getName() << " == "
<< NewF.getFunc()->getName() << '\n');
@@ -660,8 +670,7 @@
// then we may try to merge unmergable thing (ie., identical weak functions)
// which will push us into an infinite loop.
- if (F->size() != 1)
- return false;
+ assert(!F->isDeclaration() && "Expected a function definition.");
const BasicBlock *BB = &F->front();
// A thunk is:
@@ -670,6 +679,10 @@
// ret void|optional-reg
// where the args are in the same order as the arguments.
+ // Put this at the top since it triggers most often.
+ const ReturnInst *RI = dyn_cast<ReturnInst>(BB->getTerminator());
+ if (!RI) return false;
+
// Verify that the sequence of bitcast-inst's are all casts of arguments and
// that there aren't any extras (ie. no repeated casts).
int LastArgNo = -1;
@@ -677,18 +690,22 @@
while (const BitCastInst *BCI = dyn_cast<BitCastInst>(I)) {
const Argument *A = dyn_cast<Argument>(BCI->getOperand(0));
if (!A) return false;
- if ((int)A->getArgNo() >= LastArgNo) return false;
+ if ((int)A->getArgNo() <= LastArgNo) return false;
LastArgNo = A->getArgNo();
++I;
}
+ // Verify that we have a direct tail call and that the calling conventions
+ // and number of arguments match.
+ const CallInst *CI = dyn_cast<CallInst>(I++);
+ if (!CI || !CI->isTailCall() || !CI->getCalledFunction() ||
+ CI->getCallingConv() != CI->getCalledFunction()->getCallingConv() ||
+ CI->getNumArgOperands() != F->arg_size())
+ return false;
+
// Verify that the call instruction has the same arguments as this function
// and that they're all either the incoming argument or a cast of the right
// argument.
- const CallInst *CI = dyn_cast<CallInst>(I++);
- if (!CI || !CI->isTailCall() ||
- CI->getNumArgOperands() != F->arg_size()) return false;
-
for (unsigned i = 0, e = CI->getNumArgOperands(); i != e; ++i) {
const Value *V = CI->getArgOperand(i);
const Argument *A = dyn_cast<Argument>(V);
@@ -708,8 +725,7 @@
if (BCI->getOperand(0) != CI) return false;
RetOp = BCI;
}
- const ReturnInst *RI = dyn_cast<ReturnInst>(I);
- if (!RI) return false;
+ if (RI != I) return false;
if (RI->getNumOperands() == 0)
return CI->getType()->isVoidTy();
return RI->getReturnValue() == CI;
@@ -721,7 +737,7 @@
bool LocalChanged;
do {
- DEBUG(dbgs() << "size: " << M.size() << '\n');
+ DEBUG(dbgs() << "size of module: " << M.size() << '\n');
LocalChanged = false;
FnSetType FnSet;
@@ -730,7 +746,7 @@
for (Module::iterator I = M.begin(), E = M.end(); I != E;) {
Function *F = I++;
if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage() &&
- !F->isWeakForLinker() && !IsThunk(F)) {
+ !F->mayBeOverridden() && !IsThunk(F)) {
ComparableFunction CF = ComparableFunction(F, TD);
LocalChanged |= Insert(FnSet, CF);
}
@@ -743,11 +759,12 @@
for (Module::iterator I = M.begin(), E = M.end(); I != E;) {
Function *F = I++;
if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage() &&
- F->isWeakForLinker() && !IsThunk(F)) {
+ F->mayBeOverridden() && !IsThunk(F)) {
ComparableFunction CF = ComparableFunction(F, TD);
LocalChanged |= Insert(FnSet, CF);
}
}
+ DEBUG(dbgs() << "size of FnSet: " << FnSet.size() << '\n');
Changed |= LocalChanged;
} while (LocalChanged);
More information about the llvm-commits
mailing list