[PATCH] D149124: [llvm-profdata] ProfileReader cleanup - preparation for MD5 refactoring - 3

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:768
+  // (e.g. [A, A:1 @ B, A:1 @ B:2.3 @ C] [D, D:1 @ E]), so that when a match
+  // in the module is found, all prefixes of the matched function can be
+  // loaded, a list is needed to preserve that order.
----------------
wenlei wrote:
> > all prefixes of the matched function 
> 
> You meant "all function profile with matching prefix/caller"?
This comment is still inaccurate. The idea is to order the profiles so all profile sharing the same context prefix can be loaded together. i.e. If [A:1 @ B] is loaded, then [A:1 @ B:2 @ C], [A:1 @ B:3 @ D], [A:1 @ B:3 @ D:4 @ E] can all be loaded easily. 

> the profiles of all its prefix contexts (all callers on the call stack) are loaded

It's about loading callee profiles with matching caller context/prefix, rather than loading caller profiles. 


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:773-776
+  // If the profile is MD5, use the map container to lookup functions in
+  // the module. A remapper has no use on MD5 names.
+  if (useMD5())
+    return false;
----------------
huangjd wrote:
> wenlei wrote:
> > Do we need a separate entry for this? Or merge it with the `return false` in the end?
> I am keeping the logic in this order so that it is consistent with the control flow in readFuncProfiles because it is very complicated. 
sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149124