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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
