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

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 23 10:28:41 PDT 2019


wmi marked an inline comment as 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))
----------------
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.  


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67561





More information about the llvm-commits mailing list