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

Lei Wang via llvm-commits llvm-commits at lists.llvm.org
Mon May 6 09:26:27 PDT 2024


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

>From 51c8adc59dda52f30430e7283fc29d24f01c02fd Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Mon, 1 Apr 2024 10:04:47 -0700
Subject: [PATCH 1/4] [SampleFDO] Use Myers diff for stale profile matching

---
 .../Transforms/IPO/SampleProfileMatcher.h     |  55 ++++-
 .../Transforms/IPO/SampleProfileMatcher.cpp   | 223 +++++++++++++-----
 llvm/unittests/Transforms/IPO/CMakeLists.txt  |   2 +
 .../IPO/SampleProfileMatcherTests.cpp         | 134 +++++++++++
 4 files changed, 349 insertions(+), 65 deletions(-)
 create mode 100644 llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp

diff --git a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
index 7ae6194da7c9cc..44335274239b6b 100644
--- a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
+++ b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
@@ -19,6 +19,53 @@
 
 namespace llvm {
 
+// Callsite location based matching anchor.
+struct Anchor {
+  LineLocation Loc;
+  FunctionId FuncId;
+
+  Anchor(const LineLocation &Loc, const FunctionId &FuncId)
+      : Loc(Loc), FuncId(FuncId) {}
+  bool operator==(const Anchor &Other) const {
+    return this->FuncId == Other.FuncId;
+  }
+};
+
+// This class implements the Myers diff algorithm used for stale profile
+// matching. The algorithm provides a simple and efficient way to find the
+// Longest Common Subsequence(LCS) or the Shortest Edit Script(SES) of two
+// sequences. For more details, refer to the paper 'An O(ND) Difference
+// Algorithm and Its Variations' by Eugene W. Myers.
+// In the scenario of profile fuzzy matching, the two sequences are the IR
+// callsite anchors and profile callsite anchors. The subsequence equivalent
+// parts from the resulting SES are used to remap the IR locations to the
+// profile locations.
+class MyersDiff {
+public:
+  struct DiffResult {
+    LocToLocMap EqualLocations;
+    // New IR locations that are inserted in the new version.
+    std::vector<LineLocation> Insertions;
+    // Old Profile locations that are deleted in the new version.
+    std::vector<LineLocation> Deletions;
+    void addEqualLocations(const LineLocation &IRLoc,
+                           const LineLocation &ProfLoc) {
+      EqualLocations.insert({IRLoc, ProfLoc});
+    }
+    void addInsertion(const LineLocation &IRLoc) {
+      Insertions.push_back(IRLoc);
+    }
+    void addDeletion(const LineLocation &ProfLoc) {
+      Deletions.push_back(ProfLoc);
+    }
+  };
+
+  // The basic greedy version of Myers's algorithm. Refer to page 6 of the
+  // original paper.
+  DiffResult shortestEdit(const std::vector<Anchor> &A,
+                          const std::vector<Anchor> &B) const;
+};
+
 // Sample profile matching - fuzzy match.
 class SampleProfileMatcher {
   Module &M;
@@ -27,8 +74,8 @@ class SampleProfileMatcher {
   const ThinOrFullLTOPhase LTOPhase;
   SampleProfileMap FlattenedProfiles;
   // For each function, the matcher generates a map, of which each entry is a
-  // mapping from the source location of current build to the source location in
-  // the profile.
+  // mapping from the source location of current build to the source location
+  // in the profile.
   StringMap<LocToLocMap> FuncMappings;
 
   // Match state for an anchor/callsite.
@@ -143,6 +190,10 @@ class SampleProfileMatcher {
   }
   void distributeIRToProfileLocationMap();
   void distributeIRToProfileLocationMap(FunctionSamples &FS);
+  void matchNonAnchorAndWriteResults(
+      const LocToLocMap &AnchorMatchings,
+      const std::map<LineLocation, StringRef> &IRAnchors,
+      LocToLocMap &IRToProfileLocationMap);
   void runStaleProfileMatching(
       const Function &F, const std::map<LineLocation, StringRef> &IRAnchors,
       const std::map<LineLocation, std::unordered_set<FunctionId>>
diff --git a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
index 1ca89e0091daff..139a1636a7cbd3 100644
--- a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
@@ -122,15 +122,149 @@ void SampleProfileMatcher::findProfileAnchors(
   }
 }
 
+MyersDiff::DiffResult
+MyersDiff::shortestEdit(const std::vector<Anchor> &A,
+                        const std::vector<Anchor> &B) const {
+  int32_t N = A.size(), M = B.size(), Max = N + M;
+  auto Index = [&](int32_t I) { return I + Max; };
+
+  DiffResult Diff;
+  if (Max == 0)
+    return Diff;
+
+  // Backtrack the SES result.
+  auto Backtrack = [&](const std::vector<std::vector<int32_t>> &Trace,
+                       const std::vector<Anchor> &A,
+                       const std::vector<Anchor> &B) {
+    int32_t X = N, Y = M;
+    for (int32_t D = Trace.size() - 1; X > 0 || Y > 0; D--) {
+      const auto &P = Trace[D];
+      int32_t K = X - Y;
+      int32_t PrevK = K;
+      if (K == -D || (K != D && P[Index(K - 1)] < P[Index(K + 1)]))
+        PrevK = K + 1;
+      else
+        PrevK = K - 1;
+
+      int32_t PrevX = P[Index(PrevK)];
+      int32_t PrevY = PrevX - PrevK;
+      while (X > PrevX && Y > PrevY) {
+        X--;
+        Y--;
+        Diff.addEqualLocations(A[X].Loc, B[Y].Loc);
+      }
+
+      if (D == 0)
+        break;
+
+      if (Y == PrevY) {
+        X--;
+        Diff.addInsertion(A[X].Loc);
+      } else if (X == PrevX) {
+        Y--;
+        Diff.addDeletion(B[Y].Loc);
+      }
+      X = PrevX;
+      Y = PrevY;
+    }
+  };
+
+  // The greedy LCS/SES algorithm.
+  std::vector<int32_t> V(2 * Max + 1, -1);
+  V[Index(1)] = 0;
+  std::vector<std::vector<int32_t>> Trace;
+  for (int32_t D = 0; D <= Max; D++) {
+    Trace.push_back(V);
+    for (int32_t K = -D; K <= D; K += 2) {
+      int32_t X = 0, Y = 0;
+      if (K == -D || (K != D && V[Index(K - 1)] < V[Index(K + 1)]))
+        X = V[Index(K + 1)];
+      else
+        X = V[Index(K - 1)] + 1;
+      Y = X - K;
+      while (X < N && Y < M && A[X] == B[Y])
+        X++, Y++;
+
+      V[Index(K)] = X;
+
+      if (X >= N && Y >= M) {
+        // Length of an SES is D.
+        Backtrack(Trace, A, B);
+        return Diff;
+      }
+    }
+  }
+  // Length of an SES is greater than Max.
+  return Diff;
+}
+
+void SampleProfileMatcher::matchNonAnchorAndWriteResults(
+    const LocToLocMap &AnchorMatchings,
+    const std::map<LineLocation, StringRef> &IRAnchors,
+    LocToLocMap &IRToProfileLocationMap) {
+  auto InsertMatching = [&](const LineLocation &From, const LineLocation &To) {
+    // Skip the unchanged location mapping to save memory.
+    if (From != To)
+      IRToProfileLocationMap.insert({From, To});
+  };
+
+  // Use function's beginning location as the initial anchor.
+  int32_t LocationDelta = 0;
+  SmallVector<LineLocation> LastMatchedNonAnchors;
+  for (const auto &IR : IRAnchors) {
+    const auto &Loc = IR.first;
+    StringRef CalleeName = IR.second;
+    bool IsMatchedAnchor = false;
+
+    // Match the anchor location in lexical order.
+    auto R = AnchorMatchings.find(Loc);
+    if (R != AnchorMatchings.end()) {
+      const auto &Candidate = R->second;
+      InsertMatching(Loc, Candidate);
+      LLVM_DEBUG(dbgs() << "Callsite with callee:" << CalleeName
+                        << " is matched from " << Loc << " to " << Candidate
+                        << "\n");
+      LocationDelta = Candidate.LineOffset - Loc.LineOffset;
+
+      // Match backwards for non-anchor locations.
+      // The locations in LastMatchedNonAnchors have been matched forwards
+      // based on the previous anchor, spilt it evenly and overwrite the
+      // second half based on the current anchor.
+      for (size_t I = (LastMatchedNonAnchors.size() + 1) / 2;
+           I < LastMatchedNonAnchors.size(); I++) {
+        const auto &L = LastMatchedNonAnchors[I];
+        uint32_t CandidateLineOffset = L.LineOffset + LocationDelta;
+        LineLocation Candidate(CandidateLineOffset, L.Discriminator);
+        InsertMatching(L, Candidate);
+        LLVM_DEBUG(dbgs() << "Location is rematched backwards from " << L
+                          << " to " << Candidate << "\n");
+      }
+
+      IsMatchedAnchor = true;
+      LastMatchedNonAnchors.clear();
+    }
+
+    // Match forwards for non-anchor locations.
+    if (!IsMatchedAnchor) {
+      uint32_t CandidateLineOffset = Loc.LineOffset + LocationDelta;
+      LineLocation Candidate(CandidateLineOffset, Loc.Discriminator);
+      InsertMatching(Loc, Candidate);
+      LLVM_DEBUG(dbgs() << "Location is matched from " << Loc << " to "
+                        << Candidate << "\n");
+      LastMatchedNonAnchors.emplace_back(Loc);
+    }
+  }
+}
+
 // 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 in lexical order, 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.
+// 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]
@@ -149,77 +283,40 @@ void SampleProfileMatcher::runStaleProfileMatching(
   assert(IRToProfileLocationMap.empty() &&
          "Run stale profile matching only once per function");
 
-  std::unordered_map<FunctionId, std::set<LineLocation>> CalleeToCallsitesMap;
+  std::vector<Anchor> ProfileCallsiteAnchors;
   for (const auto &I : ProfileAnchors) {
     const auto &Loc = I.first;
     const auto &Callees = I.second;
     // Filter out possible indirect calls, use direct callee name as anchor.
     if (Callees.size() == 1) {
-      FunctionId CalleeName = *Callees.begin();
-      const auto &Candidates = CalleeToCallsitesMap.try_emplace(
-          CalleeName, std::set<LineLocation>());
-      Candidates.first->second.insert(Loc);
+      auto CalleeName = *Callees.begin();
+      ProfileCallsiteAnchors.emplace_back(Loc, CalleeName);
+    } else if (Callees.size() > 1) {
+      ProfileCallsiteAnchors.emplace_back(Loc,
+                                          FunctionId(UnknownIndirectCallee));
     }
   }
 
-  auto InsertMatching = [&](const LineLocation &From, const LineLocation &To) {
-    // Skip the unchanged location mapping to save memory.
-    if (From != To)
-      IRToProfileLocationMap.insert({From, To});
-  };
-
-  // Use function's beginning location as the initial anchor.
-  int32_t LocationDelta = 0;
-  SmallVector<LineLocation> LastMatchedNonAnchors;
+  std::vector<Anchor> IRCallsiteAnchors;
+  for (const auto &I : IRAnchors) {
+    const auto &Loc = I.first;
+    const auto &CalleeName = I.second;
+    if (CalleeName.empty())
+      continue;
+    IRCallsiteAnchors.emplace_back(Loc, FunctionId(CalleeName));
+  }
 
-  for (const auto &IR : IRAnchors) {
-    const auto &Loc = IR.first;
-    auto CalleeName = IR.second;
-    bool IsMatchedAnchor = false;
-    // Match the anchor location in lexical order.
-    if (!CalleeName.empty()) {
-      auto CandidateAnchors =
-          CalleeToCallsitesMap.find(getRepInFormat(CalleeName));
-      if (CandidateAnchors != CalleeToCallsitesMap.end() &&
-          !CandidateAnchors->second.empty()) {
-        auto CI = CandidateAnchors->second.begin();
-        const auto Candidate = *CI;
-        CandidateAnchors->second.erase(CI);
-        InsertMatching(Loc, Candidate);
-        LLVM_DEBUG(dbgs() << "Callsite with callee:" << CalleeName
-                          << " is matched from " << Loc << " to " << Candidate
-                          << "\n");
-        LocationDelta = Candidate.LineOffset - Loc.LineOffset;
-
-        // Match backwards for non-anchor locations.
-        // The locations in LastMatchedNonAnchors have been matched forwards
-        // based on the previous anchor, spilt it evenly and overwrite the
-        // second half based on the current anchor.
-        for (size_t I = (LastMatchedNonAnchors.size() + 1) / 2;
-             I < LastMatchedNonAnchors.size(); I++) {
-          const auto &L = LastMatchedNonAnchors[I];
-          uint32_t CandidateLineOffset = L.LineOffset + LocationDelta;
-          LineLocation Candidate(CandidateLineOffset, L.Discriminator);
-          InsertMatching(L, Candidate);
-          LLVM_DEBUG(dbgs() << "Location is rematched backwards from " << L
-                            << " to " << Candidate << "\n");
-        }
+  if (IRCallsiteAnchors.empty() || ProfileCallsiteAnchors.empty())
+    return;
 
-        IsMatchedAnchor = true;
-        LastMatchedNonAnchors.clear();
-      }
-    }
+  // Use the diff algorithm to find the SES, the resulting equal locations from
+  // IR to Profile are used as anchor to match other locations. Note that here
+  // use IR anchor as base(A) to align with the order of IRToProfileLocationMap.
+  MyersDiff Diff;
+  auto DiffRes = Diff.shortestEdit(IRCallsiteAnchors, ProfileCallsiteAnchors);
 
-    // Match forwards for non-anchor locations.
-    if (!IsMatchedAnchor) {
-      uint32_t CandidateLineOffset = Loc.LineOffset + LocationDelta;
-      LineLocation Candidate(CandidateLineOffset, Loc.Discriminator);
-      InsertMatching(Loc, Candidate);
-      LLVM_DEBUG(dbgs() << "Location is matched from " << Loc << " to "
-                        << Candidate << "\n");
-      LastMatchedNonAnchors.emplace_back(Loc);
-    }
-  }
+  matchNonAnchorAndWriteResults(DiffRes.EqualLocations, IRAnchors,
+                                IRToProfileLocationMap);
 }
 
 void SampleProfileMatcher::runOnFunction(Function &F) {
diff --git a/llvm/unittests/Transforms/IPO/CMakeLists.txt b/llvm/unittests/Transforms/IPO/CMakeLists.txt
index 4e4372179b46c7..80d0eadf0cce09 100644
--- a/llvm/unittests/Transforms/IPO/CMakeLists.txt
+++ b/llvm/unittests/Transforms/IPO/CMakeLists.txt
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
   AsmParser
   Core
   IPO
+  ProfileData
   Support
   TargetParser
   TransformUtils
@@ -13,6 +14,7 @@ add_llvm_unittest(IPOTests
   WholeProgramDevirt.cpp
   AttributorTest.cpp
   FunctionSpecializationTest.cpp
+  SampleProfileMatcherTests.cpp
   )
 
 set_property(TARGET IPOTests PROPERTY FOLDER "Tests/UnitTests/TransformsTests")
diff --git a/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp b/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
new file mode 100644
index 00000000000000..37b92c80068ddd
--- /dev/null
+++ b/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
@@ -0,0 +1,134 @@
+//===- SampleProfileMatcherTests.cpp - SampleProfileMatcher Unit Tests -----==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/IPO/SampleProfileMatcher.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+MyersDiff Diff;
+
+std::vector<Anchor>
+createAnchorsFromStrings(const std::vector<std::string> &SV) {
+  std::vector<Anchor> Anchors;
+  for (uint64_t I = 0; I < SV.size(); I++) {
+    Anchors.push_back(Anchor(LineLocation(I, 0), FunctionId(SV[I])));
+  }
+  return Anchors;
+}
+
+LocToLocMap
+createEqualLocations(const std::vector<std::pair<uint32_t, uint32_t>> &V) {
+  LocToLocMap LocMap;
+  for (auto P : V) {
+    LocMap.emplace(LineLocation(P.first, 0), LineLocation(P.second, 0));
+  }
+  return LocMap;
+}
+
+std::vector<LineLocation> createLocations(const std::vector<uint32_t> &V) {
+  std::vector<LineLocation> Locations;
+  for (auto I : V) {
+    Locations.emplace_back(LineLocation(I, 0));
+  }
+  return Locations;
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest1) {
+
+  std::vector<Anchor> AnchorsA;
+  std::vector<Anchor> AnchorsB;
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_TRUE(R.EqualLocations.empty());
+  EXPECT_TRUE(R.Deletions.empty());
+  EXPECT_TRUE(R.Insertions.empty());
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest2) {
+  std::vector<std::string> A({"a", "b", "c"});
+  std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
+  std::vector<Anchor> AnchorsB;
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_TRUE(R.EqualLocations.empty());
+  EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({2, 1, 0})));
+  EXPECT_TRUE(R.Deletions.empty());
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest3) {
+
+  std::vector<Anchor> AnchorsA;
+  std::vector<std::string> B({"a", "b", "c"});
+  std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_TRUE(R.EqualLocations.empty());
+  EXPECT_TRUE(R.Insertions.empty());
+  EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({2, 1, 0})));
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest4) {
+  std::vector<std::string> A({"a", "b", "c"});
+  std::vector<std::string> B({"a", "b", "c"});
+  std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
+  std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
+  LocToLocMap ExpectEqualLocations =
+      createEqualLocations({{0, 0}, {1, 1}, {2, 2}});
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+  EXPECT_TRUE(R.Insertions.empty());
+  EXPECT_TRUE(R.Deletions.empty());
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest5) {
+  std::vector<std::string> A({"a", "b", "c"});
+  std::vector<std::string> B({"b", "c", "d"});
+  std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
+  std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
+  LocToLocMap ExpectEqualLocations = createEqualLocations({{1, 0}, {2, 1}});
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+  EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({0})));
+  EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({2})));
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest6) {
+  std::vector<std::string> A({"a", "b", "d"});
+  std::vector<std::string> B({"a", "c", "d"});
+  std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
+  std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
+  LocToLocMap ExpectEqualLocations = createEqualLocations({{0, 0}, {2, 2}});
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+  EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({1})));
+  EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({1})));
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest7) {
+  std::vector<std::string> A({"a", "b", "c", "a", "b", "b", "a"});
+  std::vector<std::string> B({"c", "b", "a", "b", "a", "c"});
+  std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
+  std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
+  LocToLocMap ExpectEqualLocations =
+      createEqualLocations({{2, 0}, {3, 2}, {4, 3}, {6, 4}});
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+  EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({5, 1, 0})));
+  EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({5, 1})));
+}
+
+TEST(SampleProfileMatcherTests, MyersDiffTest8) {
+  std::vector<std::string> A({"a", "c", "b", "c", "b", "d", "e"});
+  std::vector<std::string> B({"a", "b", "c", "a", "a", "b", "c", "c", "d"});
+  std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
+  std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
+  LocToLocMap ExpectEqualLocations =
+      createEqualLocations({{0, 0}, {2, 1}, {3, 2}, {4, 5}, {5, 8}});
+  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+  EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({6, 1})));
+  EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({7, 6, 4, 3})));
+}

