[PATCH] D92896: [CSSPGO][llvm-profgen] Virtual unwinding with pseudo probe

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/test/tools/llvm-profgen/inline-cs-pseudoprobe.test:3
+
+; CHECK-UNWINDER: Binary(inline-cs-pseudoprobe.perfbin)'s Range Counter:
+; CHECK-UNWINDER:   (800, 858): 1
----------------
wlei wrote:
> wmi wrote:
> > Add CHECK for context similar as noinline-cs-pseudoprobe.test below? It is good to make sure we get the context right w/wo inline.
> Yeah, this is because the context is empty. added the `CHECK-UNWINDER-EMPTY` for this.
Why the context is empty? To my understanding, for CSSPGO profile, the profile context will be the same no matter how the inlining is happening for the profiled binary, so I expect to see the context just like noinline-cs-pseudoprobe.test. What am I missing?

main:2
 ...
main:2 @ foo:8
 ...


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:99
+                                            std::list<uint64_t> &CallStack) {
+  std::shared_ptr<ProbeBasedCtxKey> ProbeBasedKey =
+      std::make_shared<ProbeBasedCtxKey>();
----------------
wlei wrote:
> wmi wrote:
> > Why it needs to be a shared_ptr?
> This is because the `Hashable` interface is using shared_ptr to allow being the key of unordered_map, I haven't found a workable way for unique_ptr yet, could we leave for later refining? I will keep an eye on this.
> 
I see. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92896