[cfe-commits] r169639 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/objc-method-coverage.m

Anna Zaks ganna at apple.com
Fri Dec 7 13:51:48 PST 2012


Author: zaks
Date: Fri Dec  7 15:51:47 2012
New Revision: 169639

URL: http://llvm.org/viewvc/llvm-project?rev=169639&view=rev
Log:
[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.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
    cfe/trunk/test/Analysis/objc-method-coverage.m

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=169639&r1=169638&r2=169639&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri Dec  7 15:51:47 2012
@@ -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();
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=169639&r1=169638&r2=169639&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Dec  7 15:51:47 2012
@@ -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) {

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=169639&r1=169638&r2=169639&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Fri Dec  7 15:51:47 2012
@@ -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;
   }

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=169639&r1=169638&r2=169639&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Fri Dec  7 15:51:47 2012
@@ -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;
   }
 }

Modified: cfe/trunk/test/Analysis/objc-method-coverage.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-method-coverage.m?rev=169639&r1=169638&r2=169639&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objc-method-coverage.m (original)
+++ cfe/trunk/test/Analysis/objc-method-coverage.m Fri Dec  7 15:51:47 2012
@@ -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





More information about the cfe-commits mailing list