[analyzer] Optimization heuristic: do not reanalyze every ObjC method as
top level.
This heuristic is already turned on for non-ObjC methods
(inlining-mode=noredundancy). If a method has been previously analyzed,
while being inlined inside of another method, do not reanalyze it as top
level.
This commit applies it to ObjCMethods as well. The main caveat here is
that to catch the retain release errors, we are still going to reanalyze
all the ObjC methods but without inlining turned on.
Gives 21% performance increase on one heavy ObjC benchmark, which
suffered large performance regressions due to ObjC inlining.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169639 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index f4cb9a6..151b0d6 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -46,6 +46,16 @@
class SimpleCall;
class ExprEngine : public SubEngine {
+public:
+ /// The modes of inlining.
+ enum InliningModes {
+ /// Do not inline any of the callees.
+ Inline_None = 0,
+ /// Inline all callees.
+ Inline_All = 0x1
+ } ;
+
+private:
AnalysisManager &AMgr;
AnalysisDeclContextManager &AnalysisDeclContexts;
@@ -83,10 +93,14 @@
/// AnalysisConsumer. It can be null.
SetOfConstDecls *VisitedCallees;
+ /// The flag, which specifies the mode of inlining for the engine.
+ InliningModes HowToInline;
+
public:
ExprEngine(AnalysisManager &mgr, bool gcEnabled,
SetOfConstDecls *VisitedCalleesIn,
- FunctionSummariesTy *FS);
+ FunctionSummariesTy *FS,
+ InliningModes HowToInlineIn);
~ExprEngine();
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 2294519..2657573 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -56,7 +56,8 @@
ExprEngine::ExprEngine(AnalysisManager &mgr, bool gcEnabled,
SetOfConstDecls *VisitedCalleesIn,
- FunctionSummariesTy *FS)
+ FunctionSummariesTy *FS,
+ InliningModes HowToInlineIn)
: AMgr(mgr),
AnalysisDeclContexts(mgr.getAnalysisDeclContextManager()),
Engine(*this, FS),
@@ -69,7 +70,8 @@
currStmtIdx(0), currBldrCtx(0),
ObjCNoRet(mgr.getASTContext()),
ObjCGCEnabled(gcEnabled), BR(mgr, *this),
- VisitedCallees(VisitedCalleesIn)
+ VisitedCallees(VisitedCalleesIn),
+ HowToInline(HowToInlineIn)
{
unsigned TrimInterval = mgr.options.getGraphTrimInterval();
if (TrimInterval != 0) {
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 3ce4612..d654bb5 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -708,7 +708,7 @@
ProgramStateRef State = Pred->getState();
CallEventRef<> Call = CallTemplate.cloneWithState(State);
- if (!getAnalysisManager().shouldInlineCall()) {
+ if (HowToInline == Inline_None) {
conservativeEvalCall(*Call, Bldr, Pred, State);
return;
}
diff --git a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index cdabf81..1f5de91 100644
--- a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -52,7 +52,8 @@
static ExplodedNode::Auditor* CreateUbiViz();
STATISTIC(NumFunctionTopLevel, "The # of functions at top level.");
-STATISTIC(NumFunctionsAnalyzed, "The # of functions analysed (as top level).");
+STATISTIC(NumFunctionsAnalyzed,
+ "The # of functions and blocks analyzed (as top level).");
STATISTIC(NumBlocksInAnalyzedFunctions,
"The # of basic blocks in the analyzed functions.");
STATISTIC(PercentReachableBlocks, "The % of reachable basic blocks.");
@@ -266,6 +267,12 @@
virtual void HandleTranslationUnit(ASTContext &C);
+ /// \brief Determine which inlining mode should be used when this function is
+ /// analyzed. For example, determines if the callees should be inlined.
+ ExprEngine::InliningModes
+ getInliningModeForFunction(CallGraphNode *N,
+ SmallPtrSet<CallGraphNode*,24> Visited);
+
/// \brief Build the call graph for all the top level decls of this TU and
/// use it to define the order in which the functions should be visited.
void HandleDeclsCallGraph(const unsigned LocalTUDeclsSize);
@@ -277,10 +284,14 @@
/// set of functions which should be considered analyzed after analyzing the
/// given root function.
void HandleCode(Decl *D, AnalysisMode Mode,
+ ExprEngine::InliningModes IMode = ExprEngine::Inline_None,
SetOfConstDecls *VisitedCallees = 0);
- void RunPathSensitiveChecks(Decl *D, SetOfConstDecls *VisitedCallees);
+ void RunPathSensitiveChecks(Decl *D,
+ ExprEngine::InliningModes IMode,
+ SetOfConstDecls *VisitedCallees);
void ActionExprEngine(Decl *D, bool ObjCGCEnabled,
+ ExprEngine::InliningModes IMode,
SetOfConstDecls *VisitedCallees);
/// Visitors for the RecursiveASTVisitor.
@@ -303,14 +314,17 @@
// only determined when they are instantiated.
if (FD->isThisDeclarationADefinition() &&
!FD->isDependentContext()) {
+ assert(RecVisitorMode == AM_Syntax || Mgr->shouldInlineCall() == false);
HandleCode(FD, RecVisitorMode);
}
return true;
}
bool VisitObjCMethodDecl(ObjCMethodDecl *MD) {
- if (MD->isThisDeclarationADefinition())
+ if (MD->isThisDeclarationADefinition()) {
+ assert(RecVisitorMode == AM_Syntax || Mgr->shouldInlineCall() == false);
HandleCode(MD, RecVisitorMode);
+ }
return true;
}
@@ -351,12 +365,16 @@
}
static bool shouldSkipFunction(CallGraphNode *N,
- SmallPtrSet<CallGraphNode*,24> Visited) {
- // We want to re-analyse the functions as top level in several cases:
+ SmallPtrSet<CallGraphNode*,24> Visited,
+ SmallPtrSet<CallGraphNode*,24> VisitedAsTopLevel) {
+ if (VisitedAsTopLevel.count(N))
+ return true;
+
+ // We want to re-analyse the functions as top level in the following cases:
// - The 'init' methods should be reanalyzed because
// ObjCNonNilReturnValueChecker assumes that '[super init]' never returns
- // 'nil' and unless we analyze the 'init' functions as top level, we will not
- // catch errors within defensive code.
+ // 'nil' and unless we analyze the 'init' functions as top level, we will
+ // not catch errors within defensive code.
// - We want to reanalyze all ObjC methods as top level to report Retain
// Count naming convention errors more aggressively.
if (isa<ObjCMethodDecl>(N->getDecl()))
@@ -366,6 +384,27 @@
return Visited.count(N);
}
+ExprEngine::InliningModes
+AnalysisConsumer::getInliningModeForFunction(CallGraphNode *N,
+ SmallPtrSet<CallGraphNode*,24> Visited) {
+ ExprEngine::InliningModes HowToInline =
+ (Mgr->shouldInlineCall()) ? ExprEngine::Inline_All :
+ ExprEngine::Inline_None;
+
+ // We want to reanalyze all ObjC methods as top level to report Retain
+ // Count naming convention errors more aggressively. But we can turn off
+ // inlining when reanalyzing an already inlined function.
+ if (Visited.count(N)) {
+ assert(isa<ObjCMethodDecl>(N->getDecl()) &&
+ "We are only reanalyzing ObjCMethods.");
+ ObjCMethodDecl *ObjCM = cast<ObjCMethodDecl>(N->getDecl());
+ if (ObjCM->getMethodFamily() != OMF_init)
+ HowToInline = ExprEngine::Inline_None;
+ }
+
+ return HowToInline;
+}
+
void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
// Otherwise, use the Callgraph to derive the order.
// Build the Call Graph.
@@ -408,6 +447,7 @@
// the previously processed functions. Use external Visited set, which is
// also modified when we inline a function.
SmallPtrSet<CallGraphNode*,24> Visited;
+ SmallPtrSet<CallGraphNode*,24> VisitedAsTopLevel;
while(!BFSQueue.empty()) {
CallGraphNode *N = BFSQueue.front();
BFSQueue.pop_front();
@@ -415,20 +455,21 @@
// Push the children into the queue.
for (CallGraphNode::const_iterator CI = N->begin(),
CE = N->end(); CI != CE; ++CI) {
- if (!shouldSkipFunction(*CI, Visited))
+ if (!shouldSkipFunction(*CI, Visited, VisitedAsTopLevel))
BFSQueue.push_back(*CI);
}
// Skip the functions which have been processed already or previously
// inlined.
- if (shouldSkipFunction(N, Visited))
+ if (shouldSkipFunction(N, Visited, VisitedAsTopLevel))
continue;
// Analyze the function.
SetOfConstDecls VisitedCallees;
Decl *D = N->getDecl();
assert(D);
- HandleCode(D, AM_Path,
+
+ HandleCode(D, AM_Path, getInliningModeForFunction(N, Visited),
(Mgr->options.InliningMode == All ? 0 : &VisitedCallees));
// Add the visited callees to the global visited set.
@@ -438,7 +479,7 @@
if (VN)
Visited.insert(VN);
}
- Visited.insert(N);
+ VisitedAsTopLevel.insert(N);
}
}
@@ -546,6 +587,7 @@
}
void AnalysisConsumer::HandleCode(Decl *D, AnalysisMode Mode,
+ ExprEngine::InliningModes IMode,
SetOfConstDecls *VisitedCallees) {
Mode = getModeForDecl(D, Mode);
if (Mode == AM_None)
@@ -576,7 +618,7 @@
if (Mode & AM_Syntax)
checkerMgr->runCheckersOnASTBody(*WI, *Mgr, BR);
if ((Mode & AM_Path) && checkerMgr->hasPathSensitiveCheckers()) {
- RunPathSensitiveChecks(*WI, VisitedCallees);
+ RunPathSensitiveChecks(*WI, IMode, VisitedCallees);
NumFunctionsAnalyzed++;
}
}
@@ -587,6 +629,7 @@
//===----------------------------------------------------------------------===//
void AnalysisConsumer::ActionExprEngine(Decl *D, bool ObjCGCEnabled,
+ ExprEngine::InliningModes IMode,
SetOfConstDecls *VisitedCallees) {
// Construct the analysis engine. First check if the CFG is valid.
// FIXME: Inter-procedural analysis will need to handle invalid CFGs.
@@ -597,7 +640,7 @@
if (!Mgr->getAnalysisDeclContext(D)->getAnalysis<RelaxedLiveVariables>())
return;
- ExprEngine Eng(*Mgr, ObjCGCEnabled, VisitedCallees, &FunctionSummaries);
+ ExprEngine Eng(*Mgr, ObjCGCEnabled, VisitedCallees, &FunctionSummaries,IMode);
// Set the graph auditor.
OwningPtr<ExplodedNode::Auditor> Auditor;
@@ -623,20 +666,21 @@
}
void AnalysisConsumer::RunPathSensitiveChecks(Decl *D,
+ ExprEngine::InliningModes IMode,
SetOfConstDecls *Visited) {
switch (Mgr->getLangOpts().getGC()) {
case LangOptions::NonGC:
- ActionExprEngine(D, false, Visited);
+ ActionExprEngine(D, false, IMode, Visited);
break;
case LangOptions::GCOnly:
- ActionExprEngine(D, true, Visited);
+ ActionExprEngine(D, true, IMode, Visited);
break;
case LangOptions::HybridGC:
- ActionExprEngine(D, false, Visited);
- ActionExprEngine(D, true, Visited);
+ ActionExprEngine(D, false, IMode, Visited);
+ ActionExprEngine(D, true, IMode, Visited);
break;
}
}
diff --git a/test/Analysis/objc-method-coverage.m b/test/Analysis/objc-method-coverage.m
index 3088a29..50a4888 100644
--- a/test/Analysis/objc-method-coverage.m
+++ b/test/Analysis/objc-method-coverage.m
@@ -14,4 +14,4 @@
@end
// CHECK: ... Statistics Collected ...
-// CHECK: 2 AnalysisConsumer - The # of functions analysed (as top level).
\ No newline at end of file
+// CHECK: 2 AnalysisConsumer - The # of functions and blocks analyzed (as top level).
\ No newline at end of file
