1520162 - Add small codegen improvements for some (TypedArray) self-hosted builtins

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[André Bargull [:anba]]

Test case:

function f() {
    // Note: 16 is the maximum size before V8 starts choosing a different allocation strategy.
    var ta = new Int32Array(16);
    var r = 0
    var t = dateNow()
    for (var i = 0; i < 4000000; ++i) {
        r += ta.slice(0).length;
    }
    return [dateNow() - t, r];
}
for (var i = 0; i < 5; ++i) print(f());

The patches improve one iteration from the test case from ~600ms to ~500ms for me. For comparison: V8 needs ~370ms.

(Another ~40-50ms can be saved when inlining the @@species getter, but that patch still needs some testing...)

---

IonGraph shows the following optimization opportunities:

• We can avoid calling _ConstructorForTypedArray (a non-inlineable native function) in TypedArraySpeciesCreateWithLength when we instead create a custom SpeciesConstructor function for typed arrays which only calls _ConstructorForTypedArray when needed.
• Some MMinMax instructions can be folded away, for example minmax constant43 typedarraylength64 where constant43 is 0 can be removed, because MTypedArrayLength is always >= 0. And there's also minmax typedarraylength64 typedarraylength64 which can be folded to typedarraylength64, because Math.[min,max](x, x) is always equal to x.
  • Folding these instructions allows to fold further instructions later one.

For the test case where slice(0) is called, all minmax and the sub instruction will be removed.

43  constant 0x0
64  typedarraylength phi30
67  minmax [min] constant63 typedarraylength64
72  minmax [min] typedarraylength64 typedarraylength64
75  sub minmax72 minmax67
76  minmax [max] sub75 constant43
101 newtypedarraydynamiclength minmax76

=>

64  typedarraylength phi30
91  newtypedarraydynamiclength typedarraylength64

Comment 1

[André Bargull [:anba]]

Attachment: bug1520162-part1-typedarray-speciesconstructor.patch

This simply copies the existing SpeciesConstructor function from js/src/builtin/Utilities.js and replaces the fallback paths with calls to _ConstructorForTypedArray.

Comment 2

[André Bargull [:anba]]

Attachment: bug1520162-part2-minmax-folding.patch

Adds support to fold Math.[min,max](x, x) to just x and Math.min(typedArrayLength, 0) to 0 and also Math.max(typedArrayLength, 0) to typedArrayLength.

(Isn't it a bit confusing that some of the foldsTo methods have extra type checks for its operands, whereas others simply assume the type policies have already changed the input operands to the correct types. Or is it just me?)

Comment 3

[André Bargull [:anba]]

Attachment: bug1520162-part3-isconstructor-proxy-check-codegen.patch

branchTestClassIsProxy loads the class' flags to check for proxies, but since JSFunction::class_ never has JSCLASS_IS_PROXY set, we can move the proxy check after testing for JSFunction::class_.

Comment 4

[André Bargull [:anba]]

Attachment: bug1520162-part4-validate-typedarray-error-arg.patch

The ValidateTypedArray helper is always called with JSMSG_NON_TYPED_ARRAY_RETURNED, so we might as well remove the parameter and directly bake it into ValidateTypedArray. This change won't have any effect on the generated code, but avoids a stray MConstant(0x1e4) in the final MIR graph, where 0x1e4 is the (current) error code for JSMSG_NON_TYPED_ARRAY_RETURNED.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9036597 [details] [diff] [review]
bug1520162-part1-typedarray-speciesconstructor.patch

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

Having this inlined is nice.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 9036601 [details] [diff] [review]
bug1520162-part2-minmax-folding.patch

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

Nice. I wonder if we could use range analysis to do this in more cases, but that's a can of worms and this seems reasonable.

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 9036604 [details] [diff] [review]
bug1520162-part3-isconstructor-proxy-check-codegen.patch

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

Stealing, LGTM.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 9036605 [details] [diff] [review]
bug1520162-part4-validate-typedarray-error-arg.patch

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

Nicer and simpler.

Comment 9

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=99dd3b557d454857a666e673f8c823727abd8541

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/689577cc5af1
Part 1: Add separate SpeciesConstructor for TypedArrays to save a non-inlined runtime call. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/eeba38937ef4
Part 2: Fold MMinMax with MTypedArrayLength and same operands. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/44969cba88bf
Part 3: Test for proxies after testing for JSFunction in IsConstructor codegen. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fecb51398bd
Part 4: Remove constant error code for ValidateTypeError helper. r=jandem

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/689577cc5af1
https://hg.mozilla.org/mozilla-central/rev/eeba38937ef4
https://hg.mozilla.org/mozilla-central/rev/44969cba88bf
https://hg.mozilla.org/mozilla-central/rev/1fecb51398bd