From 35d21021337fc3dba82155c7232c2c5277b73883 Mon Sep 17 00:00:00 2001 From: Evgeniy Stepanov Date: Thu, 8 Aug 2013 08:22:39 +0000 Subject: Disable inlining between sanitized and non-sanitized functions. Inlining between functions with different values of sanitize_* attributes leads to over- or under-sanitizing, which is always bad. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@187967 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/InlineCost.cpp | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/InlineCost.cpp b/lib/Analysis/IPA/InlineCost.cpp index 37d73a8..89dcd81 100644 --- a/lib/Analysis/IPA/InlineCost.cpp +++ b/lib/Analysis/IPA/InlineCost.cpp @@ -1171,6 +1171,22 @@ InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, int Threshold) { return getInlineCost(CS, CS.getCalledFunction(), Threshold); } +/// \brief Test that two functions either have or have not the given attribute +/// at the same time. +static bool attributeMatches(Function *F1, Function *F2, + Attribute::AttrKind Attr) { + return F1->hasFnAttribute(Attr) == F2->hasFnAttribute(Attr); +} + +/// \brief Test that there are no attribute conflicts between Caller and Callee +/// that prevent inlining. +static bool functionsHaveCompatibleAttributes(Function *Caller, + Function *Callee) { + return attributeMatches(Caller, Callee, Attribute::SanitizeAddress) && + attributeMatches(Caller, Callee, Attribute::SanitizeMemory) && + attributeMatches(Caller, Callee, Attribute::SanitizeThread); +} + InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee, int Threshold) { // Cannot inline indirect calls. @@ -1179,20 +1195,22 @@ InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee, // Calls to functions with always-inline attributes should be inlined // whenever possible. - if (Callee->getAttributes().hasAttribute(AttributeSet::FunctionIndex, - Attribute::AlwaysInline)) { + if (Callee->hasFnAttribute(Attribute::AlwaysInline)) { if (isInlineViable(*Callee)) return llvm::InlineCost::getAlways(); return llvm::InlineCost::getNever(); } + // Never inline functions with conflicting attributes (unless callee has + // always-inline attribute). + if (!functionsHaveCompatibleAttributes(CS.getCaller(), Callee)) + return llvm::InlineCost::getNever(); + // Don't inline functions which can be redefined at link-time to mean // something else. Don't inline functions marked noinline or call sites // marked noinline. if (Callee->mayBeOverridden() || - Callee->getAttributes().hasAttribute(AttributeSet::FunctionIndex, - Attribute::NoInline) || - CS.isNoInline()) + Callee->hasFnAttribute(Attribute::NoInline) || CS.isNoInline()) return llvm::InlineCost::getNever(); DEBUG(llvm::dbgs() << " Analyzing call of " << Callee->getName() -- cgit v1.1 From 6e1c511aba87ab5f9962ae9328014897459f777a Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Fri, 25 Oct 2013 15:01:34 +0000 Subject: Call destroy from ~BasicCallGraph. This fix a memory leak found by valgrind. Calling it from the base class destructor would not destroy the BasicCallGraph bits. FIXME: BasicCallGraph is the only thing that inherits from CallGraph. Can we merge the two? git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@193412 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/CallGraph.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/CallGraph.cpp b/lib/Analysis/IPA/CallGraph.cpp index 7620fd9..6c18d0d 100644 --- a/lib/Analysis/IPA/CallGraph.cpp +++ b/lib/Analysis/IPA/CallGraph.cpp @@ -46,12 +46,16 @@ public: ExternalCallingNode(0), CallsExternalNode(0) { initializeBasicCallGraphPass(*PassRegistry::getPassRegistry()); } + ~BasicCallGraph() { + destroy(); + } // runOnModule - Compute the call graph for the specified module. virtual bool runOnModule(Module &M) { CallGraph::initialize(M); ExternalCallingNode = getOrInsertFunction(0); + assert(!CallsExternalNode); CallsExternalNode = new CallGraphNode(0); Root = 0; -- cgit v1.1 From c143c7573bfd0d55cf283cc2676dbd852f939c87 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Thu, 31 Oct 2013 03:03:55 +0000 Subject: Merge CallGraph and BasicCallGraph. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@193734 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/CallGraph.cpp | 234 +++++++++++++------------------------ lib/Analysis/IPA/GlobalsModRef.cpp | 2 +- lib/Analysis/IPA/IPA.cpp | 3 +- 3 files changed, 80 insertions(+), 159 deletions(-) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/CallGraph.cpp b/lib/Analysis/IPA/CallGraph.cpp index 6c18d0d..f042964 100644 --- a/lib/Analysis/IPA/CallGraph.cpp +++ b/lib/Analysis/IPA/CallGraph.cpp @@ -6,11 +6,6 @@ // License. See LICENSE.TXT for details. // //===----------------------------------------------------------------------===// -// -// This file implements the CallGraph class and provides the BasicCallGraph -// default implementation. -// -//===----------------------------------------------------------------------===// #include "llvm/Analysis/CallGraph.h" #include "llvm/IR/Instructions.h" @@ -21,172 +16,92 @@ #include "llvm/Support/raw_ostream.h" using namespace llvm; -namespace { - -//===----------------------------------------------------------------------===// -// BasicCallGraph class definition -// -class BasicCallGraph : public ModulePass, public CallGraph { - // Root is root of the call graph, or the external node if a 'main' function - // couldn't be found. - // - CallGraphNode *Root; - - // ExternalCallingNode - This node has edges to all external functions and - // those internal functions that have their address taken. - CallGraphNode *ExternalCallingNode; - - // CallsExternalNode - This node has edges to it from all functions making - // indirect calls or calling an external function. - CallGraphNode *CallsExternalNode; - -public: - static char ID; // Class identification, replacement for typeinfo - BasicCallGraph() : ModulePass(ID), Root(0), - ExternalCallingNode(0), CallsExternalNode(0) { - initializeBasicCallGraphPass(*PassRegistry::getPassRegistry()); - } - ~BasicCallGraph() { - destroy(); - } +CallGraph::CallGraph() + : ModulePass(ID), Root(0), ExternalCallingNode(0), CallsExternalNode(0) { + initializeCallGraphPass(*PassRegistry::getPassRegistry()); +} - // runOnModule - Compute the call graph for the specified module. - virtual bool runOnModule(Module &M) { - CallGraph::initialize(M); - - ExternalCallingNode = getOrInsertFunction(0); - assert(!CallsExternalNode); - CallsExternalNode = new CallGraphNode(0); - Root = 0; - - // Add every function to the call graph. - for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I) - addToCallGraph(I); - - // If we didn't find a main function, use the external call graph node - if (Root == 0) Root = ExternalCallingNode; - - return false; - } +void CallGraph::addToCallGraph(Function *F) { + CallGraphNode *Node = getOrInsertFunction(F); - virtual void getAnalysisUsage(AnalysisUsage &AU) const { - AU.setPreservesAll(); - } + // If this function has external linkage, anything could call it. + if (!F->hasLocalLinkage()) { + ExternalCallingNode->addCalledFunction(CallSite(), Node); - virtual void print(raw_ostream &OS, const Module *) const { - OS << "CallGraph Root is: "; - if (Function *F = getRoot()->getFunction()) - OS << F->getName() << "\n"; - else { - OS << "<>\n"; + // Found the entry point? + if (F->getName() == "main") { + if (Root) // Found multiple external mains? Don't pick one. + Root = ExternalCallingNode; + else + Root = Node; // Found a main, keep track of it! } - - CallGraph::print(OS, 0); } - virtual void releaseMemory() { - destroy(); - } - - /// getAdjustedAnalysisPointer - This method is used when a pass implements - /// an analysis interface through multiple inheritance. If needed, it should - /// override this to adjust the this pointer as needed for the specified pass - /// info. - virtual void *getAdjustedAnalysisPointer(AnalysisID PI) { - if (PI == &CallGraph::ID) - return (CallGraph*)this; - return this; - } - - CallGraphNode* getExternalCallingNode() const { return ExternalCallingNode; } - CallGraphNode* getCallsExternalNode() const { return CallsExternalNode; } - - // getRoot - Return the root of the call graph, which is either main, or if - // main cannot be found, the external node. - // - CallGraphNode *getRoot() { return Root; } - const CallGraphNode *getRoot() const { return Root; } - -private: - //===--------------------------------------------------------------------- - // Implementation of CallGraph construction - // - - // addToCallGraph - Add a function to the call graph, and link the node to all - // of the functions that it calls. - // - void addToCallGraph(Function *F) { - CallGraphNode *Node = getOrInsertFunction(F); - - // If this function has external linkage, anything could call it. - if (!F->hasLocalLinkage()) { - ExternalCallingNode->addCalledFunction(CallSite(), Node); - - // Found the entry point? - if (F->getName() == "main") { - if (Root) // Found multiple external mains? Don't pick one. - Root = ExternalCallingNode; - else - Root = Node; // Found a main, keep track of it! + // If this function has its address taken, anything could call it. + if (F->hasAddressTaken()) + ExternalCallingNode->addCalledFunction(CallSite(), Node); + + // If this function is not defined in this translation unit, it could call + // anything. + if (F->isDeclaration() && !F->isIntrinsic()) + Node->addCalledFunction(CallSite(), CallsExternalNode); + + // Look for calls by this function. + for (Function::iterator BB = F->begin(), BBE = F->end(); BB != BBE; ++BB) + for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE; + ++II) { + CallSite CS(cast(II)); + if (CS) { + const Function *Callee = CS.getCalledFunction(); + if (!Callee) + // Indirect calls of intrinsics are not allowed so no need to check. + Node->addCalledFunction(CS, CallsExternalNode); + else if (!Callee->isIntrinsic()) + Node->addCalledFunction(CS, getOrInsertFunction(Callee)); } } +} - // If this function has its address taken, anything could call it. - if (F->hasAddressTaken()) - ExternalCallingNode->addCalledFunction(CallSite(), Node); - - // If this function is not defined in this translation unit, it could call - // anything. - if (F->isDeclaration() && !F->isIntrinsic()) - Node->addCalledFunction(CallSite(), CallsExternalNode); - - // Look for calls by this function. - for (Function::iterator BB = F->begin(), BBE = F->end(); BB != BBE; ++BB) - for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); - II != IE; ++II) { - CallSite CS(cast(II)); - if (CS) { - const Function *Callee = CS.getCalledFunction(); - if (!Callee) - // Indirect calls of intrinsics are not allowed so no need to check. - Node->addCalledFunction(CS, CallsExternalNode); - else if (!Callee->isIntrinsic()) - Node->addCalledFunction(CS, getOrInsertFunction(Callee)); - } - } - } +void CallGraph::getAnalysisUsage(AnalysisUsage &AU) const { + AU.setPreservesAll(); +} - // - // destroy - Release memory for the call graph - virtual void destroy() { - /// CallsExternalNode is not in the function map, delete it explicitly. - if (CallsExternalNode) { - CallsExternalNode->allReferencesDropped(); - delete CallsExternalNode; - CallsExternalNode = 0; - } - CallGraph::destroy(); - } -}; +bool CallGraph::runOnModule(Module &M) { + Mod = &M; -} //End anonymous namespace + ExternalCallingNode = getOrInsertFunction(0); + assert(!CallsExternalNode); + CallsExternalNode = new CallGraphNode(0); + Root = 0; -INITIALIZE_ANALYSIS_GROUP(CallGraph, "Call Graph", BasicCallGraph) -INITIALIZE_AG_PASS(BasicCallGraph, CallGraph, "basiccg", - "Basic CallGraph Construction", false, true, true) + // Add every function to the call graph. + for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I) + addToCallGraph(I); -char CallGraph::ID = 0; -char BasicCallGraph::ID = 0; + // If we didn't find a main function, use the external call graph node + if (Root == 0) + Root = ExternalCallingNode; -void CallGraph::initialize(Module &M) { - Mod = &M; + return false; } -void CallGraph::destroy() { - if (FunctionMap.empty()) return; - - // Reset all node's use counts to zero before deleting them to prevent an - // assertion from firing. +INITIALIZE_PASS(CallGraph, "basiccg", "CallGraph Construction", false, true) + +char CallGraph::ID = 0; + +void CallGraph::releaseMemory() { + /// CallsExternalNode is not in the function map, delete it explicitly. + if (CallsExternalNode) { + CallsExternalNode->allReferencesDropped(); + delete CallsExternalNode; + CallsExternalNode = 0; + } + + if (FunctionMap.empty()) + return; + +// Reset all node's use counts to zero before deleting them to prevent an +// assertion from firing. #ifndef NDEBUG for (FunctionMapTy::iterator I = FunctionMap.begin(), E = FunctionMap.end(); I != E; ++I) @@ -199,7 +114,14 @@ void CallGraph::destroy() { FunctionMap.clear(); } -void CallGraph::print(raw_ostream &OS, Module*) const { +void CallGraph::print(raw_ostream &OS, const Module*) const { + OS << "CallGraph Root is: "; + if (Function *F = Root->getFunction()) + OS << F->getName() << "\n"; + else { + OS << "<>\n"; + } + for (CallGraph::const_iterator I = begin(), E = end(); I != E; ++I) I->second->print(OS); } diff --git a/lib/Analysis/IPA/GlobalsModRef.cpp b/lib/Analysis/IPA/GlobalsModRef.cpp index 92d0d23..7ec4644 100644 --- a/lib/Analysis/IPA/GlobalsModRef.cpp +++ b/lib/Analysis/IPA/GlobalsModRef.cpp @@ -189,7 +189,7 @@ char GlobalsModRef::ID = 0; INITIALIZE_AG_PASS_BEGIN(GlobalsModRef, AliasAnalysis, "globalsmodref-aa", "Simple mod/ref analysis for globals", false, true, false) -INITIALIZE_AG_DEPENDENCY(CallGraph) +INITIALIZE_PASS_DEPENDENCY(CallGraph) INITIALIZE_AG_PASS_END(GlobalsModRef, AliasAnalysis, "globalsmodref-aa", "Simple mod/ref analysis for globals", false, true, false) diff --git a/lib/Analysis/IPA/IPA.cpp b/lib/Analysis/IPA/IPA.cpp index 1c1816d..47357cf 100644 --- a/lib/Analysis/IPA/IPA.cpp +++ b/lib/Analysis/IPA/IPA.cpp @@ -19,8 +19,7 @@ using namespace llvm; /// initializeIPA - Initialize all passes linked into the IPA library. void llvm::initializeIPA(PassRegistry &Registry) { - initializeBasicCallGraphPass(Registry); - initializeCallGraphAnalysisGroup(Registry); + initializeCallGraphPass(Registry); initializeCallGraphPrinterPass(Registry); initializeCallGraphViewerPass(Registry); initializeFindUsedTypesPass(Registry); -- cgit v1.1 From 49837ef8111fbeace7ae6379ca733c8f8fa94cfe Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Sat, 9 Nov 2013 12:26:54 +0000 Subject: Move the old pass manager infrastructure into a legacy namespace and give the files a legacy prefix in the right directory. Use forwarding headers in the old locations to paper over the name change for most clients during the transitional period. No functionality changed here! This is just clearing some space to reduce renaming churn later on with a new system. Even when the new stuff starts to go in, it is going to be hidden behind a flag and off-by-default as it is still WIP and under development. This patch is specifically designed so that very little out-of-tree code has to change. I'm going to work as hard as I can to keep that the case. Only direct forward declarations of the PassManager class are impacted by this change. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@194324 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/CallGraphSCCPass.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/CallGraphSCCPass.cpp b/lib/Analysis/IPA/CallGraphSCCPass.cpp index a0d788f..182beca 100644 --- a/lib/Analysis/IPA/CallGraphSCCPass.cpp +++ b/lib/Analysis/IPA/CallGraphSCCPass.cpp @@ -22,7 +22,7 @@ #include "llvm/Analysis/CallGraph.h" #include "llvm/IR/Function.h" #include "llvm/IR/IntrinsicInst.h" -#include "llvm/PassManagers.h" +#include "llvm/IR/LegacyPassManagers.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Timer.h" -- cgit v1.1 From fe45fd084db872f9c7106c26e52c1cc8c9cba3a5 Mon Sep 17 00:00:00 2001 From: Paul Robinson Date: Mon, 18 Nov 2013 21:44:03 +0000 Subject: The 'optnone' attribute means don't inline anything into this function (except functions marked always_inline). Functions with 'optnone' must also have 'noinline' so they don't get inlined into any other function. Based on work by Andrea Di Biagio. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@195046 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/InlineCost.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/InlineCost.cpp b/lib/Analysis/IPA/InlineCost.cpp index 89dcd81..013691b 100644 --- a/lib/Analysis/IPA/InlineCost.cpp +++ b/lib/Analysis/IPA/InlineCost.cpp @@ -1206,6 +1206,10 @@ InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee, if (!functionsHaveCompatibleAttributes(CS.getCaller(), Callee)) return llvm::InlineCost::getNever(); + // Don't inline this call if the caller has the optnone attribute. + if (CS.getCaller()->hasFnAttribute(Attribute::OptimizeNone)) + return llvm::InlineCost::getNever(); + // Don't inline functions which can be redefined at link-time to mean // something else. Don't inline functions marked noinline or call sites // marked noinline. -- cgit v1.1 From e6725194d1045eeb5a9371316120b9b027f3d289 Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Sun, 15 Dec 2013 20:54:53 +0000 Subject: Merging r197215: ------------------------------------------------------------------------ r197215 | chandlerc | 2013-12-12 23:59:56 -0800 (Thu, 12 Dec 2013) | 24 lines [inliner] Completely change (and fix) how the inline cost analysis handles terminator instructions. The inline cost analysis inheritted some pretty rough handling of terminator insts from the original cost analysis, and then made it much, much worse by factoring all of the important analyses into a separate instruction visitor. That instruction visitor never visited the terminator. This works fine for things like conditional branches, but for many other things we simply computed The Wrong Value. First example are unconditional branches, which should be free but were counted as full cost. This is most significant for conditional branches where the condition simplifies and folds during inlining. We paid a 1 instruction tax on every branch in a straight line specialized path. =[ Oh, we also claimed that the unreachable instruction had cost. But it gets worse. Let's consider invoke. We never applied the call penalty. We never accounted for the cost of the arguments. Nope. Worse still, we didn't handle the *correctness* constraints of not inlining recursive invokes, or exception throwing returns_twice functions. Oops. See PR18206. Sadly, PR18206 requires yet another fix, but this refactoring is at least a huge step in that direction. ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_34@197351 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/InlineCost.cpp | 113 +++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 37 deletions(-) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/InlineCost.cpp b/lib/Analysis/IPA/InlineCost.cpp index 013691b..5e6d72d 100644 --- a/lib/Analysis/IPA/InlineCost.cpp +++ b/lib/Analysis/IPA/InlineCost.cpp @@ -59,6 +59,8 @@ class CallAnalyzer : public InstVisitor { bool ExposesReturnsTwice; bool HasDynamicAlloca; bool ContainsNoDuplicateCall; + bool HasReturn; + bool HasIndirectBr; /// Number of bytes allocated statically by the callee. uint64_t AllocatedSize; @@ -132,6 +134,12 @@ class CallAnalyzer : public InstVisitor { bool visitExtractValue(ExtractValueInst &I); bool visitInsertValue(InsertValueInst &I); bool visitCallSite(CallSite CS); + bool visitReturnInst(ReturnInst &RI); + bool visitBranchInst(BranchInst &BI); + bool visitSwitchInst(SwitchInst &SI); + bool visitIndirectBrInst(IndirectBrInst &IBI); + bool visitResumeInst(ResumeInst &RI); + bool visitUnreachableInst(UnreachableInst &I); public: CallAnalyzer(const DataLayout *TD, const TargetTransformInfo &TTI, @@ -139,12 +147,13 @@ public: : TD(TD), TTI(TTI), F(Callee), Threshold(Threshold), Cost(0), IsCallerRecursive(false), IsRecursiveCall(false), ExposesReturnsTwice(false), HasDynamicAlloca(false), - ContainsNoDuplicateCall(false), AllocatedSize(0), NumInstructions(0), - NumVectorInstructions(0), FiftyPercentVectorBonus(0), - TenPercentVectorBonus(0), VectorBonus(0), NumConstantArgs(0), - NumConstantOffsetPtrArgs(0), NumAllocaArgs(0), NumConstantPtrCmps(0), - NumConstantPtrDiffs(0), NumInstructionsSimplified(0), - SROACostSavings(0), SROACostSavingsLost(0) {} + ContainsNoDuplicateCall(false), HasReturn(false), HasIndirectBr(false), + AllocatedSize(0), NumInstructions(0), NumVectorInstructions(0), + FiftyPercentVectorBonus(0), TenPercentVectorBonus(0), VectorBonus(0), + NumConstantArgs(0), NumConstantOffsetPtrArgs(0), NumAllocaArgs(0), + NumConstantPtrCmps(0), NumConstantPtrDiffs(0), + NumInstructionsSimplified(0), SROACostSavings(0), + SROACostSavingsLost(0) {} bool analyzeCall(CallSite CS); @@ -785,6 +794,60 @@ bool CallAnalyzer::visitCallSite(CallSite CS) { return Base::visitCallSite(CS); } +bool CallAnalyzer::visitReturnInst(ReturnInst &RI) { + // At least one return instruction will be free after inlining. + bool Free = !HasReturn; + HasReturn = true; + return Free; +} + +bool CallAnalyzer::visitBranchInst(BranchInst &BI) { + // We model unconditional branches as essentially free -- they really + // shouldn't exist at all, but handling them makes the behavior of the + // inliner more regular and predictable. Interestingly, conditional branches + // which will fold away are also free. + return BI.isUnconditional() || isa(BI.getCondition()) || + dyn_cast_or_null( + SimplifiedValues.lookup(BI.getCondition())); +} + +bool CallAnalyzer::visitSwitchInst(SwitchInst &SI) { + // We model unconditional switches as free, see the comments on handling + // branches. + return isa(SI.getCondition()) || + dyn_cast_or_null( + SimplifiedValues.lookup(SI.getCondition())); +} + +bool CallAnalyzer::visitIndirectBrInst(IndirectBrInst &IBI) { + // We never want to inline functions that contain an indirectbr. This is + // incorrect because all the blockaddress's (in static global initializers + // for example) would be referring to the original function, and this + // indirect jump would jump from the inlined copy of the function into the + // original function which is extremely undefined behavior. + // FIXME: This logic isn't really right; we can safely inline functions with + // indirectbr's as long as no other function or global references the + // blockaddress of a block within the current function. And as a QOI issue, + // if someone is using a blockaddress without an indirectbr, and that + // reference somehow ends up in another function or global, we probably don't + // want to inline this function. + HasIndirectBr = true; + return false; +} + +bool CallAnalyzer::visitResumeInst(ResumeInst &RI) { + // FIXME: It's not clear that a single instruction is an accurate model for + // the inline cost of a resume instruction. + return false; +} + +bool CallAnalyzer::visitUnreachableInst(UnreachableInst &I) { + // FIXME: It might be reasonably to discount the cost of instructions leading + // to unreachable as they have the lowest possible impact on both runtime and + // code size. + return true; // No actual code is needed for unreachable. +} + bool CallAnalyzer::visitInstruction(Instruction &I) { // Some instructions are free. All of the free intrinsics can also be // handled by SROA, etc. @@ -808,8 +871,7 @@ bool CallAnalyzer::visitInstruction(Instruction &I) { /// construct has been detected. It returns false if inlining is no longer /// viable, and true if inlining remains viable. bool CallAnalyzer::analyzeBlock(BasicBlock *BB) { - for (BasicBlock::iterator I = BB->begin(), E = llvm::prior(BB->end()); - I != E; ++I) { + for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) { ++NumInstructions; if (isa(I) || I->getType()->isVectorTy()) ++NumVectorInstructions; @@ -825,7 +887,8 @@ bool CallAnalyzer::analyzeBlock(BasicBlock *BB) { Cost += InlineConstants::InstrCost; // If the visit this instruction detected an uninlinable pattern, abort. - if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca) + if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca || + HasIndirectBr) return false; // If the caller is a recursive function then we don't want to inline @@ -989,10 +1052,6 @@ bool CallAnalyzer::analyzeCall(CallSite CS) { } } - // Track whether we've seen a return instruction. The first return - // instruction is free, as at least one will usually disappear in inlining. - bool HasReturn = false; - // Populate our simplified values by mapping from function arguments to call // arguments with known important simplifications. CallSite::arg_iterator CAI = CS.arg_begin(); @@ -1039,33 +1098,11 @@ bool CallAnalyzer::analyzeCall(CallSite CS) { if (BB->empty()) continue; - // Handle the terminator cost here where we can track returns and other - // function-wide constructs. - TerminatorInst *TI = BB->getTerminator(); - - // We never want to inline functions that contain an indirectbr. This is - // incorrect because all the blockaddress's (in static global initializers - // for example) would be referring to the original function, and this - // indirect jump would jump from the inlined copy of the function into the - // original function which is extremely undefined behavior. - // FIXME: This logic isn't really right; we can safely inline functions - // with indirectbr's as long as no other function or global references the - // blockaddress of a block within the current function. And as a QOI issue, - // if someone is using a blockaddress without an indirectbr, and that - // reference somehow ends up in another function or global, we probably - // don't want to inline this function. - if (isa(TI)) - return false; - - if (!HasReturn && isa(TI)) - HasReturn = true; - else - Cost += InlineConstants::InstrCost; - // Analyze the cost of this block. If we blow through the threshold, this // returns false, and we can bail on out. if (!analyzeBlock(BB)) { - if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca) + if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca || + HasIndirectBr) return false; // If the caller is a recursive function then we don't want to inline @@ -1078,6 +1115,8 @@ bool CallAnalyzer::analyzeCall(CallSite CS) { break; } + TerminatorInst *TI = BB->getTerminator(); + // Add in the live successors by first checking whether we have terminator // that may be simplified based on the values simplified by this call. if (BranchInst *BI = dyn_cast(TI)) { -- cgit v1.1 From b525888b1f988f6a993054285a3a43a24c26fbca Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Sun, 15 Dec 2013 20:55:09 +0000 Subject: Merging r197216: ------------------------------------------------------------------------ r197216 | chandlerc | 2013-12-13 00:00:01 -0800 (Fri, 13 Dec 2013) | 9 lines [inliner] Fix PR18206 by preventing inlining functions that call setjmp through an invoke instruction. The original patch for this was written by Mark Seaborn, but I've reworked his test case into the existing returns_twice test case and implemented the fix by the prior refactoring to actually run the cost analysis over invoke instructions, and then here fixing our detection of the returns_twice attribute to work for both calls and invokes. We never noticed because we never saw an invoke. =[ ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_34@197352 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/IPA/InlineCost.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Analysis/IPA') diff --git a/lib/Analysis/IPA/InlineCost.cpp b/lib/Analysis/IPA/InlineCost.cpp index 5e6d72d..3bc796e 100644 --- a/lib/Analysis/IPA/InlineCost.cpp +++ b/lib/Analysis/IPA/InlineCost.cpp @@ -713,7 +713,7 @@ bool CallAnalyzer::simplifyCallSite(Function *F, CallSite CS) { } bool CallAnalyzer::visitCallSite(CallSite CS) { - if (CS.isCall() && cast(CS.getInstruction())->canReturnTwice() && + if (CS.hasFnAttr(Attribute::ReturnsTwice) && !F.getAttributes().hasAttribute(AttributeSet::FunctionIndex, Attribute::ReturnsTwice)) { // This aborts the entire analysis. -- cgit v1.1