>From 858b04060a0e606439f49a9e0f95d2ebd50e12f6 Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Thu, 25 Apr 2024 18:10:15 -0700
Subject: [PATCH 2/4] addressing comments

---
 .../Transforms/IPO/SampleProfileMatcher.h     |  8 +++--
 .../Transforms/IPO/SampleProfileMatcher.cpp   | 20 ++++++++----
 .../IPO/SampleProfileMatcherTests.cpp         | 32 ++++++++++++++-----
 3 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
index 44335274239b6b..5b56638c13344b 100644
--- a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
+++ b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
@@ -44,26 +44,30 @@ class MyersDiff {
 public:
   struct DiffResult {
     LocToLocMap EqualLocations;
+#ifndef NDEBUG
     // New IR locations that are inserted in the new version.
     std::vector<LineLocation> Insertions;
     // Old Profile locations that are deleted in the new version.
     std::vector<LineLocation> Deletions;
+#endif
     void addEqualLocations(const LineLocation &IRLoc,
                            const LineLocation &ProfLoc) {
       EqualLocations.insert({IRLoc, ProfLoc});
     }
+#ifndef NDEBUG
     void addInsertion(const LineLocation &IRLoc) {
       Insertions.push_back(IRLoc);
     }
     void addDeletion(const LineLocation &ProfLoc) {
       Deletions.push_back(ProfLoc);
     }
+#endif
   };
 
   // The basic greedy version of Myers's algorithm. Refer to page 6 of the
   // original paper.
-  DiffResult shortestEdit(const std::vector<Anchor> &A,
-                          const std::vector<Anchor> &B) const;
+  DiffResult longestCommonSequence(const std::vector<Anchor> &A,
+                                   const std::vector<Anchor> &B) const;
 };
 
 // Sample profile matching - fuzzy match.
