[PATCH] D140908: [MemProf] Context disambiguation cloning pass [patch 1/3]

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/include/llvm/ADT/SetOperations.h:60
+template <class S1Ty, class S2Ty>
+S1Ty set_intersection(const S1Ty &S1, const S2Ty &S2) {
+   if (S1.size() < S2.size())
----------------
Keep the name of the interface 'set_intersect'. To allow overloading, make the return a reference parameter.


================
Comment at: llvm/include/llvm/ADT/SetOperations.h:93
+template <class S1Ty, class S2Ty>
+S1Ty set_subtract_return_removed_remaining(S1Ty &S1, const S2Ty &S2,
+                                           S1Ty &Remaining) {
----------------
nit: it feels better to keep the name of the interface  set_subtract.  For symmetry, make the 'removed' a reference parameter.


================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:318
 
+inline raw_ostream &operator<<(raw_ostream &OS, const CallsiteInfo &SNI) {
+  OS << "Callee: " << SNI.Callee;
----------------
dumping utilities can be split out. Similarly for new interfaces like 'last', or set_subtract .


================
Comment at: llvm/test/Transforms/PGHOContextDisambiguation/basic.ll:29
+;;
+;; Code compiled with -mllvm -memprof-min-lifetime-cold-threshold=5 so that the
+;; memory freed after sleep(10) results in cold lifetimes.
----------------
why is the option called '-lifetime-cold-'? should it be '-lifetime-short-'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140908