[llvm] [SampleFDO] Read call-graph matching recovered top-level function profile (PR #101053)

Lei Wang via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 13 08:25:56 PDT 2024


================
@@ -782,6 +782,26 @@ bool SampleProfileMatcher::functionMatchesProfileHelper(
   float Similarity = 0.0;
 
   const auto *FSFlattened = getFlattenedSamplesFor(ProfFunc);
+  // Check if the function is top-level function. For extended profile format,
+  // if a function profile is unused and it's top-level, even if the profile is
+  // matched, it's not found in the profile. This is because sample reader only
+  // read the used profile at the beginning, we need to read the profile
+  // on-demand. Also save it into the FlattenedProfiles for future look-up.
+  if (!FSFlattened) {
+    DenseSet<StringRef> TopLevelFunc;
+    TopLevelFunc.insert(ProfFunc.stringRef());
+    SampleProfileMap TopLevelProfile;
+    Reader.readOnDemand(TopLevelFunc, TopLevelProfile);
----------------
wlei-llvm wrote:

This is the key part to load the top-level profile. If we don't load the profile, LCS(for cg matching) will never run. see the line (  `if (!FSFlattened)
    return false;`), the function just bails out if it doesn't find any profile.

>Asking because we probably don't want to load many functions just to do the similarity checks during LCS, as loading profile is kind of expensive?

Good question! Overall, I think the cost should be fine here. 
Note that this is in deep branch, we have a lot of filters(same caller scope, profile not in flattened profile), then the chance to run this code is probably not big.

Additionally, for the on-demand read https://github.com/llvm/llvm-project/blob/main/llvm/lib/ProfileData/SampleProfReader.cpp#L935-L937

See it check the `FuncOffsetTable`(not iterating all the `FuncOffsetTable`) and read one profile, reading nested profile in one profile is probably fine.  

(However, one thing I noticed that https://github.com/llvm/llvm-project/commit/4fe91e083a25715880992ea18ae474a3e4dae93f seems not in llvm-17- branch, we may need to port it to the old version branch)

https://github.com/llvm/llvm-project/pull/101053


More information about the llvm-commits mailing list