1338998 - Wasm baseline: Align branch targets (speculative, investigation, x86/x64)

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P3)

Description

[Lars T Hansen [:lth]]

In bug 1337367 comment 0 there is a note near the end that not aligning branch targets (on x86/x64) can have a large negative impact in tight loops.  As the baseline compiler is simple and performs branches and deposits labels in only a few places, it should be fairly straightforward to experiment with this to see if it yields anything.

(More a good second bug than a good first bug.)

Comment 1

[Lars T Hansen [:lth]]

See bug 1345070 comment 3 for a situation where a microbenchmark is significantly affected by this.

After I filed the present bug somebody made a comment on IRC that the real impact of alignment is mostly in the noise, based on experiments we've done.  I'm willing to believe that.  Even so, tight loops can be affected by it, especially in Wasm (where they are tighter and less branchy than JS).

This suggests, perhaps (and not necessarily for a baseline compiler :) that for sanity of benchmarking we might be more aggressive about aligning when we are trying to pin down performance bugs, so maybe there's space for a configuration switch for that.

Comment 2

[Lars T Hansen [:lth]]

Attachment: bug1338998-align-loops.patch

See previous comments on this bug.  I'd like to land this, not so much because I think it makes a big difference on real-world code, but because it makes benchmark measurements less variable, and we should take that stability at this low cost.

To support this I offer some measurements made yesterday, for the tls optimization for baseline.  The first run is without alignment, the second run is with loop alignment.  Entries marked '*' are those that showed "significant" (> 0.01 ratio) change in the previous run but not in the latter.

(Comparing no tls opt to tls opt, baseline-vs-baseline)

               Unopt    Opt    Unopt/Opt

Without alignment

box2d           2083    2000    1.041
bullet          2835    2580    1.098
conditionals    793     753     1.053
copy            1192    1194     .998
corrections     1455    1463     .994
fannkuch        5682    5579    1.018
fasta           2325    2258    1.029
ifs             304     299     1.016
linpack         15226   13683   1.112
lua_binarytrees 7811    7640    1.022
lua_scimark     9216    8873    1.038
memops          3381    3379    1.000
primes          1056    1143     .923  (???)
skinning        3414    3229    1.057
zlib            2619    2411    1.086

With alignment

box2d           2078    1979    1.050
bullet          2842    2568    1.106
conditionals    730     728     1.002  *
copy            1191    1187    1.003
corrections     1456    1455    1.000
fannkuch        5755    5552    1.036
fasta           2340    2348     .996  *
ifs             301     299     1.006  *
linpack         15083   13578   1.110
lua_binarytrees 7763    7641    1.015
lua_scimark     9124    8944    1.020
memops          3368    3368    1.000
primes          1054    1052    1.001  *
skinning        3404    3202    1.063
zlib            2608    2402    1.085

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8844910 [details] [diff] [review]
bug1338998-align-loops.patch

Review of attachment 8844910 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense. We've done so in Ion, so it makes sense to have this for the baseline compiler too. Thanks for the patch!

Comment 4

[Lars T Hansen [:lth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e73b4461c7d08a9bcbaf0dc164ade6daa80d93d5
Bug 1338998 - wasm baseline, align loop headers for perf testing sanity's sake. r=bbouvier

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e73b4461c7d0