[llvm] [SampleFDO] Improve stale profile matching by diff algorithm (PR #87375)

Lei Wang via llvm-commits llvm-commits at lists.llvm.org
Thu May 9 23:16:47 PDT 2024


================
@@ -222,6 +260,57 @@ void SampleProfileMatcher::runStaleProfileMatching(
   }
 }
 
+// Call target name anchor based profile fuzzy matching.
+// Input:
+// For IR locations, the anchor is the callee name of direct callsite; For
+// profile locations, it's the call target name for BodySamples or inlinee's
+// profile name for CallsiteSamples.
+// Matching heuristic:
+// First match all the anchors using the diff algorithm, then split the
+// non-anchor locations between the two anchors evenly, first half are matched
+// based on the start anchor, second half are matched based on the end anchor.
+// For example, given:
+// IR locations:      [1, 2(foo), 3, 5, 6(bar), 7]
+// Profile locations: [1, 2, 3(foo), 4, 7, 8(bar), 9]
+// The matching gives:
+//   [1,    2(foo), 3,  5,  6(bar), 7]
+//    |     |       |   |     |     |
+//   [1, 2, 3(foo), 4,  7,  8(bar), 9]
+// The output mapping: [2->3, 3->4, 5->7, 6->8, 7->9].
+void SampleProfileMatcher::runStaleProfileMatching(
+    const Function &F, const AnchorMap &IRAnchors,
+    const AnchorMap &ProfileAnchors, LocToLocMap &IRToProfileLocationMap) {
+  LLVM_DEBUG(dbgs() << "Run stale profile matching for " << F.getName()
+                    << "\n");
+  assert(IRToProfileLocationMap.empty() &&
+         "Run stale profile matching only once per function");
+
+  AnchorList FilteredProfileAnchorList;
+  for (const auto &I : ProfileAnchors)
+    FilteredProfileAnchorList.emplace_back(I);
+
+  AnchorList FilteredIRAnchorsList;
+  // Filter the non-callsite from IRAnchors.
+  for (const auto &I : IRAnchors) {
+    if (I.second.stringRef().empty())
+      continue;
+    FilteredIRAnchorsList.emplace_back(I);
+  }
----------------
wlei-llvm wrote:

>Actually for IRAnchors, why does it have to be a AnchorMap? I didn't find any usage that needs on map::find? Can we use AnchorList type from the beginning to avoid the conversion from map to vector here?

`AnchorMap` is  used here to deduplicate the IR location(the map key)(see the `findIRAnchors`), as there could have BB/call duplication optimization.  

> I'm also wondering for matching sequence, do we have to exclude indirect call on both sides?
Like what if we just allow indirect calls on both sides to match as part of sequence? Would that simplify the implementation more by removing the filtering, but without hurting the effectiveness?

We do include the indirect call for the matching, I added one test for the (see the `Run stale profile matching for test_indirect_call...`, it basically uses the dummy name(`unknown.indirect.callee`) which now both sides can have.

To clarify, here the filtering is to filter the non-call inst/location, recalling that in `findIRAnchors`, we use the empty name to represent the non-call inst. then diff alg needs to work on call only(anchor).  




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


More information about the llvm-commits mailing list