[PATCH] D100090: [CSSPGO] Fix dangling context strings and improve profile order consistency and error handling

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:338
+
+  // Remove the code profile from ProfileMap and merge them into BaseProileMap
+  StringMap<FunctionSamples> BaseProfileMap;
----------------
Nit: cold profile


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:388
+    assert(Ret.second && "Conext conflict during canonicalization");
+    FProfile = Ret.first->second;
+
----------------
Still trying to understand. So for cold profile trim, what kind of a profile can be here. In `trimAndMergeColdContextProfiles` all cold profiles are removed and all based profiles are added into `ProfileMap`.


================
Comment at: llvm/lib/ProfileData/SampleProfWriter.cpp:503
   for (auto N : V) {
     OS << N;
     encodeULEB128(0, OS);
----------------
wenlei wrote:
> hoy wrote:
> > Can brackets be output here instead of adding it to name table and looking it up there?
> Not every name needs bracket. One example is the call target name for call sites. If we do it here, we can't differentiate between such cases and the full context names used in profile header. 
I see. Yeah, unfortunately we cannot tell which one needs brackets or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100090