[llvm] r176214 - The FastISEL should be fast. But when we record statistics we use atomic operations to increment the counters.

[Jim Grosbach]

On Feb 28, 2013, at 12:42 PM, Jan Voung <jvoung at chromium.org> wrote:

> On Thu, Feb 28, 2013 at 9:19 AM, Eli Bendersky <eliben at google.com> wrote:
> >> I kept NumDAGBlocks and NumFastIselBlocks, because they are called once
> >> per blocks and not once per instructions. I don't mind if we put them behind
> >> #ifndef NDEBUG as well.
> >
> >
> > It's why I was asking, I think it might be an interesting idea to put all
> > the statistics
> > behind NDEBUG so we can feel free to collect stats whenever rather than
> > needing to worry about putting the high firing status under NDEBUG
> > explicitly.
> 
> FWIW, I'm +1 on this. You'll still have to wrap the actual ++counter
> code with DEBUG or some other macro-fu all over the code, though.
> 
> Eil
> 
> 
> 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.

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.

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.

-Jim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130228/1f8c001e/attachment.html>