Closed
Bug 1118344
Opened 9 years ago
Closed 9 years ago
IonMonkey: Inline SIMD.int32x4.add calls.
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: nbp, Assigned: nbp)
References
Details
Attachments
(3 files)
11.59 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
1.59 KB,
patch
|
bbouvier
:
review+
jandem
:
review+
|
Details | Diff | Splinter Review |
8.24 KB,
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
Attachment #8545335 -
Flags: review?(jdemooij)
Attachment #8545335 -
Flags: feedback?
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8545336 -
Flags: review?(jdemooij)
Attachment #8545336 -
Flags: review?(benj)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8545337 -
Flags: review?(benj)
Updated•9 years ago
|
Attachment #8545335 -
Flags: review?(jdemooij) → review+
Updated•9 years ago
|
Attachment #8545336 -
Flags: review?(jdemooij) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8545335 -
Flags: feedback?
Assignee | ||
Comment 4•9 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 5•9 years ago
|
||
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 6•9 years ago
|
||
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 7•9 years ago
|
||
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
Assignee | ||
Comment 8•9 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•9 years ago
|
||
jandem's patch to follow the return type change of getTemplateObjectForNative(): https://hg.mozilla.org/integration/mozilla-inbound/rev/1515b55fc761
Comment 10•9 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•