<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Feb 28, 2013, at 12:42 PM, Jan Voung <<a href="mailto:jvoung@chromium.org">jvoung@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><div dir="ltr">On Thu, Feb 28, 2013 at 9:19 AM, Eli Bendersky<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div class="im">>> I kept NumDAGBlocks and NumFastIselBlocks, because they are called once<br>>> per blocks and not once per instructions. I don't mind if we put them behind<br>>> #ifndef NDEBUG as well.<br>><br>><br>> It's why I was asking, I think it might be an interesting idea to put all<br>> the statistics<br>> behind NDEBUG so we can feel free to collect stats whenever rather than<br>> needing to worry about putting the high firing status under NDEBUG<br>> explicitly.<br><br></div>FWIW, I'm +1 on this. You'll still have to wrap the actual ++counter<br>code with DEBUG or some other macro-fu all over the code, though.<br><br>Eil<br></blockquote></div><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Perhaps we could just re-define the operator++(), operator--(), etc. in Statistics.h to only do real work "#if !defined(NDEBUG) || defined(LLVM_ENABLE_STATS)", and to be nops otherwise. Where LLVM_ENABLE_STATS would be like LLVM_ENABLE_DUMP... If that sounds reasonable, I could spin up a patch.</div></div></div></blockquote><div><br></div><div>This sounds good to me as long as we make the definitions smart enough that the linker will be able to optimize away the file-level globals, too. Having the operators be inline empty functions should be enough, I think.</div><div><br></div><div>In general, I very much like the idea of making the statistics be for debug builds only. llvm::PrintStatistics() should check NDEBUG and inform the user that stats are disabled in the case of release builds, too.</div><div><br></div><div>-Jim</div></div></body></html>