[PATCH] D68252: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS.

Volodymyr Sapsai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 9 19:08:33 PDT 2019


vsapsai marked 2 inline comments as done.
vsapsai added a comment.

Thanks for the review.



================
Comment at: llvm/include/llvm/ADT/Statistic.h:47
 
-class Statistic {
+class StatisticBase {
 public:
----------------
dsanders wrote:
> Do we actually need the common base class? I'm thinking that since NoopStatistic never registers (and can't because the interfaces to do so changed to TrackingStatistic*), then there's a good chance that nothing reads DebugType, Name, Desc, Value, Initialized and we might be able to save a little memory by eliminating them.
Good point. I've tried to remove the common base class but we are reading Name and Desc at least in [`FusionCandidate::reportInvalidCandidate`](https://github.com/llvm/llvm-project/blob/d6e9e99cec95c83293c68d3b30534e34f53a1923/llvm/lib/Transforms/Scalar/LoopFuse.cpp#L339-L342) and [`reportLoopFusion`](https://github.com/llvm/llvm-project/blob/d6e9e99cec95c83293c68d3b30534e34f53a1923/llvm/lib/Transforms/Scalar/LoopFuse.cpp#L1326-L1331). So I've made `StatisticBase` to store only DebugType, Name, Desc and moved Value and Initialized to `TrackingStatistic`. It saves a little bit of memory and makes `NoopStatistic::getValue()` cheaper as we don't touch atomic.


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

https://reviews.llvm.org/D68252





More information about the cfe-commits mailing list