[PATCH] D82919: [SampleFDO] Enable sample-profile-top-down-load by default.

[Wei Mi via Phabricator via llvm-commits]

wmi created this revision.
wmi added reviewers: wenlei, davidxl.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

sample-profile-top-down-load is an internal option which can enable top-down order of inlining and profile annotation in sample profile load pass. It was found to be beneficial for better profile annotation.

      

Recently we found it could also solve some build time issue. Suppose function A has many callsites in function B. In the last release binary where sample profile was collected, the outline copy of A is large because there are many other functions inlined into A. However although all the callsites calling A in B are inlined, but every inlined body is small (A was inlined into B before other functions are inlined into A), there is no build time issue in last release.

      

In an optimized build using the sample profile collected from last release, without top-down inlining, we saw a case that A got very large because of inlining, and then multiple callsites of A got inlined into B, and that led to a huge B which caused significant build time issue besides profile annotation issue.

      

To solve that problem, the patch proposes to enable the flag sample-profile-top-down-load by default.

      

I reevaluated the performance again in two server benchmarks. Run one benchmark 6 times, it had no performance change in 4 runs and had 0.2% improvement in 2 runs. Run another benchmark 6 times and it had no performance change.


Repository:
  rL LLVM

https://reviews.llvm.org/D82919

Files:
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/test/Transforms/SampleProfile/inline-topdown.ll


Index: llvm/test/Transforms/SampleProfile/inline-topdown.ll
===================================================================
--- llvm/test/Transforms/SampleProfile/inline-topdown.ll
+++ llvm/test/Transforms/SampleProfile/inline-topdown.ll
@@ -1,10 +1,10 @@
 ; Note that this needs new pass manager for now. Passing `-sample-profile-top-down-load` to legacy pass manager is a no-op.
 
 ; Test we aren't doing specialization for inlining with default source order
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-topdown.prof -S | FileCheck -check-prefix=DEFAULT %s
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-topdown.prof -sample-profile-top-down-load=false -S | FileCheck -check-prefix=DEFAULT %s
 
 ; Test we specialize based on call path with context-sensitive profile while inlining with '-sample-profile-top-down-load'
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-topdown.prof -sample-profile-merge-inlinee -sample-profile-top-down-load -S | FileCheck -check-prefix=TOPDOWN  %s
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-topdown.prof -sample-profile-merge-inlinee -sample-profile-top-down-load=true -S | FileCheck -check-prefix=TOPDOWN  %s
 
 
 @.str = private unnamed_addr constant [11 x i8] c"sum is %d\0A\00", align 1
Index: llvm/lib/Transforms/IPO/SampleProfile.cpp
===================================================================
--- llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -153,7 +153,7 @@
              "profile loader decided not to inline a call site."));
 
 static cl::opt<bool> ProfileTopDownLoad(
-    "sample-profile-top-down-load", cl::Hidden, cl::init(false),
+    "sample-profile-top-down-load", cl::Hidden, cl::init(true),
     cl::desc("Do profile annotation and inlining for functions in top-down "
              "order of call graph during sample profile loading."));
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82919.274629.patch
Type: text/x-patch
Size: 1982 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200630/5d7398d1/attachment.bin>