Page MenuHomePhabricator

Add pass to print out DebugCounter info
ClosedPublic

Authored by zhizhouy on Jul 30 2018, 5:48 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

zhizhouy created this revision.Jul 30 2018, 5:48 PM

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 updated this revision to Diff 158171.Jul 30 2018, 10:08 PM
zhizhouy marked 4 inline comments as done.

Thanks for reviewing. Comments fixed.

zhizhouy updated this revision to Diff 159822.Aug 8 2018, 4:07 PM
zhizhouy set the repository for this revision to rL LLVM.

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.

Ping for the updated approach...

Thanks for reviewing!

davide added inline comments.Sep 5 2018, 2:03 PM
lib/Support/DebugCounter.cpp
118 ↗(On Diff #159822)

why do you need to sort the counters here?

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.

Ping for suggestion, thanks!

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

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 accepted this revision.Oct 22 2018, 7:13 AM

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.

This revision is now accepted and ready to land.Oct 22 2018, 7:13 AM

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. :)

This revision was automatically updated to reflect the committed changes.