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

[Eli Bendersky]

On Fri, Mar 1, 2013 at 9:44 AM, Jim Grosbach <grosbach at apple.com> wrote:
>
> On Mar 1, 2013, at 9:13 AM, Jan Voung <jvoung at chromium.org> wrote:
>
> I gave it a try and it does look like the linker can optimize away the
> file-level globals as long as the inline operators are empty functions.  It
> optimizes away even more though!
>
> I modified llvm::PrintStatistics to print a warning message if -stats are
> disabled.  However, it looks like llvm::PrintStatistics() is normally only
> ever called if the statistics module is linked in at all, and if any of the
> stats are actually Registered via the "init()" calls in the original
> operators.  Since the inline operators now do nothing, nothing ever gets
> registered.  So there is no printing.  Also, the Statistic object file isn't
> even linked in anymore so the "-stats" commandline option isn't registered
> anymore. You'll get a non-helpful "Unknown command line argument..." instead
> of the message I'd put into PrintStatistics().  We'd need to have some sort
> of dummy statistic in each of the tools (llc, opt, ...) that gets registered
> to force linking and to get llvm::PrintStatistics called on shutdown.
>
>
> While not perfect, I'm OK with the "Unknown command line argument…"
> diagnostic. The situation I wanted to make sure to avoid is having -stats
> report misleading (zero) values for the counters.
>

+1. I don't think it's a big deal. To alleviate it, we can add
"Requires a build with asserts" in parentheses to the description of
-stats, so it will be easy to figure out why one build version of the
tools gives -stats and another doesn't.

> Another problem is that it looks like a lot of the "make check" tests use
> -stats.  For example: test/Analysis/RegionInfo/block_sort.ll.  I think there
> are about 80 of these tests.  Do people run "make check" for Release builds
> instead of a Release+Asserts build?  Otherwise, we'd have to rewrite each of
> the tests to not depend on stats, and it's not clear how to do that for all
> of the tests.
>
>
> Yes; however, I think we can make those tests depend on +Asserts and not run
> otherwise, or be XFAIL or something like that. I don't think we have to
> rewrite them all.

Yes, IMHO the best way to go here would be to ignore these tests
("unsupported") via a lit config setting. Longer term goal could be to
rewrite as many tests as possible not to use stats.

Eli