[llvm] [memprof] Simplify readMemprof (NFC) (PR #119930)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Kazu Hirata (kazutakahirata)

<details>
<summary>Changes</summary>

This patch essentially replaces:

  std::pair<const std::vector<Frame> *, unsigned>

with:

  ArrayRef<Frame>

This way, we can store and pass ArrayRef<Frame>, conceptually one
item, instead of the pointer and index.

The only problem is that std::set<ArrayRef<Frame>> doesn't work
because ArrayRef doesn't come with operator<.  This patch works around
the problem by providing CallStackRef, a thin wrapper around
ArrayRef<Frame>.


---
Full diff: https://github.com/llvm/llvm-project/pull/119930.diff


1 Files Affected:

- (modified) llvm/lib/Transforms/Instrumentation/MemProfiler.cpp (+20-12) 


``````````diff
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index ab7d3c70029910..03c9c64e8a1db6 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -763,9 +763,8 @@ static AllocationType addCallStack(CallStackTrie &AllocTrie,
 // non-zero.
 static bool
 stackFrameIncludesInlinedCallStack(ArrayRef<Frame> ProfileCallStack,
-                                   ArrayRef<uint64_t> InlinedCallStack,
-                                   unsigned StartIndex = 0) {
-  auto StackFrame = ProfileCallStack.begin() + StartIndex;
+                                   ArrayRef<uint64_t> InlinedCallStack) {
+  auto StackFrame = ProfileCallStack.begin();
   auto InlCallStackIter = InlinedCallStack.begin();
   for (; StackFrame != ProfileCallStack.end() &&
          InlCallStackIter != InlinedCallStack.end();
@@ -969,10 +968,20 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
   // Build maps of the location hash to all profile data with that leaf location
   // (allocation info and the callsites).
   std::map<uint64_t, std::set<const AllocationInfo *>> LocHashToAllocInfo;
-  // For the callsites we need to record the index of the associated frame in
-  // the frame array (see comments below where the map entries are added).
-  std::map<uint64_t, std::set<std::pair<const std::vector<Frame> *, unsigned>>>
-      LocHashToCallSites;
+  // A thin wrapper around ArrayRef<Frame> to facilitate std::set<CallStackRef>.
+  struct CallStackRef : public ArrayRef<Frame> {
+    CallStackRef(ArrayRef<Frame> CS, unsigned Pos)
+        : ArrayRef<Frame>(CS.drop_front(Pos)) {}
+    // std::set requires std::less.
+    bool operator<(const CallStackRef &R) const {
+      const CallStackRef &L = *this;
+      return std::make_pair(L.data(), L.size()) <
+             std::make_pair(R.data(), R.size());
+    }
+  };
+  // For the callsites we need to record slices of the frame array (see comments
+  // below where the map entries are added).
+  std::map<uint64_t, std::set<CallStackRef>> LocHashToCallSites;
   for (auto &AI : MemProfRec->AllocSites) {
     NumOfMemProfAllocContextProfiles++;
     // Associate the allocation info with the leaf frame. The later matching
@@ -989,7 +998,7 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
     unsigned Idx = 0;
     for (auto &StackFrame : CS) {
       uint64_t StackId = computeStackId(StackFrame);
-      LocHashToCallSites[StackId].insert(std::make_pair(&CS, Idx++));
+      LocHashToCallSites[StackId].emplace(CS, Idx++);
       ProfileHasColumns |= StackFrame.Column;
       // Once we find this function, we can stop recording.
       if (StackFrame.Function == FuncGUID)
@@ -1029,8 +1038,7 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
       // and another callsite).
       std::map<uint64_t, std::set<const AllocationInfo *>>::iterator
           AllocInfoIter;
-      std::map<uint64_t, std::set<std::pair<const std::vector<Frame> *,
-                                            unsigned>>>::iterator CallSitesIter;
+      std::map<uint64_t, std::set<CallStackRef>>::iterator CallSitesIter;
       for (const DILocation *DIL = I.getDebugLoc(); DIL != nullptr;
            DIL = DIL->getInlinedAt()) {
         // Use C++ linkage name if possible. Need to compile with
@@ -1121,8 +1129,8 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
       for (auto CallStackIdx : CallSitesIter->second) {
         // If we found and thus matched all frames on the call, create and
         // attach call stack metadata.
-        if (stackFrameIncludesInlinedCallStack(
-                *CallStackIdx.first, InlinedCallStack, CallStackIdx.second)) {
+        if (stackFrameIncludesInlinedCallStack(CallStackIdx,
+                                               InlinedCallStack)) {
           NumOfMemProfMatchedCallSites++;
           addCallsiteMetadata(I, InlinedCallStack, Ctx);
           // Only need to find one with a matching call stack and add a single

``````````

</details>


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