[PATCH] D67561: [SampleFDO] minimize performance impact when profile-sample-accurate is enabled

[Wei Mi via Phabricator via llvm-commits]

wmi marked an inline comment as done and an inline comment as not done.
wmi added inline comments.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:1792
+    // actually not be cold after current build.
+    StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
+    if (NamesInProfile.count(CanonName))
----------------
wmi wrote:
> davidxl wrote:
> > wmi wrote:
> > > davidxl wrote:
> > > > If this is no symbol list, then it should not use this heuristic to be consistent with old behavior? 
> > > I think it is beneficial to have the change when profile-sample-accurate is true no matter symbol list is there or not. symbol list is used to prevent new hot functions from being treated as cold.  NamesInProfile is used to prevent functions are mistakenly treated as cold because inlining difference between sampled binary and the IR after early inlining in current build. These two changes are orthogonal to each other and are both beneficial individually. 
> > Keeping the old behavior without Symbol list may be able to fix Chrome's size regression.
> I am evaluating whether we need to do it for both changes: using NamesInProfile and adjusting threshold in callsiteIsHot. I feel maybe not using NamesInProfile without symbol list is enough to remove the size regression. I will evaluate and have a patch for it.  
For my experiment, adjusting threshold in callsiteIsHot only increases size by 0.09% so most of the size increase comes from treating names in NamesInProfile as not cold.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67561/new/

https://reviews.llvm.org/D67561