Add pass to print out DebugCounter info

[zhizhouy]

Summary

This patch added a pass to print out {Counter, Skip, StopAfter} info of all passes which have DebugCounter set.

It can be used to monitor how many times does certain transformation happen in a pass, and also help check if -debug-counter option is set correctly.

Please refer to this thread for motivation.

[george.burgess.iv]

Thanks for this!

I don't have a strong feeling for what the right place for this is (e.g. in IR/, ...), and I'm not confident enough with our pass infra and such as a whole to stamp this. I dunno how familiar davide is with these bits; if "not very," we may have to search for someone to poke for a LGTM.

Happily, none of this keeps me from my hobby: nitpicking. :)

include/llvm/IR/DebugCounterPrinter.h

1 ↗

(On Diff #158142)

Nit: Looks like we're missing a '-' to bring this to 80 characters.

(One of the few benefits of having a skinny monitor: wrapping makes things like this a bit more obvious...)

lib/Support/DebugCounter.cpp

105 ↗

(On Diff #158142)

nit: Looks like we might be able to shrink this to CounterNames(RegisteredCounters.begin(), RegisteredCounters.end()); and drop the loop below?

It's a bit unfortunate that the std::map isn't directly accessible in RegisteredCounters, since that would give us exactly what we're looking for without this intermediate vector. I'm OK with having the vector, though, since this'll only be run for debugging purposes anyway...

109 ↗

(On Diff #158142)

nit: please remove the llvm:: (unless it's necessary?)

113 ↗

(On Diff #158142)

Nit: style guide says to use const auto & unless you need a copy, which I don't think we do.

[zhizhouy]

Thanks for reviewing. Comments fixed.

[zhizhouy]

Just noticed that calling print() function at destructor (like what Statistic class did) when option is set will be more straight forward.

Also the original patch could not work for llc or clang. So I modified this patch, not using an extra pass any more.

[zhizhouy]

Ping for the updated approach...

Thanks for reviewing!

[davide]

lib/Support/DebugCounter.cpp

118 ↗

(On Diff #159822)

why do you need to sort the counters here?

[zhizhouy]

Thanks for reviewing.

lib/Support/DebugCounter.cpp

118 ↗

(On Diff #159822)

The reason we sort it here is the order of entries in RegisteredCounters is not stable since it is a map. We want to make sure the each time of the output should be same. So I put it in to a vector with the order of names.

[zhizhouy]

Ping for suggestion, thanks!

[greened]

LGTM, though calling it a "pass" is misleading. It's simply a dump of counter values at the end of execution.

[zhizhouy]

Thanks for reviewing.

though calling it a "pass" is misleading

You're right, the patch was first coming with a pass implementation, but later we find this approach is more direct. Will change the name when submitting it.

[greened]

In D50031#1253407, @zhizhouy wrote:

Thanks for reviewing.

though calling it a "pass" is misleading

You're right, the patch was first coming with a pass implementation, but later we find this approach is more direct. Will change the name when submitting it.

Ok, sounds good. For some reason I'm not able to Accept this so it seems like someone else will need to.

[greened]

Ok, sounds good. For some reason I'm not able to Accept this so it seems like someone else will need to.

Ha! And then somehow I Accepted it. :)

llvm/trunk/include/llvm/Support/DebugCounter.h

 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
 
 namespace llvm {
 
 class DebugCounter {
 public:
+  ~DebugCounter();
+
   /// Returns a reference to the singleton instance.
   static DebugCounter &instance();
 
   // Used by the command line option parser to push a new value it parsed.
   void push_back(const std::string &);
 
   // Register a counter with the specified name.
   //

llvm/trunk/lib/Support/DebugCounter.cpp

 } // namespace
 
 // Create our command line option.
 static DebugCounterList DebugCounterOption(
     "debug-counter", cl::Hidden,
     cl::desc("Comma separated list of debug counter skip and count"),
     cl::CommaSeparated, cl::ZeroOrMore, cl::location(DebugCounter::instance()));
 
+static cl::opt<bool> PrintDebugCounter(
+    "print-debug-counter", cl::Hidden, cl::init(false), cl::Optional,
+    cl::desc("Print out debug counter info after all counters accumulated"));
+
 static ManagedStatic<DebugCounter> DC;
 
+// Print information when destroyed, iff command line option is specified.
+DebugCounter::~DebugCounter() {
+  if (isCountingEnabled() && PrintDebugCounter)
+    print(dbgs());
+}
+
 DebugCounter &DebugCounter::instance() { return *DC; }
 
 // This is called by the command line parser when it sees a value for the
 // debug-counter option defined above.
 void DebugCounter::push_back(const std::string &Val) {
   if (Val.empty())
     return;
   // The strings should come in as counter=value
if (CounterPair.first.endswith("-skip")) {
     Counter.IsSet = true;
   } else {
     errs() << "DebugCounter Error: " << CounterPair.first
            << " does not end with -skip or -count\n";
   }
 }
 
 void DebugCounter::print(raw_ostream &OS) const {
+  SmallVector<StringRef, 16> CounterNames(RegisteredCounters.begin(),
+                                          RegisteredCounters.end());
+  sort(CounterNames.begin(), CounterNames.end());
+
+  auto &Us = instance();
   OS << "Counters and values:\n";
-  for (const auto &KV : Counters)
-    OS << left_justify(RegisteredCounters[KV.first], 32) << ": {"
-       << KV.second.Count << "," << KV.second.Skip << ","
-       << KV.second.StopAfter << "}\n";
+  for (auto &CounterName : CounterNames) {
+    unsigned CounterID = getCounterId(CounterName);
+    OS << left_justify(RegisteredCounters[CounterID], 32) << ": {"
+       << Us.Counters[CounterID].Count << "," << Us.Counters[CounterID].Skip
+       << "," << Us.Counters[CounterID].StopAfter << "}\n";
+  }
 }
 
 LLVM_DUMP_METHOD void DebugCounter::dump() const {
   print(dbgs());
 }

llvm/trunk/test/Other/print-debug-counter.ll

+; RUN: opt -S -debug-counter=early-cse-skip=1,early-cse-count=1 -early-cse \
+; RUN:        -debug-counter=newgvn-vn-skip=1,newgvn-vn-count=2 -newgvn \
+; RUN:        -instcombine -print-debug-counter < %s 2>&1 | FileCheck %s
+;; Test debug counter prints correct info in right order.
+; CHECK-LABEL: Counters and values:
+; CHECK:       early-cse
+; CHECK-SAME:  {4,1,1}
+; CHECK:       instcombine-visit
+; CHECK-SAME:  {12,0,-1}
+; CHECK:       newgvn-vn
+; CHECK-SAME:  {9,1,2}
+define i32 @f1(i32 %a, i32 %b) {
+bb:
+  %add1 = add i32 %a, %b
+  %add2 = add i32 %a, %b
+  %add3 = add i32 %a, %b
+  %add4 = add i32 %a, %b
+  %ret1 = add i32 %add1, %add2
+  %ret2 = add i32 %add3, %add4
+  %ret = add i32 %ret1, %ret2
+  ret i32 %ret
+}
+
+define i32 @f2(i32 %a, i32 %b) {
+bb:
+  %add1 = add i32 %a, %b
+  %add2 = add i32 %a, %b
+  %ret = add i32 %add1, %add2
+  ret i32 %ret
+}