use [reg+reg+offset] addressing mode for wasm memory indexes #2560

MikeHolman · 2017-02-17T05:55:08Z

Change how we index wasm memory to use addressing mode for constant offsets instead of having separate add instructions.

Also, disable constant encoding for IndirOpnds that are used for wasm mem access.

unity                  Left score        Right score       ∆ Score  ∆ Score %  Comment
---------------------  ----------------  ----------------  -------  ---------  --------
Overall                 90531.50 ±0.06%   96132.00 ±0.18%  5600.50      6.19%  Improved
Mandelbrot Script       81022.00 ±0.00%   81395.50 ±0.46%   373.50      0.46%
Instantiate & Destroy   26301.00 ±0.67%   27729.50 ±0.83%  1428.50      5.43%  Improved
CryptoHash Script      169851.50 ±0.18%  178607.00 ±0.37%  8755.50      5.15%  Improved
Animation & Skinning      546.50 ±0.09%     580.00 ±0.00%    33.50      6.13%  Improved
Asteroid Field          24582.50 ±0.32%   26391.00 ±0.10%  1808.50      7.36%  Improved
Physics Spheres          1657.00 ±0.18%    1764.00 ±0.00%   107.00      6.46%  Improved
Physics Spheres          1657.00 ±0.18%    1764.00 ±0.00%   107.00      6.46%  Improved
2D Physics Boxes         2117.00 ±1.04%    2213.50 ±0.47%    96.50      4.56%  Improved
---------------------  ----------------  ----------------  -------  ---------  --------
Total                   11100.34 ±0.30%   11692.81 ±0.27%   592.47      5.34%  Improved

This change is

Cellule · 2017-02-22T00:06:40Z

Reviewed 22 of 22 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/Backend/IRBuilderAsmJs.cpp, line 1524 at r1 (raw file):

    Js::RegSlot indexRegSlot = GetRegSlotFromIntReg(slotIndex);
    IR::RegOpnd * indexOpnd = BuildSrcOpnd(indexRegSlot, TyUint32);
    // don't support encoding in case of

nit: unfinished comment

lib/Backend/LowerMDShared.cpp, line 1172 at r1 (raw file):

    MakeDstEquSrc1(instr);

    if (!needFlags)

I don't fully understand why this is needed

lib/Runtime/ByteCode/OpLayoutsAsmJs.h, line 97 at r1 (raw file):

    {
        // force encode 4 bytes because it can be a value
        uint32                               SlotIndex;

SlotIndex is now always a register thus should have type typename SizePolicy::RegSlotType

Comments from Reviewable

MikeHolman · 2017-02-28T00:50:24Z

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/Backend/IRBuilderAsmJs.cpp, line 1524 at r1 (raw file):

Previously, Cellule (Michael Ferris) wrote…

nit: unfinished comment

removed

lib/Backend/LowerMDShared.cpp, line 1172 at r1 (raw file):

Previously, Cellule (Michael Ferris) wrote…

I don't fully understand why this is needed

Unlike ADD, INC will not set CF flag on unsigned overflow

lib/Runtime/ByteCode/OpLayoutsAsmJs.h, line 97 at r1 (raw file):

Previously, Cellule (Michael Ferris) wrote…

SlotIndex is now always a register thus should have type typename SizePolicy::RegSlotType

Done.

Comments from Reviewable

Cellule · 2017-02-28T19:27:19Z

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

…sm memory indexes Merge pull request #2560 from MikeHolman:wasmarrayindex Change how we index wasm memory to use addressing mode for constant offsets instead of having separate add instructions. Also, disable constant encoding for IndirOpnds that are used for wasm mem access. ```` unity Left score Right score ∆ Score ∆ Score % Comment --------------------- ---------------- ---------------- ------- --------- -------- Overall 90531.50 ±0.06% 96132.00 ±0.18% 5600.50 6.19% Improved Mandelbrot Script 81022.00 ±0.00% 81395.50 ±0.46% 373.50 0.46% Instantiate & Destroy 26301.00 ±0.67% 27729.50 ±0.83% 1428.50 5.43% Improved CryptoHash Script 169851.50 ±0.18% 178607.00 ±0.37% 8755.50 5.15% Improved Animation & Skinning 546.50 ±0.09% 580.00 ±0.00% 33.50 6.13% Improved Asteroid Field 24582.50 ±0.32% 26391.00 ±0.10% 1808.50 7.36% Improved Physics Spheres 1657.00 ±0.18% 1764.00 ±0.00% 107.00 6.46% Improved Physics Spheres 1657.00 ±0.18% 1764.00 ±0.00% 107.00 6.46% Improved 2D Physics Boxes 2117.00 ±1.04% 2213.50 ±0.47% 96.50 4.56% Improved --------------------- ---------------- ---------------- ------- --------- -------- Total 11100.34 ±0.30% 11692.81 ±0.27% 592.47 5.34% Improved ````  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/microsoft/chakracore/2560)

msftclas added the cla-not-required label Feb 17, 2017

MikeHolman requested a review from Cellule February 17, 2017 05:55

MikeHolman force-pushed the wasmarrayindex branch from 9c05f3d to 58fd215 Compare February 17, 2017 06:00

use [reg+reg+offset] addressing mode for wasm memory indexes

e5b7395

MikeHolman force-pushed the wasmarrayindex branch from 58fd215 to e5b7395 Compare February 17, 2017 06:42

dilijev assigned MikeHolman Feb 23, 2017

address review comments

3122e34

chakrabot merged commit 3122e34 into chakra-core:master Feb 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

use [reg+reg+offset] addressing mode for wasm memory indexes #2560

use [reg+reg+offset] addressing mode for wasm memory indexes #2560

MikeHolman commented Feb 17, 2017 •

edited

Loading

Uh oh!

Cellule commented Feb 22, 2017

Uh oh!

MikeHolman commented Feb 28, 2017

Uh oh!

Cellule commented Feb 28, 2017

Uh oh!

use [reg+reg+offset] addressing mode for wasm memory indexes #2560

use [reg+reg+offset] addressing mode for wasm memory indexes #2560

Conversation

MikeHolman commented Feb 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Cellule commented Feb 22, 2017

Uh oh!

MikeHolman commented Feb 28, 2017

Uh oh!

Cellule commented Feb 28, 2017

Uh oh!

MikeHolman commented Feb 17, 2017 •

edited

Loading