[PATCH] D147456: [AutoFDO] Stale profile matching(part 1)

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:1061
 
+  void setStaleProfileMappings(LocToLocMap *M) { StaleProfileMappings = M; }
+
----------------
drive-by nits:
- s/M/LTLM or basically anything that's not suggestive of "M == Module", which is seen everywhere else in the codebase
- can StaleProfileMappings be `const LocToLocMap`? (and same then for the parameter of this setter) - it would simplify the burden on a future reader/maintainer
- is `StaleProfileMappings` expected to be updated multiple times? if not, could you `assert(StaleProfileMappings == null && "this should be set only once") ` - same maintainability argument.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:483
+  void runOnFunction(const Function &F, const FunctionSamples &FS);
+  void setStaleProfileMappings();
+  void setStaleProfileMappings(FunctionSamples &FS);
----------------
drive-by nit: could this be called `distributeStaleProfileMappings`?


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:484
+  void setStaleProfileMappings();
+  void setStaleProfileMappings(FunctionSamples &FS);
+  void
----------------
do these need to be public? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147456