[PATCH] D157061: [SampleProfile] Potential use after move in SampleProfileLoader::promoteMergeNotInlinedContextSamples

[William Junda Huang via Phabricator via llvm-commits]

huangjd updated this revision to Diff 547434.
huangjd added a comment.

Updated comment, remove unnecessary reset of SampleProfileMatcher


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157061/new/

https://reviews.llvm.org/D157061

Files:
  llvm/lib/Transforms/IPO/SampleProfile.cpp


Index: llvm/lib/Transforms/IPO/SampleProfile.cpp
===================================================================
--- llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -565,6 +565,7 @@
   void promoteMergeNotInlinedContextSamples(
       MapVector<CallBase *, const FunctionSamples *> NonInlinedCallSites,
       const Function &F);
+  void updateFunctionSamplesPointers(const Function &F);
   std::vector<Function *> buildFunctionOrder(Module &M, LazyCallGraph &CG);
   std::unique_ptr<ProfiledCallGraph> buildProfiledCallGraph(Module &M);
   void generateMDProfMetadata(Function &F);
@@ -1581,6 +1582,7 @@
 void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
     MapVector<CallBase *, const FunctionSamples *> NonInlinedCallSites,
     const Function &F) {
+  std::vector<std::pair<Function *, FunctionSamples>> NewFunctionSamples;
   // Accumulate not inlined callsite information into notInlinedSamples
   for (const auto &Pair : NonInlinedCallSites) {
     CallBase *I = Pair.first;
@@ -1619,10 +1621,16 @@
         // Note that we have to do the merge right after processing function.
         // This allows OutlineFS's profile to be used for annotation during
         // top-down processing of functions' annotation.
-        FunctionSamples *OutlineFS = Reader->getOrCreateSamplesFor(*Callee);
-        OutlineFS->merge(*FS, 1);
-        // Set outlined profile to be synthetic to not bias the inliner.
-        OutlineFS->SetContextSynthetic();
+        // We handle create new samples later because it can trigger a rehash,
+        // invalidating all pointers to FunctionSamples, so we need to deep
+        // copy the original FunctionSamples and handle all insertions at once.
+        FunctionSamples *OutlineFS = Reader->getSamplesFor(*Callee);
+        if (OutlineFS != nullptr) {
+          OutlineFS->merge(*FS, 1);
+          // Set outlined profile to be synthetic to not bias the inliner.
+          OutlineFS->SetContextSynthetic();
+        } else
+          NewFunctionSamples.emplace_back(Callee, *FS);
       }
     } else {
       auto pair =
@@ -1630,6 +1638,40 @@
       pair.first->second.entryCount += FS->getHeadSamplesEstimate();
     }
   }
+  for (const auto &Pair : NewFunctionSamples) {
+    FunctionSamples *OutlineFS = Reader->getOrCreateSamplesFor(*Pair.first);
+    OutlineFS->merge(Pair.second, 1);
+    // Set outlined profile to be synthetic to not bias the inliner.
+    OutlineFS->SetContextSynthetic();
+  }
+  // Insertion to the sample profile map may invalidates all pointers to
+  // existing FunctionSamples, so we need to update any fields with them. This
+  // only happens occasionally.
+  if (NewFunctionSamples.size() > 0) {
+    updateFunctionSamplesPointers(F);
+  }
+}
+
+/// Update all fields containing a pointer to FunctionSamples, used after the
+/// sample profile map is rehashed because FunctionSamples may be moved.
+void SampleProfileLoader::updateFunctionSamplesPointers(const Function &F) {
+  assert(Reader->profileIsCS() == FunctionSamples::ProfileIsCS);
+  if (Reader->profileIsCS())
+    ContextTracker = std::make_unique<SampleContextTracker>(
+        Reader->getProfiles(), &GUIDToFuncNameMap);
+  if (FunctionSamples::ProfileIsCS)
+    Samples = ContextTracker->getBaseSamplesFor(F);
+  else
+    Samples = Reader->getSamplesFor(F);
+  assert(Samples && !Samples->empty());
+  for (auto &DILocation2Sample : DILocation2SampleMap) {
+    auto DIL = DILocation2Sample.first;
+    auto &Sample = DILocation2Sample.second;
+    if (FunctionSamples::ProfileIsCS)
+      Sample = ContextTracker->getContextSamplesFor(DIL);
+    else
+      Sample = Samples->findFunctionSamples(DIL, Reader->getRemapper());
+  }
 }
 
 /// Returns the sorted CallTargetMap \p M by count in descending order.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157061.547434.patch
Type: text/x-patch
Size: 3824 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230805/40edb789/attachment.bin>