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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:182
+
+    // Since tail call frame is missing in stack sample, we intentionally
+    // don't want detect tail call now, otherwise we would be trying to
----------------
The comment could be retired since we have a tail call tracker coming that tracks both in-LBR tail calls and out-of-LBR tail calls universally. 


================
Comment at: llvm/tools/llvm-profgen/llvm-profgen.cpp:37-42
   PerfReader Reader;
   Reader.run();
 
+  std::unique_ptr<ProfileGenerator> Generator =
+      ProfileGenerator::create(&Reader);
+  Generator->generateProfile();
----------------
wlei wrote:
> wenlei wrote:
> > If we let ProfileGenerator be the driver, I think we should also let ProfileGenerator initiate the perf loading (line 38); otherwise if we intend to decouple them, and let PerfReader read profile outside of ProfileGenerator, then it's better only pass the loaded profile to ProfileGenerator for cleaner separation. 
> Good suggestion, change to not include PerfReader in ProfileGenerator, then I also decoupled the unwinder from the reader. for the unwinder, the input is the aggregated hybrid sample, the output is the sample counters which is later forwarded to the generator. 
Perhaps it's better to include the unwinder in the reader since this driver will also handle non-CS profiles in future.

The dataflow from the reader to the profile generator may need a flexible definition (currently is `Unwinder.getSampleCounters()`) for future extension. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89723