1119303 - MSimdBox should use ooCallVM for allocating TypedObjects.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

So far, we are able to allocate SIMD objects from the nursery (Bug 1112454) as long as we do not have to trigger a Minor GC.  Today, we are unable to do so because we are not yet capable of spilling SIMD registers from a Safepoint (Bug 1112164)

Once this would be possible, this bug should be a minor fix which consist of duplicating the code we have for allocating TypedObjects, and doing the same out-of-line callVM before initializing the rest of the objects with the SIMD register which is popped back from the stack.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Use an oolCallVM to allocate SIMD objects when the nursery is full.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8569374 [details] [diff] [review]
Use an oolCallVM to allocate SIMD objects when the nursery is full.

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

Straightforward.

::: js/src/jit-test/tests/SIMD/nursery-overflow.js
@@ +16,5 @@
> +function test() {
> +  var arr = [];
> +
> +  // overflow the nursery with live SIMD objects.
> +  for (var i = 0; i < 100000; i++) {

According to my calculation, the OolVMCall will be triggered if sizeof(SIMD.int32x4(1,1,1,1)) =~ 170 bytes and I am not sure the typed object overhead is that much... Can you confirm you've manually checked, please (by setting a breakpoint in the ool vm call?)

@@ +25,5 @@
> +}
> +
> +var arr = test();
> +for (var i = 1; i < arr.length; i++)
> +  assertEqX4(i4sub(arr[i], arr[i - 1]), [1, 1, 1, 1]);

Hmmm, you're actually testing i4sub as well here :P Do you mind please changing this to just (starting from i = 0):

assertEqX4(arr[i], [i, i, i, i]);

::: js/src/jit/CodeGenerator.cpp
@@ +4402,1 @@
>  

Any chance to share some code with visitNewTypedObject? This first part seems to be redundant with visitNewTypedObject now.

Comment 3

[Nicolas B. Pierron [:nbp]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #2)
> Comment on attachment 8569374 [details] [diff] [review]
> Use an oolCallVM to allocate SIMD objects when the nursery is full.
> 
> Review of attachment 8569374 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Straightforward.
> 
> ::: js/src/jit-test/tests/SIMD/nursery-overflow.js
> @@ +16,5 @@
> > +function test() {
> > +  var arr = [];
> > +
> > +  // overflow the nursery with live SIMD objects.
> > +  for (var i = 0; i < 100000; i++) {
> 
> According to my calculation, the OolVMCall will be triggered if
> sizeof(SIMD.int32x4(1,1,1,1)) =~ 170 bytes and I am not sure the typed
> object overhead is that much... Can you confirm you've manually checked,
> please (by setting a breakpoint in the ool vm call?)

I did check manually, by using IONFLAGS=bailouts before and after.  And we had 2 bailouts reporting simdbox (way more by adding extra zeros), and of course, no bailouts at all after the patch.

> ::: js/src/jit/CodeGenerator.cpp
> @@ +4402,1 @@
> >  
> 
> Any chance to share some code with visitNewTypedObject? This first part
> seems to be redundant with visitNewTypedObject now.

Creating a member function to factor 3 lines sounds over kill.  The oolCallVM should warn us if we add / remove arguments to the prototype of the function.

Comment 4

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ee3f84412e6

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1ee3f84412e6