[PATCH] D92334: [CSSPGO][llvm-profgen] Pseudo probe decoding and disassembling

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PseudoProbe.h:38
+// the inline context of the probes.
+struct PseudoProbeInlineTree {
+  // Inlinee function GUID
----------------
Please use `class` instead of `struct` to hide member fields like `ProbeVector` and `Children`.


================
Comment at: llvm/tools/llvm-profgen/PseudoProbe.h:69
+// Function descriptor decoded from .pseudo_probe_desc section
+struct PseudoProbeFunction {
+  uint64_t FuncGUID = 0;
----------------
Nit: consider renaming this as `PseudoProbeFuncDesc`.


================
Comment at: llvm/tools/llvm-profgen/PseudoProbe.h:105
+  PseudoProbeInlineTree *InlineTree;
+  const char *PseudoProbeTypeStr[3] = {"Block", "IndirectCall", "DirectCall"};
+  PseudoProbe(uint64_t Ad, uint64_t G, uint32_t I, PseudoProbeType K,
----------------
Declare as static to save space.


================
Comment at: llvm/tools/llvm-profgen/PseudoProbe.h:106
+  const char *PseudoProbeTypeStr[3] = {"Block", "IndirectCall", "DirectCall"};
+  PseudoProbe(uint64_t Ad, uint64_t G, uint32_t I, PseudoProbeType K,
+              uint8_t At, PseudoProbeInlineTree *Tree)
----------------
Nit: add spaces between functions and fields.


================
Comment at: llvm/tools/llvm-profgen/PseudoProbe.h:110
+        InlineTree(Tree){};
+  bool isEntry() {
+    return Index == static_cast<uint32_t>(PseudoProbeReservedId::Last) + 1;
----------------
`const` qualifier for these getters. 


================
Comment at: llvm/tools/llvm-profgen/PseudoProbe.h:127
+  // \p ContextStack is populated in root to leaf order
+  void getInlineContext(std::list<std::string> &ContextStack,
+                        GUIDProbeFunctionMap &GUID2FuncMAP, bool ShowName);
----------------
Nit: consider using `SmallVector` for fewer allocations and better locality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92334