[PATCH] D29998: Add initial support for debug counting

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 18 16:14:52 PST 2017


dberlin marked an inline comment as done.
dberlin added a comment.

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.



================
Comment at: include/llvm/Support/CommandLine.h:1447-1448
 class list : public Option, public list_storage<DataType, StorageClass> {
+protected:
   std::vector<unsigned> Positions;
   ParserClass Parser;
----------------
davide wrote:
> Do you need all these member variables to be protected?
Actually, i don't think we need any of them any more.
I'll check and revert if i can.


================
Comment at: lib/Support/DebugCounter.cpp:25
+    outs() << "  -" << ArgStr;
+    Option::printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + 6);
+    const auto &CounterInstance = DebugCounter::instance();
----------------
davide wrote:
> Add a small comment  to explain the hardcoded `6`.
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.


================
Comment at: lib/Support/DebugCounter.cpp:32
+      outs() << "    =" << Info.first;
+      outs().indent(NumSpaces) << " -   " << Info.second << '\n';
+    }
----------------
davide wrote:
> NumSpaces can probably be folded here, but up to you (if you think it's less readable)
I tried and it creates a really weird line split, so i'm going to leave it ATM.


================
Comment at: lib/Support/DebugCounter.cpp:50
+    return;
+  // The strings should come in as counter=value
+  auto CounterPair = StringRef(Val).split('=');
----------------
davide wrote:
> full stop at the end of the comment
fixed


https://reviews.llvm.org/D29998





More information about the llvm-commits mailing list