[Analyzer] Add -analyzer-config option for function size the inliner considers as large

[seaneveson]

Summary

Dear All,

I would like to propose a small patch to add an option (-analyzer-config min-blocks-for-inline-large=14) to control the function size the inliner considers as large, in relation to "max-times-inline-large". In my patch the option defaults to the original hard coded behaviour, which I believe should be adjustable with the other inlining settings.

The analyzer-config test has been modified so that the analyzer will reach the getMinBlocksForInlineLarge() method and store the result in the ConfigTable, to ensure it is dumped by the debug checker.

Regards,

Sean Eveson
SN Systems - Sony Computer Entertainment Group

[seaneveson]

Ping

[krememek]

This looks fine, but I'm not a fan of the actual name chosen here. It's not very clear what it means by just looking at the name, and as we grow the number of configuration options that will become more important.

A few suggestions:

• min-block-size-treat-functions-as-large
• min-cfg-size-treat-functions-as-large (since "blocks" is a bit ambiguous)
• min-cfg-size-categorize-functions-as-large

Unrelated to this patch, several of our other options suffer from my same concerns, e.g.: max-times-inline-large, max-inclinable-size, but changing those are not the focus of this patch.

Other than the name, this looks good to me. Whatever name we choose, I think the method name should also be appropriately named, e.g.:

• getMinCFGSizeToTreatFunctionsAsLarge()

[seaneveson]

I changed the name to min-cfg-size-treat-functions-as-large, thanks for the suggestion.

If the patch is acceptable can someone commit it for me?

[dcoughlin]

LGTM. I will commit. Thanks Sean!

[krememek]

LGTM as well. Thanks Sean.

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

private:
   Optional<bool> StableReportFilename;
 
   /// \sa getGraphTrimInterval
   Optional<unsigned> GraphTrimInterval;
 
   /// \sa getMaxTimesInlineLarge
   Optional<unsigned> MaxTimesInlineLarge;
 
+  /// \sa getMinCFGSizeTreatFunctionsAsLarge
+  Optional<unsigned> MinCFGSizeTreatFunctionsAsLarge;
+
   /// \sa getMaxNodesPerTopLevelFunction
   Optional<unsigned> MaxNodesPerTopLevelFunction;
 
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
   /// option.
   /// Example:
public:
   /// node reclamation, set the option to "0".
   unsigned getGraphTrimInterval();
 
   /// Returns the maximum times a large function could be inlined.
   ///
   /// This is controlled by the 'max-times-inline-large' config option.
   unsigned getMaxTimesInlineLarge();
 
+  /// Returns the number of basic blocks a function needs to have to be
+  /// considered large for the 'max-times-inline-large' config option.
+  ///
+  /// This is controlled by the 'min-cfg-size-treat-functions-as-large' config
+  /// option.
+  unsigned getMinCFGSizeTreatFunctionsAsLarge();
+
   /// Returns the maximum number of nodes the analyzer can generate while
   /// exploring a top level function (for each exploded graph).
   /// 150000 is default; 0 means no limit.
   ///
   /// This is controlled by the 'max-nodes' config option.
   unsigned getMaxNodesPerTopLevelFunction();
 
 public:

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

 }
 
 unsigned AnalyzerOptions::getMaxTimesInlineLarge() {
   if (!MaxTimesInlineLarge.hasValue())
     MaxTimesInlineLarge = getOptionAsInteger("max-times-inline-large", 32);
   return MaxTimesInlineLarge.getValue();
 }
 
+unsigned AnalyzerOptions::getMinCFGSizeTreatFunctionsAsLarge() {
+  if (!MinCFGSizeTreatFunctionsAsLarge.hasValue())
+    MinCFGSizeTreatFunctionsAsLarge = getOptionAsInteger(
+      "min-cfg-size-treat-functions-as-large", 14);
+  return MinCFGSizeTreatFunctionsAsLarge.getValue();
+}
+
 unsigned AnalyzerOptions::getMaxNodesPerTopLevelFunction() {
   if (!MaxNodesPerTopLevelFunction.hasValue()) {
     int DefaultValue = 0;
     UserModeKind HighLevelMode = getUserMode();
     switch (HighLevelMode) {
       default:
         llvm_unreachable("Invalid mode.");
       case UMK_Shallow:

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   if ((StackDepth >= Opts.InlineMaxStackDepth) &&
       ((CalleeCFG->getNumBlockIDs() > Opts.getAlwaysInlineSize())
        || IsRecursive))
     return false;
 
   // Do not inline large functions too many times.
   if ((Engine.FunctionSummaries->getNumTimesInlined(D) >
        Opts.getMaxTimesInlineLarge()) &&
-      CalleeCFG->getNumBlockIDs() > 13) {
+       CalleeCFG->getNumBlockIDs() >=
+       Opts.getMinCFGSizeTreatFunctionsAsLarge()) {
     NumReachedInlineCountMax++;
     return false;
   }
 
   if (HowToInline == Inline_Minimal &&
       (CalleeCFG->getNumBlockIDs() > Opts.getAlwaysInlineSize()
       || IsRecursive))
     return false;

test/Analysis/analyzer-config.c

-// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper > %t 2>&1
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
-void foo() { bar(); }
+void foo() { 
+  // Call bar 33 times so max-times-inline-large is met and
+  // min-blocks-for-inline-large is checked
+  for (int i = 0; i < 34; ++i) {
+    bar();
+  }
+}
 
 // CHECK: [config]
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-temporary-dtors = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 150000
 // CHECK-NEXT: max-times-inline-large = 32
+// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 12
+// CHECK-NEXT: num-entries = 13
 

test/Analysis/analyzer-config.cpp

-// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper > %t 2>&1
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
-void foo() { bar(); }
+void foo() { 
+  // Call bar 33 times so max-times-inline-large is met and
+  // min-blocks-for-inline-large is checked
+  for (int i = 0; i < 34; ++i) {
+    bar();
+  }
+}
 
 class Foo {
 public:
   void bar() {}
   void foo() { bar(); }
 };
 
 // CHECK: [config]
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-temporary-dtors = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 150000
 // CHECK-NEXT: max-times-inline-large = 32
+// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 17
+// CHECK-NEXT: num-entries = 18