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

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 15 17:20:33 PDT 2023


wenlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1622
         // top-down processing of functions' annotation.
-        FunctionSamples *OutlineFS = Reader->getOrCreateSamplesFor(*Callee);
         OutlineFS->merge(*FS, 1);
----------------
We can actually remove the `getOrCreateSamplesFor` API from reader now to guarantee it's immutable after profile is read. 


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1626
+        if (!OutlineFS)
+          OutlineFS = &OutlineFunctionSamples[
+              FunctionSamples::getCanonicalFnName(Callee->getName())];
----------------
We could also be inserting into `OutlineFunctionSamples` which can cause rehash while using a FunctionSamples from `OutlineFunctionSamples`? 

I'm wondering if `OutlineFunctionSamples` should use a container that guarantees reference stability, e.g. `std::map` instead of `std::unordered_map`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157061



More information about the llvm-commits mailing list