From 18839be9c5c8b9f882dd241769784035b082d4e1 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Mon, 26 Apr 2021 16:47:32 -0700 Subject: [PATCH] [ADT] Remove StatisticBase and make NoopStatistic empty In LLVM_ENABLE_STATS=0 builds, `llvm::Statistic` maps to `llvm::NoopStatistic` but has 3 mostly unused pointers. GlobalOpt considers that the pointers can potentially retain allocated objects, so GlobalOpt cannot optimize out the `NoopStatistic` variables (see D69428 for more context), wasting 23KiB for stage 2 clang. This patch makes `NoopStatistic` empty and thus reclaims the wasted space. The clang size is even smaller than applying D69428 (slightly smaller in both .bss and .text). ``` # This means the D69428 optimization on clang is mostly nullified by this patch. HEAD+D69428: size(.bss) = 0x0725a8 HEAD+D101211: size(.bss) = 0x072238 # bloaty - HEAD+D69428 vs HEAD+D101211 # With D101211, we also save a lot of string table space (.rodata). FILE SIZE VM SIZE -------------- -------------- -0.0% -32 -0.0% -24 .eh_frame -0.0% -336 [ = ] 0 .symtab -0.0% -360 [ = ] 0 .strtab [ = ] 0 -0.2% -880 .bss -0.0% -2.11Ki -0.0% -2.11Ki .rodata -0.0% -2.89Ki -0.0% -2.89Ki .text -0.0% -5.71Ki -0.0% -5.88Ki TOTAL ``` Note: LoopFuse is a disabled pass. For now this patch adds `#if LLVM_ENABLE_STATS` so `OptimizationRemarkMissed` is skipped in LLVM_ENABLE_STATS==0 builds. If these `OptimizationRemarkMissed` are useful in LLVM_ENABLE_STATS==0 builds, we can replace `llvm::Statistic` with `llvm::TrackingStatistic`, or use a different abstraction to keep track of the strings. Similarly, skip the code in `mlir/lib/Pass/PassStatistics.cpp` which calls `getName`/`getDesc`/`getValue`. Reviewed By: lattner Differential Revision: https://reviews.llvm.org/D101211 --- llvm/include/llvm/ADT/Statistic.h | 30 +++++++++++-------------- llvm/lib/Transforms/Scalar/LoopFuse.cpp | 4 ++++ mlir/include/mlir/Pass/Pass.h | 4 ---- mlir/lib/Pass/PassStatistics.cpp | 4 ++++ mlir/test/Pass/pipeline-stats.mlir | 1 + 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/llvm/include/llvm/ADT/Statistic.h b/llvm/include/llvm/ADT/Statistic.h index aa338ccff19a6..de7dabc382c37 100644 --- a/llvm/include/llvm/ADT/Statistic.h +++ b/llvm/include/llvm/ADT/Statistic.h @@ -46,27 +46,22 @@ class raw_ostream; class raw_fd_ostream; class StringRef; -class StatisticBase { +class TrackingStatistic { public: - const char *DebugType; - const char *Name; - const char *Desc; + const char *const DebugType; + const char *const Name; + const char *const Desc; - StatisticBase(const char *DebugType, const char *Name, const char *Desc) - : DebugType(DebugType), Name(Name), Desc(Desc) {} - - const char *getDebugType() const { return DebugType; } - const char *getName() const { return Name; } - const char *getDesc() const { return Desc; } -}; - -class TrackingStatistic : public StatisticBase { -public: std::atomic Value; std::atomic Initialized; TrackingStatistic(const char *DebugType, const char *Name, const char *Desc) - : StatisticBase(DebugType, Name, Desc), Value(0), Initialized(false) {} + : DebugType(DebugType), Name(Name), Desc(Desc), Value(0), + Initialized(false) {} + + const char *getDebugType() const { return DebugType; } + const char *getName() const { return Name; } + const char *getDesc() const { return Desc; } unsigned getValue() const { return Value.load(std::memory_order_relaxed); } @@ -132,9 +127,10 @@ class TrackingStatistic : public StatisticBase { void RegisterStatistic(); }; -class NoopStatistic : public StatisticBase { +class NoopStatistic { public: - using StatisticBase::StatisticBase; + NoopStatistic(const char * /*DebugType*/, const char * /*Name*/, + const char * /*Desc*/) {} unsigned getValue() const { return 0; } diff --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp index 63f02d0269160..ca19913e37ee1 100644 --- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp +++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp @@ -372,11 +372,13 @@ struct FusionCandidate { bool reportInvalidCandidate(llvm::Statistic &Stat) const { using namespace ore; assert(L && Preheader && "Fusion candidate not initialized properly!"); +#if LLVM_ENABLE_STATS ++Stat; ORE.emit(OptimizationRemarkAnalysis(DEBUG_TYPE, Stat.getName(), L->getStartLoc(), Preheader) << "[" << Preheader->getParent()->getName() << "]: " << "Loop is not a candidate for fusion: " << Stat.getDesc()); +#endif return false; } }; @@ -1533,6 +1535,7 @@ struct LoopFuser { assert(FC0.Preheader && FC1.Preheader && "Expecting valid fusion candidates"); using namespace ore; +#if LLVM_ENABLE_STATS ++Stat; ORE.emit(RemarkKind(DEBUG_TYPE, Stat.getName(), FC0.L->getStartLoc(), FC0.Preheader) @@ -1540,6 +1543,7 @@ struct LoopFuser { << "]: " << NV("Cand1", StringRef(FC0.Preheader->getName())) << " and " << NV("Cand2", StringRef(FC1.Preheader->getName())) << ": " << Stat.getDesc()); +#endif } /// Fuse two guarded fusion candidates, creating a new fused loop. diff --git a/mlir/include/mlir/Pass/Pass.h b/mlir/include/mlir/Pass/Pass.h index 10b1a5c57a816..736ad422d346c 100644 --- a/mlir/include/mlir/Pass/Pass.h +++ b/mlir/include/mlir/Pass/Pass.h @@ -139,10 +139,6 @@ class Pass { /// Assign the statistic to the given value. Statistic &operator=(unsigned value); - - private: - /// Hide some of the details of llvm::Statistic that we don't use. - using llvm::Statistic::getDebugType; }; /// Returns the main statistics for this pass instance. diff --git a/mlir/lib/Pass/PassStatistics.cpp b/mlir/lib/Pass/PassStatistics.cpp index d909c98abf563..425aeae35e059 100644 --- a/mlir/lib/Pass/PassStatistics.cpp +++ b/mlir/lib/Pass/PassStatistics.cpp @@ -64,6 +64,7 @@ static void printResultsAsList(raw_ostream &os, OpPassManager &pm) { // If this is not an adaptor, add the stats to the list if there are any. if (!adaptor) { +#if LLVM_ENABLE_STATS auto statistics = pass->getStatistics(); if (statistics.empty()) return; @@ -76,6 +77,7 @@ static void printResultsAsList(raw_ostream &os, OpPassManager &pm) { for (auto &it : llvm::enumerate(pass->getStatistics())) passEntry[it.index()].value += it.value()->getValue(); } +#endif return; } @@ -103,6 +105,7 @@ static void printResultsAsList(raw_ostream &os, OpPassManager &pm) { /// Print the results in pipeline mode that mirrors the internal pass manager /// structure. static void printResultsAsPipeline(raw_ostream &os, OpPassManager &pm) { +#if LLVM_ENABLE_STATS std::function printPass = [&](unsigned indent, Pass *pass) { if (auto *adaptor = dyn_cast(pass)) { @@ -132,6 +135,7 @@ static void printResultsAsPipeline(raw_ostream &os, OpPassManager &pm) { }; for (Pass &pass : pm.getPasses()) printPass(/*indent=*/0, &pass); +#endif } static void printStatistics(OpPassManager &pm, PassDisplayMode displayMode) { diff --git a/mlir/test/Pass/pipeline-stats.mlir b/mlir/test/Pass/pipeline-stats.mlir index 6aee2bfc83145..e75dfe41befca 100644 --- a/mlir/test/Pass/pipeline-stats.mlir +++ b/mlir/test/Pass/pipeline-stats.mlir @@ -1,3 +1,4 @@ +// REQUIRES: asserts // RUN: mlir-opt %s -verify-each=true -pass-pipeline='func(test-stats-pass,test-stats-pass)' -pass-statistics -pass-statistics-display=list 2>&1 | FileCheck -check-prefix=LIST %s // RUN: mlir-opt %s -verify-each=true -pass-pipeline='func(test-stats-pass,test-stats-pass)' -pass-statistics -pass-statistics-display=pipeline 2>&1 | FileCheck -check-prefix=PIPELINE %s