871634 - ion: Rerun GVN after UCE

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Niko Matsakis [:nmatsakis]]

We have found that UCE sometimes exposes better opportunities for GVN. In particular UCE can eliminate dead code paths that contribute undefineds and other stray values into the "typeset".

Comment 1

[Marty Rosenberg [:mjrosenb]]

I'd recommend merging the two passes rather than re-running the GVN.

Comment 2

[Niko Matsakis [:nmatsakis]]

Attachment: Rerun GVN after UCE

Comment 3

[Niko Matsakis [:nmatsakis]]

Merging GVN and UCE? I've thought about it. It seems like a reasonably good idea, if more complicated. Of course, in the general case iteration will always be required to fully optimize (that is, GVN can expose opportunities for UCE and vice versa).

Comment 4

[Niko Matsakis [:nmatsakis]]

h4writer was asking what motivated this change.

Here is evidence of the importance of this optimization (or a similar one, such as merging GVN and UCE):

The parjs-benchmarks/nbody-seeded.js benchmark becomes *twice as fast* using this optimization. The reason is that there is code like:

    foo = self.get(i)[0];
    bar = self.get(i)[1];
    zed = seld.get(i)[2];

Before GVN is applied, there are lots of seemingly important branches. After GVN, we have constant optimized many of those branch tests to false/true. UCE then runs and removes the dead code. When we then *re-run* GVN, we are able to extract out self.get(i) into common code, avoiding many loads and in some cases hoisting bounds checks as well.

Comment 5

[Niko Matsakis [:nmatsakis]]

Here are some measurements from my local machine with an optimized, threadsafe, 32-bit shell build, running v8:

        
             Before   After   %
Crypto       23800    23790   
Deltablue    25052    24616   -1.7%
EarleyBoyer  25873    25843   
NavierStokes 27751    27899   +0.5%
RayTrace     35854    35304   -1.5%
RegExp       3744     3785    +1.1%
Richards     24157    23984   -0.7%
Splay        19662    20259   +3.0%

Comment 6

[Niko Matsakis [:nmatsakis]]

Attachment: Kraken and sunspider comparisons

Comment 7

[Niko Matsakis [:nmatsakis]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=ed904da17ad4

Comment 8

[Niko Matsakis [:nmatsakis]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8f231c38dd5

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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