[PATCH] D107192: [llvm-profgen][CSSPGO] Support count based aggregated type of hybrid perf script

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/test/tools/llvm-profgen/Inputs/noinline-cs-noprobe.aggperfscript:6
+	    7f68c5788793
+ 0x4005c8/0x4005dc/P/-/-/0  0x40062f/0x4005b0/P/-/-/0  0x400645/0x4005ff/P/-/-/0  0x400637/0x400645/P/-/-/0  0x4005e9/0x400634/P/-/-/0  0x4005d7/0x4005e5/P/-/-/0  0x40062f/0x4005b0/P/-/-/0  0x400645/0x4005ff/P/-/-/0  0x400637/0x400645/P/-/-/0  0x4005e9/0x400634/P/-/-/0  0x4005d7/0x4005e5/P/-/-/0  0x40062f/0x4005b0/P/-/-/0  0x400645/0x4005ff/P/-/-/0  0x400637/0x400645/P/-/-/0  0x4005e9/0x400634/P/-/-/0  0x4005c8/0x4005dc/P/-/-/0
+2
----------------
Can you remove the `/P/-/-/0` part and see if the existing parser can handle both formats? That part is unnecessary.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:66
+  PERF_LBR_STACK = 3,     // Hybrid sample including call stack and LBR stack.
+  PERF_LBR_STACK_AGG = 4, // Aggregated hybrid sample perf
 };
----------------
wenlei wrote:
> How about we make the aggregation form an "extension" applicable to any perf script type instead of a new type? This is because we could have a count for LBR perf script too (when we use llvm-profgen for line-based AutoFDO too in the future). If we make aggregation a separate type, we would potentially need to double the number of types.. 
+1, this is a good idea.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107192