Add initial support for debug counting

[• dberlin]

Summary

(Note: I named it debug counting because that's what we called it in
GCC. I'm also mostly leaving this here until i get back to it because
i promised some folks who wanted to use it. I'll ping the review when
it's cleaned up a bit).

We have support for bisection, and bugpoint can reduce testcases
often to a single pass. But that doesn't help reduce it to a single
transform by a single pass. Which debug counting lets us do.

Debug counting lets you instrument a pass so that it only executes a
certain thing (rwhatever you want) after skipping it a certain time of
times, and then only does a certain number of executions before saying
"skip" again.

To make it concrete, for predicateinfo, if i instrument use renaming,
i can make it so it skips renaming the first N uses, renames the next
N, and then skips the rest.

This lets you narrow down a miscompilation to, often, a single
transformation, and then also debug it (by using the same command line
parameters).

[• dberlin]

A few notes:

1. I have scripts that automate binary searching the counters that i'll

upload at some point.

My current scripts only binary search a single counter, but actually, if your
miscompile requires multiple transforms, you can binary search all the
counters.

2. Not sure what to do about counter registration.

It's easy for passes, i think.
For utilities, right now you have to just create a static object that calls registerCounter.

Right now, nothing prints the list of counters.

I could set up some macros like we have STATISTIC to register them, if folks think that is the right approach.

[mehdi_amini]

In D29998#677673, @dberlin wrote:

A few notes:

1. I have scripts that automate binary searching the counters that i'll upload at some point.

llvm/utils/bisect ?

[• dberlin]

In D29998#677742, @mehdi_amini wrote:

In D29998#677673, @dberlin wrote:

A few notes:

1. I have scripts that automate binary searching the counters that i'll upload at some point.

llvm/utils/bisect ?

Right, it's a variant of that that

1. Knows how to format the counters
2. Binary searches the skip, then the count.

[• dberlin]

• Add my current hacked up script to bisect skip and count

[• dberlin]

• Update to be cleaner

[davide]

When I saw this GCC feature I was really jealous =) Thanks for working on this, reviewing it now.

[davide]

include/llvm/Support/DebugCounter.h

70–71 ↗

