Bug 38446 - SunSpider: all four bitops benchmarks can be replaced with NOP
Summary: SunSpider: all four bitops benchmarks can be replaced with NOP
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-02 20:55 PDT by Nicholas Nethercote
Modified: 2011-07-02 14:09 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.47 KB, patch)
2011-07-02 13:34 PDT, Maciej Stachowiak
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nicholas Nethercote 2010-05-02 20:55:35 PDT
There are four bitops benchmarks in SunSpider:  3bit-bits-in-byte, bits-in-byte, bitwise-and, nsieve-bits.  All of them compute things but never do anything with the computed values which means a sufficiently clever compiler can optimise them all down to a single NOP.

I noticed this because I'm working on improving dead code elimination for Mozilla's TraceMonkey, which is now sufficiently clever to turn this (from 3bit-bits-in-byte):

  for(var y=0; y<256; y++) func(y);

into an empty loop, because func() has no side effects and its result isn't used.  While this provides me personally with a certain level of satisfaction it doesn't seem a desirable property for a benchmark.

Note that some of these contain empty loops like this:

  for(var x=0; x<500; x++)

which are presumably there to "spin up" the benchmark, but can also be eliminated entirely.

It's possible that some of the other SunSpider benchmarks have the same undesirable property, I haven't checked;  the bitops ones are certainly the smallest and so the most affected.  They should be changed to record a value and print it out at the end, something like that.
Comment 1 Nicholas Nethercote 2010-05-16 15:32:02 PDT
I noticed recently that string-validate-input.js also has this property.  Because that benchmark is dominated by string concatenation, a JS engine can over-optimize this benchmark by applying a lazy concatenation approach -- ie. build a tree of to-be-concatenated strings and then concatenate them when needed.  Except in this string-validate-input.js the final string is never used so the actual final concatenation step is not needed.

The final string should be used.  Printing the whole thing out is a bad idea, it's 420,000 chars and so I/O costs would dominate.  Maybe printing out every 1000th char would be ok?  That provide a reasonable amount of "use" of the final string so that over-optimizing the benchmark would be difficult.  Or computing a really simple hash of the string would be a more thorough use.  bitops-bitwise-and.js provides an example of a really simple hash that could be used -- just '&' together every char.
Comment 2 Maciej Stachowiak 2011-07-02 13:06:21 PDT
(In reply to comment #1)
> I noticed recently that string-validate-input.js also has this property.  Because that benchmark is dominated by string concatenation, a JS engine can over-optimize this benchmark by applying a lazy concatenation approach -- ie. build a tree of to-be-concatenated strings and then concatenate them when needed.  Except in this string-validate-input.js the final string is never used so the actual final concatenation step is not needed.
> 
> The final string should be used.  Printing the whole thing out is a bad idea, it's 420,000 chars and so I/O costs would dominate.  Maybe printing out every 1000th char would be ok?  That provide a reasonable amount of "use" of the final string so that over-optimizing the benchmark would be difficult.  Or computing a really simple hash of the string would be a more thorough use.  bitops-bitwise-and.js provides an example of a really simple hash that could be used -- just '&' together every char.

I filed bug 63864 for this latter issue.
Comment 3 Maciej Stachowiak 2011-07-02 13:34:41 PDT
Created attachment 99558 [details]
Patch
Comment 4 Maciej Stachowiak 2011-07-02 14:09:50 PDT
Committed r90319: <http://trac.webkit.org/changeset/90319>