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

Thu Jan 21 12:27:52 PST 2021

wenlei added inline comments.

================
Comment at: llvm/test/tools/llvm-profgen/recursion-compression.test:1
+; Firstly test uncompression(--compress-recursion=0)
+; RUN: llvm-profgen --perfscript=%S/Inputs/recursion-compression.perfscript --binary=%S/Inputs/recursion-compression.perfbin --output=%t --compress-recursion=0
----------------
wlei wrote:
> wenlei wrote:
> > A test case to cover line-based profile too? That goes through slightly different path for handling the inline context expansion, so e2e test would be good (doesn't have to be as through as the one for probe though).
> Good catch, the line-based test added
Test for `--compress-recursion=0` is gone? and I don't see new test case here? 

================
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
----------------
wlei wrote:
> wenlei wrote:
> > wlei wrote:
> > > wmi wrote:
> > > > 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?
> > > Good idea, added the uncompressed test case
> > Would it be better to place the check for compressed form side by side with uncompressed form for readability?
> Just tried but the problem here is the result is sorted by the total sample, if the order changed after compressed and merged, it couldn't properly match the line. So how about keep the current one or I can give some comments for readability?
I see.. then we can leave it. thanks..

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93556