1118344 - IonMonkey: Inline SIMD.int32x4.add calls.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

Add code in MCallOptimize.cpp such that we can inline calls to SIMD.int32x4.add.   Also, add a test case which can run under Ion, and which exercise the use of this operation with unboxed SIMD operands.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Baseline ICCall_Native accept any templateObject and not only a NativeObject.

Comment 2

[Nicolas B. Pierron [:nbp]]

Attachment: Baseline ICCall_Native records templateObject for js::simd_int32x4_add.

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: IonMonkey: Inline SIMD.int32x4.add calls.

Comment 4

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8545335 [details] [diff] [review]
Baseline ICCall_Native accept any templateObject and not only a NativeObject.

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

::: js/src/jit/MCallOptimize.cpp
@@ +347,5 @@
>  {
>      uint32_t initLength = 0;
>      AllocatingBehaviour allocating = NewArray_Unallocating;
>  
> +    NativeObject *templateObject = &inspector->getTemplateObjectForNative(pc, js_Array)->as<NativeObject>();

This "as" call is not working with --ion-eager, as baseline did not monitor the result of a call yet.
I've changed this code to be:

>    JSObject *templateObject = inspector->getTemplateObjectForNative(pc, js_Array);
>    if (!templateObject)
>        return InliningStatus_NotInlined;
>    ArrayObject *templateArray = &templateObject->as<ArrayObject>();

and renamed all uses of templateObject into templateArray within this function.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8545336 [details] [diff] [review]
Baseline ICCall_Native records templateObject for js::simd_int32x4_add.

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

All good.

::: js/src/jit/BaselineIC.cpp
@@ +9106,5 @@
>          return true;
>      }
>  
> +    if (native == js::simd_int32x4_add) {
> +        Rooted<SimdTypeDescr *> descr(cx, &cx->global()->int32x4TypeDescr().as<SimdTypeDescr>());

you can also use the Int32x4::GetTypeDescr:

Rooted<SimdTypeDescr *> descr(cx, &js::Int32x4::GetTypeDescr(cx->global()));

but that's only slightly better looking, so up to you.

@@ +9107,5 @@
>      }
>  
> +    if (native == js::simd_int32x4_add) {
> +        Rooted<SimdTypeDescr *> descr(cx, &cx->global()->int32x4TypeDescr().as<SimdTypeDescr>());
> +        res.set(TypedObject::createZeroed(cx, descr, 0, gc::TenuredHeap));

(or even add a default value for the second parameter of js::CreateSimd, making it {0,0,0,0}, and use this function here instead of the two lines)

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8545337 [details] [diff] [review]
IonMonkey: Inline SIMD.int32x4.add calls.

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

Nice!

::: js/src/jit/MCallOptimize.cpp
@@ +2721,5 @@
> +    MOZ_ASSERT(inlineTypedObject->typeDescr().as<SimdTypeDescr>().type() == js::Int32x4::type);
> +
> +    // If the type of any of the arguments is neither a SIMD type, an Object
> +    // type, or a Value, then the applyTypes phase will add a fallible box &
> +    // unbox sequence.  This does not matter much the binary arithmetic

nit: forgot 'as' between 'much' and 'the'

Comment 7

[Nicolas B. Pierron [:nbp]]

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

(red on ARM fixed by Bug 1112156)

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14e79b1c87cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/b52cf00c286b
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bedf918aa45

Comment 8

[Nicolas B. Pierron [:nbp]]

Quick Fix to disable inlining of simd_int32x4_add on ARM: (r=jandem)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c39414ca483a

Comment 9

[Nicolas B. Pierron [:nbp]]

jandem's patch to follow the return type change of getTemplateObjectForNative():
https://hg.mozilla.org/integration/mozilla-inbound/rev/1515b55fc761

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/14e79b1c87cb
https://hg.mozilla.org/mozilla-central/rev/b52cf00c286b
https://hg.mozilla.org/mozilla-central/rev/4bedf918aa45
https://hg.mozilla.org/mozilla-central/rev/1515b55fc761
https://hg.mozilla.org/mozilla-central/rev/c39414ca483a