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

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb 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};
----------------
spupyrev wrote:
> 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?
Sounds reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154737