(On Diff #88911)

Add a diagnostic? ("enable debug if you want counters!") or something?

79–80 ↗

(On Diff #88911)

Can counter ever go below zero?

lib/Support/DebugCounter.cpp

72–74 ↗

(On Diff #88911)

At least can we output something to stderr, here, I think

[• dberlin]

include/llvm/Support/DebugCounter.h

70–71 ↗

(On Diff #88911)

We can't, really.
Because the shouldExecute call happens all the time, whether you are in debug mode or not. It just does nothing with debugging off.
(or at least, that's how GCC works)

79–80 ↗

(On Diff #88911)

We use -1 to say "always execute".
I'll update.

[Bigcheese]

lib/Support/DebugCounter.cpp

19 ↗

(On Diff #88911)

I believe this should be:

explicit DebugCounterList(Mods &&... Ms) : Base(std::forward<Mods>(Ms)...) {}

Which is the idiomatic way to forward arguments.

[• dberlin]

• Update for review comments, make it print options properly and warn.

[• dberlin]

• Add comments

[davide]

LGTM! (after some minor comments inline have been addressed).
I used this (yesterday) to track down an issue internally, so very happy about this feature already.
I'd commit the PredicateInfo bits separately (it would be awesome if you can actually a test for that).

include/llvm/Support/CommandLine.h

1447–1448 ↗

(On Diff #89048)

Do you need all these member variables to be protected?

lib/Support/DebugCounter.cpp

25 ↗

(On Diff #89048)

Add a small comment to explain the hardcoded 6.

28–30 ↗

(On Diff #89048)

Same here.

32 ↗

(On Diff #89048)

NumSpaces can probably be folded here, but up to you (if you think it's less readable)

50 ↗

(On Diff #89048)

full stop at the end of the comment

[mehdi_amini]

I think some documentation in the dev guide would be welcome!

[davide]

In D29998#681089, @mehdi_amini wrote:

I think some documentation in the dev guide would be welcome!

Yes, I agree.

[• dberlin]

I'll commit the predicateinfo bit separately. I added it to the CL as a demo, as everything gets dead-stripped on Darwin if you don't register a counter somewhere.
(it took me a good 30 minutes to figure out what was going on, as the debug-counter option disappears from opt you have no counters.
I'm actually fairly surprised this is legal, but ....)

I believe i can test the predicateinfo piece pretty easily, because we can just have a test where it skips renaming a use and verify that it skips the rename.

include/llvm/Support/CommandLine.h

1447–1448 ↗

(On Diff #89048)

Actually, i don't think we need any of them any more.
I'll check and revert if i can.

lib/Support/DebugCounter.cpp

25 ↗

(On Diff #89048)

It appears to be the option width used in CommandLine.cpp,but it's uncommented there too.
I've at least commented where it comes from.

32 ↗

(On Diff #89048)

I tried and it creates a really weird line split, so i'm going to leave it ATM.

50 ↗

(On Diff #89048)

fixed

llvm/trunk/include/llvm/Support/CommandLine.h

public:
 
   // printOptionInfo - Print out information about this option.  The
   // to-be-maintained width is specified.
   //
   virtual void printOptionInfo(size_t GlobalWidth) const = 0;
 
   virtual void printOptionValue(size_t GlobalWidth, bool Force) const = 0;
 
+  static void printHelpStr(StringRef HelpStr, size_t Indent,
+                           size_t FirstLineIndentedBy);
+
   virtual void getExtraOptionNames(SmallVectorImpl<StringRef> &) {}
 
   // addOccurrence - Wrapper around handleOccurrence that enforces Flags.
   //
   virtual bool addOccurrence(unsigned pos, StringRef ArgName, StringRef Value,
                              bool MultiArg = false);
 
   // Prints option name followed by message.  Always returns true.

llvm/trunk/lib/Support/CMakeLists.txt

add_llvm_library(LLVMSupport
   COM.cpp
   CommandLine.cpp
   Compression.cpp
   ConvertUTF.cpp
   ConvertUTFWrapper.cpp
   CrashRecoveryContext.cpp
   DataExtractor.cpp
   Debug.cpp
+  DebugCounter.cpp
   DeltaAlgorithm.cpp
   DAGDeltaAlgorithm.cpp
   Dwarf.cpp
   Error.cpp
   ErrorHandling.cpp
   FileUtilities.cpp
   FileOutputBuffer.cpp
   FoldingSet.cpp

llvm/trunk/lib/Support/CommandLine.cpp

 
 //===----------------------------------------------------------------------===//
 // cl::alias class implementation
 //
 
 // Return the width of the option tag for printing...
 size_t alias::getOptionWidth() const { return ArgStr.size() + 6; }
 
-static void printHelpStr(StringRef HelpStr, size_t Indent,
+void Option::printHelpStr(StringRef HelpStr, size_t Indent,
                                  size_t FirstLineIndentedBy) {
   std::pair<StringRef, StringRef> Split = HelpStr.split('\n');
   outs().indent(Indent - FirstLineIndentedBy) << " - " << Split.first << "\n";
   while (!Split.second.empty()) {
     Split = Split.second.split('\n');
     outs().indent(Indent) << Split.first << "\n";
   }
 }
 

 void basic_parser_impl::printOptionInfo(const Option &O,
                                         size_t GlobalWidth) const {
   outs() << "  -" << O.ArgStr;
 
   auto ValName = getValueName();
   if (!ValName.empty())
     outs() << "=<" << getValueStr(O, ValName) << '>';
 
-  printHelpStr(O.HelpStr, GlobalWidth, getOptionWidth(O));
+  Option::printHelpStr(O.HelpStr, GlobalWidth, getOptionWidth(O));
 }
 
 void basic_parser_impl::printOptionName(const Option &O,
                                         size_t GlobalWidth) const {
   outs() << "  -" << O.ArgStr;
   outs().indent(GlobalWidth - O.ArgStr.size());
 }
 

 
 // printOptionInfo - Print out information about this option.  The
 // to-be-maintained width is specified.
 //
 void generic_parser_base::printOptionInfo(const Option &O,
                                           size_t GlobalWidth) const {
   if (O.hasArgStr()) {
     outs() << "  -" << O.ArgStr;
-    printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 6);
+    Option::printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 6);
 
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
       size_t NumSpaces = GlobalWidth - getOption(i).size() - 8;
       outs() << "    =" << getOption(i);
       outs().indent(NumSpaces) << " -   " << getDescription(i) << '\n';
     }
   } else {
     if (!O.HelpStr.empty())
       outs() << "  " << O.HelpStr << '\n';
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
       auto Option = getOption(i);
       outs() << "    -" << Option;
-      printHelpStr(getDescription(i), GlobalWidth, Option.size() + 8);
+      Option::printHelpStr(getDescription(i), GlobalWidth, Option.size() + 8);
     }
   }
 }
 
 static const size_t MaxOptWidth = 8; // arbitrary spacing for printOptionDiff
 
 // printGenericOptionDiff - Print the value of this option and it's default.
 //