[PATCH] D89723: [CSSPGO][llvm-profgen]Context-sensitive profile data generation

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:484-487
+  if (getPerfScriptType() == PERF_LBR_STACK) {
+    // Unwind samples if it's hybird sample
+    unwindSamples();
+  }
----------------
wlei wrote:
> wenlei wrote:
> > What would be the workflow for (non-CS) AutoFDO with this new implementation? 
> > 
> > It looks like `parseTrace` is responsible for aggregation only, then even for AutoFDO, there'll be a post-process after that, to get range:count, right?
> > 
> > so it looks to me that a unified workflow could be something like this?
> > 
> > ```
> > for (auto Filename : PerfTraceFilenames)
> >     parseAndAggregateTrace(Filename);
> > 
> > generateRawProfile(); 
> > ```
> > 
> > In side `generateRawProfile`, we would do simple range overlap computation for AutoFDO, or unwind for CSSPGO.
> > 
> > Also see comments on `AggregationCounter` - in addition to unifying the workflow, it would be good to unify data structure as well if possible. What do you think?
> Good suggestion! As you mention, we can incorporate all into unwinder by treating non-CS profile as hybrid sample with empty call stack. So how about we do that when implementing non-CS part, right now I will change to code like blow?
> 
> ```
> void generateRawProfile (..) {
>   if(getPerfScriptType() == PERF_LBR) {
>      // range overlap computation for regular AutoFdo
>      ...
>     } else if (getPerfScriptType() == PERF_LBR_STACK) {
>     // Unwind samples if it's hybird sample
>     unwindSamples();
>   }
> }
> ```
Yes, that looks good for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89723