[PATCH] D146452: [AutoFDO] Use flattened profiles for profile staleness metrics

Mingming Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 20 23:33:26 PDT 2023

mingmingl added inline comments.

Comment at: llvm/include/llvm/ProfileData/SampleProf.h:1310
+  static void flattenProfile(SampleProfileMap &ProfileMap) {
+    SampleProfileMap TmpProfiles;
nit: const SampleProfileMap

Comment at: llvm/include/llvm/ProfileData/SampleProf.h:1313
+    flattenProfile(TmpProfiles, ProfileMap);
+    ProfileMap = std::move(TmpProfiles);
+  }
nit: I'm wondering if it's simpler to pass 'ProfileMap' as opposed to use `std::move`

Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2076-2077
   if (ReportProfileStaleness || PersistProfileStaleness) {
+    if (!FlattenProfileForMatching.getNumOccurrences())
+      FlattenProfileForMatching = true;
for my information, is it intended that flattening is true if it's not set (num occurrences = 0)? ask since it seems to change existing behavior.

Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2091-2095
+  if (FunctionSamples::ProfileIsCS) {
+    for (const auto &I : Reader.getProfiles())
+      FlatProfiles[I.second.getName()].merge(I.second);
+  } else
+    ProfileConverter::flattenProfile(FlatProfiles, Reader.getProfiles());
If I read correctly, `FunctionSamples::ProfileIsCS` is a per-reader instance bool (correct me if I miss anything). So it might be useful to add tests for context-sensitive profile and non-CS profiles.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list