[PATCH] D89715: [CSSPGO][llvm-profgen]Instruction symbolization

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CallContext.h:21
+// Function name, LineLocation
+typedef std::pair<std::string, LineLocation> SourceLocation;
+
----------------
Name it `FrameLocation`? `SourceLocation` sounds too general and not different from `LineLocation`.


================
Comment at: llvm/tools/llvm-profgen/CallContext.h:24
+// Caller, callee
+typedef SmallVector<SourceLocation, 4> SourceLocationVec;
+
----------------
Name it `FrameLocationStack`? Also would be good to update the comment to be accurate - this is not a caller, callee pair.


================
Comment at: llvm/tools/llvm-profgen/CallContext.h:40
+inline std::string getLocWithContext(const SourceLocationVec &Context) {
+  std::string ContextStr;
+  for (const auto &Callsite : Context) {
----------------
`std::ostringstream` is probably better since there can be quite a few appendings. Same for `getReversedLocWithContext`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89715