[PATCH] D95929: [CSSPGO][llvm-profgen] Add brackets for context id to support extended binary format

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:276
+    } else {
+      OCalleeCtxStr << "[";
     }
----------------
hoy wrote:
> wlei wrote:
> > hoy wrote:
> > > Should this be run unconditionally?
> > in line273 `ContextId.rsplit(" @ ").first.str();` has already had the '[', so only add for the else branch
> I see. I'm wondering if it's possible to find a place that can centralize the appending of brackets, say right before populating or looking up the function profiles?
Yeah, I thought about that, the way in this diff seems very error-prone. 

One solution is to modify the ProfileMap at the end (right before it call the `write`), this way is less intrusive. The trade-off is it could slow down the tool. Since we use the StringRef to save string copy, if we add brackets at the end, we have to create new string instance.  

But since our tool is as fast, I‘m inclined to use a less intrusive way. what do you think? 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95929