[PATCH] D93556: [CSSPGO][llvm-profgen] Compress recursive cycles in calling context

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.h:108-110
+          // Populate the non-common-suffix part of the adjacent sequence.
+          std::copy(BeginIter + Right + 1, BeginIter + Left + I + 1,
+                    BeginIter + End);
----------------
wlei wrote:
> wmi wrote:
> > Could you give an example of what the sequence looks like after the population?
> Sure, an example is added in the code comments
Thanks for the example. I can understand where are the redundent comparisons but I havn't understood how to skip the windows by doing the population (std::copy). I don't see the std::copy change anything for the example below. Could you clarify? 

Similarly I don't see how the duplicated str is removed from Context sequence when Duplication is found above (if (Right - Left == I) is true). Could you also clarify that?


================
Comment at: llvm/unittests/tools/llvm-profgen/ContextCompressionTest.cpp:14
+
+TEST(TestCompression, TestNoSizeLimit) {
+  SmallVector<std::string, 16> Context = {"m", "a", "a", "b", "c", "a",
----------------
wlei wrote:
> wmi wrote:
> > How about have a similar test for pseudo probe?
> Do you mean to add compression test for the element whose type is `PseudoProbe`?
> The pseudo probe in the context key is represented as a `PseudoProbe` pointer, i.e an integer, so it should be same as the string. The only thing for the type T is implementing the `==` operator.
Ok, thanks for the clarification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93556