[PATCH] D154737: [BOLT] Add stale-related logging

Sergey Pupyrev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 14 09:33:49 PDT 2023


spupyrev added inline comments.


================
Comment at: bolt/include/bolt/Core/BinaryFunction.h:391-400
+  /// Inference stats:
+  ///   the total number of basic blocks in the profile
+  uint32_t NumStaleBlocks{0};
+  ///   the number matched basic blocks
+  uint32_t NumMatchedBlocks{0};
+  ///   the total count of samples in the profile
+  uint64_t StaleSampleCount{0};
----------------
maksfb wrote:
> spupyrev wrote:
> > maksfb wrote:
> > > Let's move the stats out of the `BinaryFunction`.  Once the input function list is populated, function addresses are fixed. You can use a map, e.g., to attach stats to `BinaryFunction *`. I'm also not sure you need per-function stats or having aggregated is enough.
> > Yes that's what I was thinking on doing. Is there a good example of storing stats or some other info across different passes?
> We don't have a good stats aggregator in BOLT. The easiest way is to create a new class and include its instance in the BC. Another way is to modify the pass manager, which is more involved. Also worth checking what LLVM has implemented.
BC seems to already have something similar, e.g., `BinaryContext::MissedMacroFusionPairs`, although that's not a separate object.
LLVM has `ADT/Statistic.h` which looks similar but implemented via static variables so some changes would be needed to access the stats in different passes.

I guess the best would be to have a stat wrapper in BC and move existing fields (MissedMacroFusionPairs, MissedMacroFusionExecCount) there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154737



More information about the llvm-commits mailing list