[PATCH] D93556: [CSSPGO][llvm-profgen] Compress recursive cycles in calling context
Wei Mi via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 10 21:29:57 PST 2021
wmi added inline comments.
================
Comment at: llvm/test/tools/llvm-profgen/recursion-compression.test:1
+; RUN: llvm-profgen --perfscript=%S/Inputs/recursion-compression.perfscript --binary=%S/Inputs/recursion-compression.perfbin --output=%t --compress-recursion=-1 --show-unwinder-output | FileCheck %s --check-prefix=CHECK-UNWINDER
+; RUN: FileCheck %s --input-file %t
----------------
Could you show the result of --compress-recursion=0 (I assume no compression will happen) so it is easy to check which part of the output is compressed for --compress-recursion=-1?
================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:446
+ const BranchSample &BranchCounter,
+ SmallVector<std::string, 16> &ContextStrStack, ProfiledBinary *Binary) {
for (auto BI : BranchCounter) {
----------------
Use SmallVectorImpl<std::string>& as a parameter type instead of SmallVector<std::string, 16>&. There are some other places with the same issue.
================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:470
FunctionSamples &PseudoProbeCSProfileGenerator::getFunctionProfileForLeaf(
- StringRef PrefixContextId, SmallVector<std::string, 16> &LeafInlinedContext,
+ SmallVector<std::string, 16> &ContextStrStack,
const PseudoProbeFuncDesc *LeafFuncDesc) {
----------------
Use SmallVectorImpl<std::string>& as a parameter type instead of SmallVector<std::string, 16>&.
================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:497
FunctionSamples &PseudoProbeCSProfileGenerator::getFunctionProfileForLeafProbe(
- StringRef PrefixContextId, const PseudoProbe *LeafProbe,
+ SmallVector<std::string, 16> ContextStrStack, const PseudoProbe *LeafProbe,
ProfiledBinary *Binary) {
----------------
Nit: Change it to ContextStrStackCopy to make the vector copy more explicit.
================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.h:96-97
+ while (Right + I < Context.size()) {
+ // Find the first diverging point backwards. When stops, p points at
+ // the diverging point in the current sequence.
+ uint32_t Left = Right;
----------------
Is p a variable or does it refer to some other variable?
================
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);
----------------
Could you give an example of what the sequence looks like after the population?
================
Comment at: llvm/unittests/tools/llvm-profgen/ContextCompressionTest.cpp:14
+
+TEST(TestCompression, TestNoSizeLimit) {
+ SmallVector<std::string, 16> Context = {"m", "a", "a", "b", "c", "a",
----------------
How about have a similar test for pseudo probe?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93556/new/
https://reviews.llvm.org/D93556
More information about the llvm-commits
mailing list