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

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/ProfileData/SampleProfReader.h:334
+  /// Get the pointer of name table which includes all the names used in
+  /// FunctionSamples.
+  virtual std::vector<StringRef> *getNameTable() { return nullptr; }
----------------
Make it clearer that it includes all the names that have samples either in outline instance or an inline instance. It is a subset of the symbol list of the binary.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:481
+/// inlining to happen for warm callsites and it is helpful for performance.
 static bool callsiteIsHot(const FunctionSamples *CallsiteFS,
+                          ProfileSummaryInfo *PSI, bool ProfSampleAccEnabled) {
----------------
Make this a member function to avoid passing the boolean parameter around.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:1616
 
+  if (SampleProfileRecordCoverage || SampleProfileSampleCoverage)
+    CoverageTracker.setProfSampleAccurateEnabled();
----------------
what are these two flags?


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:1792
+    // actually not be cold after current build.
+    StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
+    if (NamesInProfile.count(CanonName))
----------------
If this is no symbol list, then it should not use this heuristic to be consistent with old behavior? 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67561