Follow-up from bug 877662.

Comment on attachment 761247 [details] [diff] [review] Add SSE2 version of AudioNodeEngine.cpp routines added in bug 815643. r= Review of attachment 761247 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the spec question clarified. ::: content/media/AudioNodeEngineSSE2.cpp @@ +258,5 @@ > + unsigned i; > + __m128 in0, in1, in2, in3, > + outreal0, outreal1, outreal2, outreal3, > + outimag0, outimag1, outimag2, outimag3; > + Can we please assert that aSize is a multiple of 8? @@ +297,5 @@ > + > + _mm_store_ps(&aOutput[i], outreal0); > + _mm_store_ps(&aOutput[i + 4], outreal1); > + _mm_store_ps(&aOutput[i + 8], outreal2); > + _mm_store_ps(&aOutput[i + 12], outreal3); What does the the Web Audio spec say about the expected result of the real-valued product (1 + 0*i)*(Inf + 0*i)? The above will return (Inf + NaN*i) instead of the real number result Inf + 0*i as one might expect (and the NaN will spread to the real values in subsequent operations). @@ +313,5 @@ > + acc0 = _mm_setzero_ps(); > + acc1 = _mm_setzero_ps(); > + acc2 = _mm_setzero_ps(); > + acc3 = _mm_setzero_ps(); > + Can we please assert that aLength is a multiple of 16?

Making a P1 since early results (attached to the bug this came from: bug 877662) showed this could really help with perf. We should reconsider the priority (drop to P2) if we think the perf win isn't going to be big enough.

Karl -- Paul is clearing his plate to focus on bug 848954. Like with bug 877662, can you also pick this one up? Plus, see Comment 4. If you think this should be lowered to a P2, I'm fine with that. Thanks.

Per email discussion with Karl, lowering this to P2.

Hi Karl, Just to be extra clear, I'd also love to have this fixed by end of Q3, but after talking with Paul, I (and Paul) feel it's less important than bug 1172997. (That's what the rank is meant to indicate -- this is a 15, and 1172997 is a 10.)

I talked with Karl today, and he and I agree that it'd be more efficient to find another person to work on this one. Same with Bug 877662.

I was thinking of picking this up on the side. I've written a piece of code a while back that can help aligning the buffers, which is what was left to do here.

Review commit: https://reviewboard.mozilla.org/r/46687/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46687/

Review commit: https://reviewboard.mozilla.org/r/46689/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46689/

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

Comment on attachment 8741698 [details] MozReview Request: Bug 881587 - Use SSE2 version of AudioNodeEngine.cpp routines added in bug 815643. r?padenot https://reviewboard.mozilla.org/r/46689/#review43291 Thanks !

https://hg.mozilla.org/integration/mozilla-inbound/rev/e719cc5de7b7 https://hg.mozilla.org/integration/mozilla-inbound/rev/b1da60432e41

https://hg.mozilla.org/mozilla-central/rev/e719cc5de7b7 https://hg.mozilla.org/mozilla-central/rev/b1da60432e41