Statistic: Only print statistics on exit for -stats

[MatzeB]

Summary

Previously enabling the statistics with EnableStatistics() would lead to
them getting printed to stderr/-info-output-file on exit. However
frontends may want a way to enable statistics and do the printing on
their own instead of the forced printing on exit.

This changes the code so that only the -stats option enables printing on
exit, EnableStatistics() only enables the tracking but requires invoking
one of the PrintStatistics() variants.

Also remove the -stats-json option; I am about to propose a better
system for clang which supersedes it.

[bruno]

Hi Matthias,

Thanks for working on this.
You probably want to update Other/statistic.ll to remove the -stats-json tests?

What's your plan regarding the forthcoming output on a file? It seems to me that we can default to JSON whenever the output is anything but the default stderr.

[MatzeB]

In D24819#550961, @bruno wrote:

Hi Matthias,

Thanks for working on this.
You probably want to update Other/statistic.ll to remove the -stats-json tests?

What's your plan regarding the forthcoming output on a file? It seems to me that we can default to JSON whenever the output is anything but the default stderr.

My plan is getting this: https://reviews.llvm.org/D24820. This leaves the choice to the frontend which can either call PrintStatistics(OS) or PrintStatisticsJSON(OS). I consider the -stats llvm option as an easy ad-hoc measure to get some statistics printed; For bigger data gathering you should have support from your frontend to choose a sensible filename to write the statistics to in a machine parseable format.

[bruno]

In D24819#550968, @MatzeB wrote:

In D24819#550961, @bruno wrote:

Hi Matthias,

Thanks for working on this.
You probably want to update Other/statistic.ll to remove the -stats-json tests?

What's your plan regarding the forthcoming output on a file? It seems to me that we can default to JSON whenever the output is anything but the default stderr.

My plan is getting this: https://reviews.llvm.org/D24820. This leaves the choice to the frontend which can either call PrintStatistics(OS) or PrintStatisticsJSON(OS). I consider the -stats llvm option as an easy ad-hoc measure to get some statistics printed; For bigger data gathering you should have support from your frontend to choose a sensible filename to write the statistics to in a machine parseable format.

It seems useful to me to allow json stats to be collected when running from llc and opt as well, don't you think?

[MatzeB]

In D24819#550994, @bruno wrote:

In D24819#550968, @MatzeB wrote:

In D24819#550961, @bruno wrote:

Hi Matthias,

Thanks for working on this.
You probably want to update Other/statistic.ll to remove the -stats-json tests?

What's your plan regarding the forthcoming output on a file? It seems to me that we can default to JSON whenever the output is anything but the default stderr.

My plan is getting this: https://reviews.llvm.org/D24820. This leaves the choice to the frontend which can either call PrintStatistics(OS) or PrintStatisticsJSON(OS). I consider the -stats llvm option as an easy ad-hoc measure to get some statistics printed; For bigger data gathering you should have support from your frontend to choose a sensible filename to write the statistics to in a machine parseable format.

It seems useful to me to allow json stats to be collected when running from llc and opt as well, don't you think?

I was thinking of -stats-json as a stopgap measure until we have frontend integration. But sure we can keep it in, I must admit we should probably have it for the unittest anyway.

[bruno]

I was thinking of -stats-json as a stopgap measure until we have frontend integration. But sure we can keep it in, I must admit we should probably have it for the unittest anyway.

Why not something similar as the FE? That is, if "-stat=file" then default to json, otherwise default stderr output?

[MatzeB]

Leave the -stats-json option in, simplifying the patch.

[bruno]

LGTM

llvm/trunk/lib/Support/Statistic.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cstring>
 using namespace llvm;
 
 /// -stats - Command line option to cause transformations to emit stats about
 /// what they did.
 ///
-static cl::opt<bool>
-Enabled(
-    "stats",
+static cl::opt<bool> Stats("stats",
     cl::desc("Enable statistics output from program (available with Asserts)"));
 
 
 static cl::opt<bool> StatsAsJSON("stats-json",
                                  cl::desc("Display statistics as json data"));
 
+static bool Enabled;
+
 namespace {
 /// StatisticInfo - This class is used in a ManagedStatic so that it is created
 /// on demand (when the first statistic is bumped) and destroyed only when
 /// llvm_shutdown is called.  We print statistics from the destructor.
 class StatisticInfo {
   std::vector<const Statistic*> Stats;
   friend void llvm::PrintStatistics();
   friend void llvm::PrintStatistics(raw_ostream &OS);

 
 /// RegisterStatistic - The first time a statistic is bumped, this method is
 /// called.
 void Statistic::RegisterStatistic() {
   // If stats are enabled, inform StatInfo that this statistic should be
   // printed.
   sys::SmartScopedLock<true> Writer(*StatLock);
   if (!Initialized) {
-    if (Enabled)
+    if (Stats || Enabled)
       StatInfo->addStatistic(this);
 
     TsanHappensBefore(this);
     sys::MemoryFence();
     // Remember we have been registered.
     TsanIgnoreWritesBegin();
     Initialized = true;
     TsanIgnoreWritesEnd();
   }
 }
 
 // Print information when destroyed, iff command line option is specified.
 StatisticInfo::~StatisticInfo() {
+  if (::Stats)
     llvm::PrintStatistics();
 }
 
 void llvm::EnableStatistics() {
-  Enabled.setValue(true);
+  Enabled = true;
 }
 
 bool llvm::AreStatisticsEnabled() {
-  return Enabled;
+  return Enabled || Stats;
 }
 
 void StatisticInfo::sort() {
   std::stable_sort(Stats.begin(), Stats.end(),
                    [](const Statistic *LHS, const Statistic *RHS) {
     if (int Cmp = std::strcmp(LHS->getDebugType(), RHS->getDebugType()))
       return Cmp < 0;
 
if (StatsAsJSON)
     PrintStatisticsJSON(*OutStream);
   else
     PrintStatistics(*OutStream);
 
 #else
   // Check if the -stats option is set instead of checking
   // !Stats.Stats.empty().  In release builds, Statistics operators
   // do nothing, so stats are never Registered.
-  if (Enabled) {
+  if (Stats) {
     // Get the stream to write to.
     std::unique_ptr<raw_ostream> OutStream = CreateInfoOutputFile();
     (*OutStream) << "Statistics are disabled.  "
                  << "Build with asserts or with -DLLVM_ENABLE_STATS\n";
   }
 #endif
 }