diff --git a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
index 139a1636a7cbd3..de81450f0da252 100644
--- a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
@@ -123,8 +123,8 @@ void SampleProfileMatcher::findProfileAnchors(
 }
 
 MyersDiff::DiffResult
-MyersDiff::shortestEdit(const std::vector<Anchor> &A,
-                        const std::vector<Anchor> &B) const {
+MyersDiff::longestCommonSequence(const std::vector<Anchor> &A,
+                                 const std::vector<Anchor> &B) const {
   int32_t N = A.size(), M = B.size(), Max = N + M;
   auto Index = [&](int32_t I) { return I + Max; };
 
@@ -159,10 +159,14 @@ MyersDiff::shortestEdit(const std::vector<Anchor> &A,
 
       if (Y == PrevY) {
         X--;
+#ifndef NDEBUG
         Diff.addInsertion(A[X].Loc);
+#endif
       } else if (X == PrevX) {
         Y--;
+#ifndef NDEBUG
         Diff.addDeletion(B[Y].Loc);
+#endif
       }
       X = PrevX;
       Y = PrevY;
@@ -213,7 +217,7 @@ void SampleProfileMatcher::matchNonAnchorAndWriteResults(
   SmallVector<LineLocation> LastMatchedNonAnchors;
   for (const auto &IR : IRAnchors) {
     const auto &Loc = IR.first;
-    StringRef CalleeName = IR.second;
+    [[maybe_unused]] StringRef CalleeName = IR.second;
     bool IsMatchedAnchor = false;
 
     // Match the anchor location in lexical order.
@@ -309,11 +313,13 @@ void SampleProfileMatcher::runStaleProfileMatching(
   if (IRCallsiteAnchors.empty() || ProfileCallsiteAnchors.empty())
     return;
 
-  // Use the diff algorithm to find the SES, the resulting equal locations from
-  // IR to Profile are used as anchor to match other locations. Note that here
-  // use IR anchor as base(A) to align with the order of IRToProfileLocationMap.
+  // Use the diff algorithm to find the LCS/SES, the resulting equal locations
+  // from IR to Profile are used as anchor to match other locations. Note that
+  // here use IR anchor as base(A) to align with the order of
+  // IRToProfileLocationMap.
   MyersDiff Diff;
-  auto DiffRes = Diff.shortestEdit(IRCallsiteAnchors, ProfileCallsiteAnchors);
+  auto DiffRes =
+      Diff.longestCommonSequence(IRCallsiteAnchors, ProfileCallsiteAnchors);
 
   matchNonAnchorAndWriteResults(DiffRes.EqualLocations, IRAnchors,
                                 IRToProfileLocationMap);
diff --git a/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp b/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
index 37b92c80068ddd..25d5c053d3ccbd 100644
--- a/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
+++ b/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
@@ -43,20 +43,24 @@ TEST(SampleProfileMatcherTests, MyersDiffTest1) {
 
   std::vector<Anchor> AnchorsA;
   std::vector<Anchor> AnchorsB;
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_TRUE(R.EqualLocations.empty());
+#ifndef NDEBUG
   EXPECT_TRUE(R.Deletions.empty());
   EXPECT_TRUE(R.Insertions.empty());
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest2) {
   std::vector<std::string> A({"a", "b", "c"});
   std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
   std::vector<Anchor> AnchorsB;
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_TRUE(R.EqualLocations.empty());
+#ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({2, 1, 0})));
   EXPECT_TRUE(R.Deletions.empty());
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest3) {
@@ -64,10 +68,12 @@ TEST(SampleProfileMatcherTests, MyersDiffTest3) {
   std::vector<Anchor> AnchorsA;
   std::vector<std::string> B({"a", "b", "c"});
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_TRUE(R.EqualLocations.empty());
+#ifndef NDEBUG
   EXPECT_TRUE(R.Insertions.empty());
   EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({2, 1, 0})));
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest4) {
@@ -77,10 +83,12 @@ TEST(SampleProfileMatcherTests, MyersDiffTest4) {
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations =
       createEqualLocations({{0, 0}, {1, 1}, {2, 2}});
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+#ifndef NDEBUG
   EXPECT_TRUE(R.Insertions.empty());
   EXPECT_TRUE(R.Deletions.empty());
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest5) {
@@ -89,10 +97,12 @@ TEST(SampleProfileMatcherTests, MyersDiffTest5) {
   std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations = createEqualLocations({{1, 0}, {2, 1}});
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+#ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({0})));
   EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({2})));
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest6) {
@@ -101,10 +111,12 @@ TEST(SampleProfileMatcherTests, MyersDiffTest6) {
   std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations = createEqualLocations({{0, 0}, {2, 2}});
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+#ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({1})));
   EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({1})));
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest7) {
@@ -114,10 +126,12 @@ TEST(SampleProfileMatcherTests, MyersDiffTest7) {
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations =
       createEqualLocations({{2, 0}, {3, 2}, {4, 3}, {6, 4}});
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+#ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({5, 1, 0})));
   EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({5, 1})));
+#endif
 }
 
 TEST(SampleProfileMatcherTests, MyersDiffTest8) {
@@ -127,8 +141,10 @@ TEST(SampleProfileMatcherTests, MyersDiffTest8) {
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations =
       createEqualLocations({{0, 0}, {2, 1}, {3, 2}, {4, 5}, {5, 8}});
-  auto R = Diff.shortestEdit(AnchorsA, AnchorsB);
+  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
+#ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({6, 1})));
   EXPECT_EQ(R.Deletions, createLocations(std::vector<uint32_t>({7, 6, 4, 3})));
+#endif
 }

>From 390efcc74a4bb8540fe57f21564529d13bd69c68 Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Mon, 29 Apr 2024 16:10:35 -0700
Subject: [PATCH 3/4] addressing comments

---
 .../Transforms/IPO/SampleProfileMatcher.h     | 14 +++---
 .../Transforms/IPO/SampleProfileMatcher.cpp   | 47 ++++++++++++-------
 .../IPO/SampleProfileMatcherTests.cpp         | 16 +++----
 3 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
index 5b56638c13344b..6de49657885c4a 100644
--- a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
+++ b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
@@ -39,7 +39,8 @@ struct Anchor {
 // In the scenario of profile fuzzy matching, the two sequences are the IR
 // callsite anchors and profile callsite anchors. The subsequence equivalent
 // parts from the resulting SES are used to remap the IR locations to the
-// profile locations.
+// profile locations. As the number of function callsite is usually not big, we
+// currently just implements the basic greedy version(page 6 of the paper).
 class MyersDiff {
 public:
   struct DiffResult {
@@ -64,10 +65,8 @@ class MyersDiff {
 #endif
   };
 
-  // The basic greedy version of Myers's algorithm. Refer to page 6 of the
-  // original paper.
-  DiffResult longestCommonSequence(const std::vector<Anchor> &A,
-                                   const std::vector<Anchor> &B) const;
+  DiffResult shortestEditScript(const std::vector<Anchor> &A,
+                                const std::vector<Anchor> &B) const;
 };
 
 // Sample profile matching - fuzzy match.
@@ -194,7 +193,10 @@ class SampleProfileMatcher {
   }
   void distributeIRToProfileLocationMap();
   void distributeIRToProfileLocationMap(FunctionSamples &FS);
-  void matchNonAnchorAndWriteResults(
+  LocToLocMap longestCommonSequence(
+      const std::vector<Anchor> &IRCallsiteAnchors,
+      const std::vector<Anchor> &ProfileCallsiteAnchors) const;
+  void matchNonCallsiteLocsAndWriteResults(
       const LocToLocMap &AnchorMatchings,
       const std::map<LineLocation, StringRef> &IRAnchors,
       LocToLocMap &IRToProfileLocationMap);
diff --git a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
index de81450f0da252..d9284717196f47 100644
--- a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
@@ -123,8 +123,8 @@ void SampleProfileMatcher::findProfileAnchors(
 }
 
 MyersDiff::DiffResult
-MyersDiff::longestCommonSequence(const std::vector<Anchor> &A,
-                                 const std::vector<Anchor> &B) const {
+MyersDiff::shortestEditScript(const std::vector<Anchor> &A,
+                              const std::vector<Anchor> &B) const {
   int32_t N = A.size(), M = B.size(), Max = N + M;
   auto Index = [&](int32_t I) { return I + Max; };
 
@@ -135,7 +135,7 @@ MyersDiff::longestCommonSequence(const std::vector<Anchor> &A,
   // Backtrack the SES result.
   auto Backtrack = [&](const std::vector<std::vector<int32_t>> &Trace,
                        const std::vector<Anchor> &A,
-                       const std::vector<Anchor> &B) {
+                       const std::vector<Anchor> &B, DiffResult &Diff) {
     int32_t X = N, Y = M;
     for (int32_t D = Trace.size() - 1; X > 0 || Y > 0; D--) {
       const auto &P = Trace[D];
@@ -174,8 +174,11 @@ MyersDiff::longestCommonSequence(const std::vector<Anchor> &A,
   };
 
   // The greedy LCS/SES algorithm.
+
+  // An array contains the endpoints of the furthest reaching D-paths.
   std::vector<int32_t> V(2 * Max + 1, -1);
   V[Index(1)] = 0;
+  // Trace is used to backtrack the SES result.
   std::vector<std::vector<int32_t>> Trace;
   for (int32_t D = 0; D <= Max; D++) {
     Trace.push_back(V);
@@ -193,7 +196,7 @@ MyersDiff::longestCommonSequence(const std::vector<Anchor> &A,
 
       if (X >= N && Y >= M) {
         // Length of an SES is D.
-        Backtrack(Trace, A, B);
+        Backtrack(Trace, A, B, Diff);
         return Diff;
       }
     }
@@ -202,8 +205,18 @@ MyersDiff::longestCommonSequence(const std::vector<Anchor> &A,
   return Diff;
 }
 
-void SampleProfileMatcher::matchNonAnchorAndWriteResults(
-    const LocToLocMap &AnchorMatchings,
+LocToLocMap SampleProfileMatcher::longestCommonSequence(
+    const std::vector<Anchor> &IRCallsiteAnchors,
+    const std::vector<Anchor> &ProfileCallsiteAnchors) const {
+  // Use the diff algorithm to find the LCS/SES, the resulting equal locations
+  // from IR to Profile are used as anchor to match other locations.
+  auto SES =
+      MyersDiff().shortestEditScript(IRCallsiteAnchors, ProfileCallsiteAnchors);
+  return SES.EqualLocations;
+}
+
+void SampleProfileMatcher::matchNonCallsiteLocsAndWriteResults(
+    const LocToLocMap &MatchedAnchors,
     const std::map<LineLocation, StringRef> &IRAnchors,
     LocToLocMap &IRToProfileLocationMap) {
   auto InsertMatching = [&](const LineLocation &From, const LineLocation &To) {
@@ -221,8 +234,8 @@ void SampleProfileMatcher::matchNonAnchorAndWriteResults(
     bool IsMatchedAnchor = false;
 
     // Match the anchor location in lexical order.
-    auto R = AnchorMatchings.find(Loc);
-    if (R != AnchorMatchings.end()) {
+    auto R = MatchedAnchors.find(Loc);
+    if (R != MatchedAnchors.end()) {
       const auto &Candidate = R->second;
       InsertMatching(Loc, Candidate);
       LLVM_DEBUG(dbgs() << "Callsite with callee:" << CalleeName
@@ -313,16 +326,16 @@ void SampleProfileMatcher::runStaleProfileMatching(
   if (IRCallsiteAnchors.empty() || ProfileCallsiteAnchors.empty())
     return;
 
-  // Use the diff algorithm to find the LCS/SES, the resulting equal locations
-  // from IR to Profile are used as anchor to match other locations. Note that
-  // here use IR anchor as base(A) to align with the order of
-  // IRToProfileLocationMap.
-  MyersDiff Diff;
-  auto DiffRes =
-      Diff.longestCommonSequence(IRCallsiteAnchors, ProfileCallsiteAnchors);
+  // Match the callsite anchors by finding the longest common subsequence
+  // between IR and profile. Note that we need to use IR anchor as base(A side)
+  // to align with the order of IRToProfileLocationMap.
+  LocToLocMap MatchedAnchors =
+      longestCommonSequence(IRCallsiteAnchors, ProfileCallsiteAnchors);
 
-  matchNonAnchorAndWriteResults(DiffRes.EqualLocations, IRAnchors,
-                                IRToProfileLocationMap);
+  // Match the non-callsite locations and write the result to
+  // IRToProfileLocationMap.
+  matchNonCallsiteLocsAndWriteResults(MatchedAnchors, IRAnchors,
+                                      IRToProfileLocationMap);
 }
 
 void SampleProfileMatcher::runOnFunction(Function &F) {
diff --git a/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp b/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
index 25d5c053d3ccbd..9bdd45f4ae155e 100644
--- a/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
+++ b/llvm/unittests/Transforms/IPO/SampleProfileMatcherTests.cpp
@@ -43,7 +43,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest1) {
 
   std::vector<Anchor> AnchorsA;
   std::vector<Anchor> AnchorsB;
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_TRUE(R.EqualLocations.empty());
 #ifndef NDEBUG
   EXPECT_TRUE(R.Deletions.empty());
@@ -55,7 +55,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest2) {
   std::vector<std::string> A({"a", "b", "c"});
   std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
   std::vector<Anchor> AnchorsB;
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_TRUE(R.EqualLocations.empty());
 #ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({2, 1, 0})));
@@ -68,7 +68,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest3) {
   std::vector<Anchor> AnchorsA;
   std::vector<std::string> B({"a", "b", "c"});
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_TRUE(R.EqualLocations.empty());
 #ifndef NDEBUG
   EXPECT_TRUE(R.Insertions.empty());
@@ -83,7 +83,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest4) {
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations =
       createEqualLocations({{0, 0}, {1, 1}, {2, 2}});
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
 #ifndef NDEBUG
   EXPECT_TRUE(R.Insertions.empty());
@@ -97,7 +97,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest5) {
   std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations = createEqualLocations({{1, 0}, {2, 1}});
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
 #ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({0})));
@@ -111,7 +111,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest6) {
   std::vector<Anchor> AnchorsA = createAnchorsFromStrings(A);
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations = createEqualLocations({{0, 0}, {2, 2}});
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
 #ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({1})));
@@ -126,7 +126,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest7) {
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations =
       createEqualLocations({{2, 0}, {3, 2}, {4, 3}, {6, 4}});
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
 #ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({5, 1, 0})));
@@ -141,7 +141,7 @@ TEST(SampleProfileMatcherTests, MyersDiffTest8) {
   std::vector<Anchor> AnchorsB = createAnchorsFromStrings(B);
   LocToLocMap ExpectEqualLocations =
       createEqualLocations({{0, 0}, {2, 1}, {3, 2}, {4, 5}, {5, 8}});
-  auto R = Diff.longestCommonSequence(AnchorsA, AnchorsB);
+  auto R = Diff.shortestEditScript(AnchorsA, AnchorsB);
   EXPECT_EQ(R.EqualLocations, ExpectEqualLocations);
 #ifndef NDEBUG
   EXPECT_EQ(R.Insertions, createLocations(std::vector<uint32_t>({6, 1})));

>From 40b03c978750995120204c417848c04b2eedfff4 Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Sun, 5 May 2024 20:00:06 -0700
Subject: [PATCH 4/4] addressing feedback

---
 .../Transforms/IPO/SampleProfileMatcher.h     |  33 ++--
 .../Transforms/IPO/SampleProfileMatcher.cpp   | 146 +++++++++---------
 2 files changed, 83 insertions(+), 96 deletions(-)

diff --git a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
index 6de49657885c4a..a95ad4f8053d95 100644
--- a/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
+++ b/llvm/include/llvm/Transforms/IPO/SampleProfileMatcher.h
@@ -26,6 +26,7 @@ struct Anchor {
 
   Anchor(const LineLocation &Loc, const FunctionId &FuncId)
       : Loc(Loc), FuncId(FuncId) {}
+  Anchor(const LineLocation &Loc, StringRef &FName) : Loc(Loc), FuncId(FName) {}
   bool operator==(const Anchor &Other) const {
     return this->FuncId == Other.FuncId;
   }
@@ -69,6 +70,8 @@ class MyersDiff {
                                 const std::vector<Anchor> &B) const;
 };
 
+using AnchorMap = std::map<LineLocation, Anchor>;
+
 // Sample profile matching - fuzzy match.
 class SampleProfileMatcher {
   Module &M;
@@ -145,18 +148,13 @@ class SampleProfileMatcher {
     return nullptr;
   }
   void runOnFunction(Function &F);
-  void findIRAnchors(const Function &F,
-                     std::map<LineLocation, StringRef> &IRAnchors);
-  void findProfileAnchors(
-      const FunctionSamples &FS,
-      std::map<LineLocation, std::unordered_set<FunctionId>> &ProfileAnchors);
+  void findIRAnchors(const Function &F, AnchorMap &IRAnchors);
+  void findProfileAnchors(const FunctionSamples &FS, AnchorMap &ProfileAnchors);
   // Record the callsite match states for profile staleness report, the result
   // is saved in FuncCallsiteMatchStates.
-  void recordCallsiteMatchStates(
-      const Function &F, const std::map<LineLocation, StringRef> &IRAnchors,
-      const std::map<LineLocation, std::unordered_set<FunctionId>>
-          &ProfileAnchors,
-      const LocToLocMap *IRToProfileLocationMap);
+  void recordCallsiteMatchStates(const Function &F, const AnchorMap &IRAnchors,
+                                 const AnchorMap &ProfileAnchors,
+                                 const LocToLocMap *IRToProfileLocationMap);
 
   bool isMismatchState(const enum MatchState &State) {
     return State == MatchState::InitialMismatch ||
@@ -196,15 +194,12 @@ class SampleProfileMatcher {
   LocToLocMap longestCommonSequence(
       const std::vector<Anchor> &IRCallsiteAnchors,
       const std::vector<Anchor> &ProfileCallsiteAnchors) const;
-  void matchNonCallsiteLocsAndWriteResults(
-      const LocToLocMap &AnchorMatchings,
-      const std::map<LineLocation, StringRef> &IRAnchors,
-      LocToLocMap &IRToProfileLocationMap);
-  void runStaleProfileMatching(
-      const Function &F, const std::map<LineLocation, StringRef> &IRAnchors,
-      const std::map<LineLocation, std::unordered_set<FunctionId>>
-          &ProfileAnchors,
-      LocToLocMap &IRToProfileLocationMap);
+  void matchNonCallsiteLocsAndWriteResults(const LocToLocMap &AnchorMatchings,
+                                           const AnchorMap &IRAnchors,
+                                           LocToLocMap &IRToProfileLocationMap);
+  void runStaleProfileMatching(const Function &F, const AnchorMap &IRAnchors,
+                               const AnchorMap &ProfileAnchors,
+                               LocToLocMap &IRToProfileLocationMap);
   void reportOrPersistProfileStats();
 };
 } // end namespace llvm
diff --git a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
index d9284717196f47..251efe32cfbe9b 100644
--- a/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfileMatcher.cpp
@@ -24,8 +24,8 @@ extern cl::opt<bool> SalvageStaleProfile;
 extern cl::opt<bool> PersistProfileStaleness;
 extern cl::opt<bool> ReportProfileStaleness;
 
-void SampleProfileMatcher::findIRAnchors(
-    const Function &F, std::map<LineLocation, StringRef> &IRAnchors) {
+void SampleProfileMatcher::findIRAnchors(const Function &F,
+                                         AnchorMap &IRAnchors) {
   // For inlined code, recover the original callsite and callee by finding the
   // top-level inline frame. e.g. For frame stack "main:1 @ foo:2 @ bar:3", the
   // top-level frame is "main:1", the callsite is "1" and the callee is "foo".
@@ -39,7 +39,7 @@ void SampleProfileMatcher::findIRAnchors(
 
     LineLocation Callsite = FunctionSamples::getCallSiteIdentifier(DIL);
     StringRef CalleeName = PrevDIL->getSubprogramLinkageName();
-    return std::make_pair(Callsite, CalleeName);
+    return std::make_pair(Callsite, Anchor(Callsite, CalleeName));
   };
 
   auto GetCanonicalCalleeName = [](const CallBase *CB) {
@@ -69,7 +69,8 @@ void SampleProfileMatcher::findIRAnchors(
               if (!isa<IntrinsicInst>(&I))
                 CalleeName = GetCanonicalCalleeName(CB);
             }
-            IRAnchors.emplace(LineLocation(Probe->Id, 0), CalleeName);
+            LineLocation Loc = LineLocation(Probe->Id, 0);
+            IRAnchors.emplace(Loc, Anchor(Loc, CalleeName));
           }
         }
       } else {
@@ -84,27 +85,28 @@ void SampleProfileMatcher::findIRAnchors(
         } else {
           LineLocation Callsite = FunctionSamples::getCallSiteIdentifier(DIL);
           StringRef CalleeName = GetCanonicalCalleeName(dyn_cast<CallBase>(&I));
-          IRAnchors.emplace(Callsite, CalleeName);
+          IRAnchors.emplace(Callsite, Anchor(Callsite, CalleeName));
         }
       }
     }
   }
 }
 
-void SampleProfileMatcher::findProfileAnchors(
-    const FunctionSamples &FS,
-    std::map<LineLocation, std::unordered_set<FunctionId>> &ProfileAnchors) {
+void SampleProfileMatcher::findProfileAnchors(const FunctionSamples &FS,
+                                              AnchorMap &ProfileAnchors) {
   auto isInvalidLineOffset = [](uint32_t LineOffset) {
     return LineOffset & 0x8000;
   };
 
+  std::map<LineLocation, std::unordered_set<FunctionId>> ProfileCallsites;
+
   for (const auto &I : FS.getBodySamples()) {
     const LineLocation &Loc = I.first;
     if (isInvalidLineOffset(Loc.LineOffset))
       continue;
     for (const auto &I : I.second.getCallTargets()) {
       auto Ret =
-          ProfileAnchors.try_emplace(Loc, std::unordered_set<FunctionId>());
+          ProfileCallsites.try_emplace(Loc, std::unordered_set<FunctionId>());
       Ret.first->second.insert(I.first);
     }
   }
@@ -116,32 +118,48 @@ void SampleProfileMatcher::findProfileAnchors(
     const auto &CalleeMap = I.second;
     for (const auto &I : CalleeMap) {
       auto Ret =
-          ProfileAnchors.try_emplace(Loc, std::unordered_set<FunctionId>());
+          ProfileCallsites.try_emplace(Loc, std::unordered_set<FunctionId>());
       Ret.first->second.insert(I.first);
     }
   }
+
+  for (const auto &I : ProfileCallsites) {
+    const auto &Loc = I.first;
+    const auto &Callees = I.second;
+    if (Callees.size() == 1) {
+      auto CalleeName = *Callees.begin();
+      ProfileAnchors.emplace(Loc, Anchor(Loc, CalleeName));
+    } else if (Callees.size() > 1) {
+      // use a dummy name(UnknownIndirectCallee) for unknown indrect callee
+      // name.
+      ProfileAnchors.emplace(Loc,
+                             Anchor(Loc, FunctionId(UnknownIndirectCallee)));
+    }
+  }
 }
 
 MyersDiff::DiffResult
-MyersDiff::shortestEditScript(const std::vector<Anchor> &A,
-                              const std::vector<Anchor> &B) const {
-  int32_t N = A.size(), M = B.size(), Max = N + M;
-  auto Index = [&](int32_t I) { return I + Max; };
+MyersDiff::shortestEditScript(const std::vector<Anchor> &AnchorVec1,
+                              const std::vector<Anchor> &AnchorVec2) const {
+  int32_t Size1 = AnchorVec1.size(), Size2 = AnchorVec2.size(),
+          MaxDepth = Size1 + Size2;
+  auto Index = [&](int32_t I) { return I + MaxDepth; };
 
   DiffResult Diff;
-  if (Max == 0)
+  if (MaxDepth == 0)
     return Diff;
 
   // Backtrack the SES result.
   auto Backtrack = [&](const std::vector<std::vector<int32_t>> &Trace,
-                       const std::vector<Anchor> &A,
-                       const std::vector<Anchor> &B, DiffResult &Diff) {
-    int32_t X = N, Y = M;
-    for (int32_t D = Trace.size() - 1; X > 0 || Y > 0; D--) {
-      const auto &P = Trace[D];
+                       const std::vector<Anchor> &AnchorVec1,
+                       const std::vector<Anchor> &AnchorVec2,
+                       DiffResult &Diff) {
+    int32_t X = Size1, Y = Size2;
+    for (int32_t Depth = Trace.size() - 1; X > 0 || Y > 0; Depth--) {
+      const auto &P = Trace[Depth];
       int32_t K = X - Y;
       int32_t PrevK = K;
-      if (K == -D || (K != D && P[Index(K - 1)] < P[Index(K + 1)]))
+      if (K == -Depth || (K != Depth && P[Index(K - 1)] < P[Index(K + 1)]))
         PrevK = K + 1;
       else
         PrevK = K - 1;
@@ -151,21 +169,21 @@ MyersDiff::shortestEditScript(const std::vector<Anchor> &A,
       while (X > PrevX && Y > PrevY) {
         X--;
         Y--;
-        Diff.addEqualLocations(A[X].Loc, B[Y].Loc);
+        Diff.addEqualLocations(AnchorVec1[X].Loc, AnchorVec2[Y].Loc);
       }
 
-      if (D == 0)
+      if (Depth == 0)
         break;
 
       if (Y == PrevY) {
         X--;
 #ifndef NDEBUG
-        Diff.addInsertion(A[X].Loc);
+        Diff.addInsertion(AnchorVec1[X].Loc);
 #endif
       } else if (X == PrevX) {
         Y--;
 #ifndef NDEBUG
-        Diff.addDeletion(B[Y].Loc);
+        Diff.addDeletion(AnchorVec2[Y].Loc);
 #endif
       }
       X = PrevX;
@@ -176,32 +194,32 @@ MyersDiff::shortestEditScript(const std::vector<Anchor> &A,
   // The greedy LCS/SES algorithm.
 
   // An array contains the endpoints of the furthest reaching D-paths.
-  std::vector<int32_t> V(2 * Max + 1, -1);
+  std::vector<int32_t> V(2 * MaxDepth + 1, -1);
   V[Index(1)] = 0;
   // Trace is used to backtrack the SES result.
   std::vector<std::vector<int32_t>> Trace;
-  for (int32_t D = 0; D <= Max; D++) {
+  for (int32_t Depth = 0; Depth <= MaxDepth; Depth++) {
     Trace.push_back(V);
-    for (int32_t K = -D; K <= D; K += 2) {
+    for (int32_t K = -Depth; K <= Depth; K += 2) {
       int32_t X = 0, Y = 0;
-      if (K == -D || (K != D && V[Index(K - 1)] < V[Index(K + 1)]))
+      if (K == -Depth || (K != Depth && V[Index(K - 1)] < V[Index(K + 1)]))
         X = V[Index(K + 1)];
       else
         X = V[Index(K - 1)] + 1;
       Y = X - K;
-      while (X < N && Y < M && A[X] == B[Y])
+      while (X < Size1 && Y < Size2 && AnchorVec1[X] == AnchorVec2[Y])
         X++, Y++;
 
       V[Index(K)] = X;
 
-      if (X >= N && Y >= M) {
+      if (X >= Size1 && Y >= Size2) {
         // Length of an SES is D.
-        Backtrack(Trace, A, B, Diff);
+        Backtrack(Trace, AnchorVec1, AnchorVec2, Diff);
         return Diff;
       }
     }
   }
-  // Length of an SES is greater than Max.
+  // Length of an SES is greater than MaxDepth.
   return Diff;
 }
 
@@ -216,8 +234,7 @@ LocToLocMap SampleProfileMatcher::longestCommonSequence(
 }
 
 void SampleProfileMatcher::matchNonCallsiteLocsAndWriteResults(
-    const LocToLocMap &MatchedAnchors,
-    const std::map<LineLocation, StringRef> &IRAnchors,
+    const LocToLocMap &MatchedAnchors, const AnchorMap &IRAnchors,
     LocToLocMap &IRToProfileLocationMap) {
   auto InsertMatching = [&](const LineLocation &From, const LineLocation &To) {
     // Skip the unchanged location mapping to save memory.
@@ -230,7 +247,7 @@ void SampleProfileMatcher::matchNonCallsiteLocsAndWriteResults(
   SmallVector<LineLocation> LastMatchedNonAnchors;
   for (const auto &IR : IRAnchors) {
     const auto &Loc = IR.first;
-    [[maybe_unused]] StringRef CalleeName = IR.second;
+    [[maybe_unused]] StringRef CalleeName = IR.second.FuncId.stringRef();
     bool IsMatchedAnchor = false;
 
     // Match the anchor location in lexical order.
@@ -291,36 +308,23 @@ void SampleProfileMatcher::matchNonCallsiteLocsAndWriteResults(
 //   [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 std::map<LineLocation, StringRef> &IRAnchors,
-    const std::map<LineLocation, std::unordered_set<FunctionId>>
-        &ProfileAnchors,
-    LocToLocMap &IRToProfileLocationMap) {
+    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");
 
   std::vector<Anchor> ProfileCallsiteAnchors;
-  for (const auto &I : ProfileAnchors) {
-    const auto &Loc = I.first;
-    const auto &Callees = I.second;
-    // Filter out possible indirect calls, use direct callee name as anchor.
-    if (Callees.size() == 1) {
-      auto CalleeName = *Callees.begin();
-      ProfileCallsiteAnchors.emplace_back(Loc, CalleeName);
-    } else if (Callees.size() > 1) {
-      ProfileCallsiteAnchors.emplace_back(Loc,
-                                          FunctionId(UnknownIndirectCallee));
-    }
-  }
+  for (const auto &I : ProfileAnchors)
+    ProfileCallsiteAnchors.emplace_back(I.second);
 
   std::vector<Anchor> IRCallsiteAnchors;
+  // Filter the non-callsite from IRAnchors.
   for (const auto &I : IRAnchors) {
-    const auto &Loc = I.first;
-    const auto &CalleeName = I.second;
-    if (CalleeName.empty())
+    if (I.second.FuncId.stringRef().empty())
       continue;
-    IRCallsiteAnchors.emplace_back(Loc, FunctionId(CalleeName));
+    IRCallsiteAnchors.emplace_back(I.second);
   }
 
   if (IRCallsiteAnchors.empty() || ProfileCallsiteAnchors.empty())
@@ -352,11 +356,11 @@ void SampleProfileMatcher::runOnFunction(Function &F) {
   // Anchors for IR. It's a map from IR location to callee name, callee name is
   // empty for non-call instruction and use a dummy name(UnknownIndirectCallee)
   // for unknown indrect callee name.
-  std::map<LineLocation, StringRef> IRAnchors;
+  AnchorMap IRAnchors;
   findIRAnchors(F, IRAnchors);
   // Anchors for profile. It's a map from callsite location to a set of callee
   // name.
-  std::map<LineLocation, std::unordered_set<FunctionId>> ProfileAnchors;
+  AnchorMap ProfileAnchors;
   findProfileAnchors(*FSFlattened, ProfileAnchors);
 
   // Compute the callsite match states for profile staleness report.
@@ -388,9 +392,8 @@ void SampleProfileMatcher::runOnFunction(Function &F) {
 }
 
 void SampleProfileMatcher::recordCallsiteMatchStates(
-    const Function &F, const std::map<LineLocation, StringRef> &IRAnchors,
-    const std::map<LineLocation, std::unordered_set<FunctionId>>
-        &ProfileAnchors,
+    const Function &F, const AnchorMap &IRAnchors,
+    const AnchorMap &ProfileAnchors,
     const LocToLocMap *IRToProfileLocationMap) {
   bool IsPostMatch = IRToProfileLocationMap != nullptr;
   auto &CallsiteMatchStates =
@@ -411,23 +414,12 @@ void SampleProfileMatcher::recordCallsiteMatchStates(
     // After fuzzy profile matching, use the matching result to remap the
     // current IR callsite.
     const auto &ProfileLoc = MapIRLocToProfileLoc(I.first);
-    const auto &IRCalleeName = I.second;
+    const auto &IRCalleeId = I.second.FuncId;
     const auto &It = ProfileAnchors.find(ProfileLoc);
     if (It == ProfileAnchors.end())
       continue;
-    const auto &Callees = It->second;
-
-    bool IsCallsiteMatched = false;
-    // Since indirect call does not have CalleeName, check conservatively if
-    // callsite in the profile is a callsite location. This is to reduce num of
-    // false positive since otherwise all the indirect call samples will be
-    // reported as mismatching.
-    if (IRCalleeName == SampleProfileMatcher::UnknownIndirectCallee)
-      IsCallsiteMatched = true;
-    else if (Callees.size() == 1 && Callees.count(getRepInFormat(IRCalleeName)))
-      IsCallsiteMatched = true;
-
-    if (IsCallsiteMatched) {
+    const auto &ProfCalleeId = It->second.FuncId;
+    if (IRCalleeId == ProfCalleeId) {
       auto It = CallsiteMatchStates.find(ProfileLoc);
       if (It == CallsiteMatchStates.end())
         CallsiteMatchStates.emplace(ProfileLoc, MatchState::InitialMatch);
@@ -444,8 +436,8 @@ void SampleProfileMatcher::recordCallsiteMatchStates(
   // IR callsites.
   for (const auto &I : ProfileAnchors) {
     const auto &Loc = I.first;
-    [[maybe_unused]] const auto &Callees = I.second;
-    assert(!Callees.empty() && "Callees should not be empty");
+    [[maybe_unused]] StringRef CalleeName = I.second.FuncId.stringRef();
+    assert(!CalleeName.empty() && "Callees should not be empty");
     auto It = CallsiteMatchStates.find(Loc);
     if (It == CallsiteMatchStates.end())
       CallsiteMatchStates.emplace(Loc, MatchState::InitialMismatch);



More information about the llvm-commits mailing list