[PATCH] D107173: [CSSPGO] Introduce MD5-based context-sensitive profile

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:222
+    if (!Context.isBaseContext()) {
+      auto Hash = MD5Hash(Context.getContextInMd5());
+      FuncToCtxtProfiles[Context.getGUID()].emplace(Hash, FSamples);
----------------
hoy wrote:
> The ordering based `Hash` in `FuncToCtxtProfiles` is mainly to achieve a consistent context promotion between md5 and non-md5 profiles which in turn gives a consistent codegen. However it is expansive. I tried sorting by the the combination of total sample counts and head sample counts, but still could not get every case covered. I think we might want to do this for non-md5 profile only, to favor md5 performance.
On the cost of hashing strings, if we using a std::set<FunctionSamples *, ..>, with comparer that checks total samples order and md5 string order sequentially, we would have stable order, with low cost as md5 string order is just a fall back that's rarely used, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107173