IonMonkey: Inline SIMD.int32x4.add calls.

RESOLVED FIXED in mozilla38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Trunk
mozilla38
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8545335 [details] [diff] [review]
Baseline ICCall_Native accept any templateObject and not only a NativeObject.
Attachment #8545335 - Flags: review?(jdemooij)
Attachment #8545335 - Flags: feedback?
(Assignee)

Comment 2

4 years ago
Created attachment 8545336 [details] [diff] [review]
Baseline ICCall_Native records templateObject for js::simd_int32x4_add.
Attachment #8545336 - Flags: review?(jdemooij)
Attachment #8545336 - Flags: review?(benj)
(Assignee)

Comment 3

4 years ago
Created attachment 8545337 [details] [diff] [review]
IonMonkey: Inline SIMD.int32x4.add calls.
Attachment #8545337 - Flags: review?(benj)
Attachment #8545335 - Flags: review?(jdemooij) → review+
Attachment #8545336 - Flags: review?(jdemooij) → review+
(Assignee)

Updated

4 years ago
Attachment #8545335 - Flags: feedback?
(Assignee)

Comment 4

4 years ago
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 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)
Attachment #8545336 - Flags: review?(benj) → review+
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'
Attachment #8545337 - Flags: review?(benj) → review+
(Assignee)

Comment 8

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

Comment 9

4 years ago
jandem's patch to follow the return type change of getTemplateObjectForNative():
https://hg.mozilla.org/integration/mozilla-inbound/rev/1515b55fc761
You need to log in before you can comment on or make changes to this bug.