1263910 - Optimize ReverbAccumulationBuffer code to use SSE2 calls when possible

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Dan Minor [:dminor]]

The ReverbAccumulationBuffer has to deal with wrap around which makes it unlikely that the read and write pointers will be properly aligned and the number of values to write will be a multiple of 16.

We should attempt to do enough scalar operations to get things properly aligned and then switch to vector operations rather than doing everything with scalar operations.

Comment 1

[Karl Tomlinson (:karlt)]

AFAIR ReverbAccumulationBuffer is now only used with blocks of WEBAUDIO_BLOCK_SIZE, so providing necessary alignment may just require making the length of ReverbAccumulationBuffer::m_buffer a multiple of 4.

Comment 2

[Dan Minor [:dminor]]

Attachment: MozReview Request: Bug 1263910 - Make AudioBufferAddWithScale handle unaligned buffers; r=padenot

ReverbAccumulationBuffer often produces unaligned buffers due to the way
it wraps around results. This modifies AudioBufferAddWithScale on SSE2
platforms to handle unaligned buffers by performing scalar operations
until both the input and output buffers are aligned to 16 bytes. It then
does as many vector operations as possible and switches back to scalar
operations for anything that is left over.

This could also be done within the ReverbAccumulationBuffer code but doing
it directly within the AudioNodeEngine code makes it available to other
callers in the future, at the cost of a few extra branches in the case
where everything was aligned anyway.

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

Comment 3

[Paul Adenot (:padenot)]

Comment on attachment 8751335 [details]
MozReview Request: Bug 1263910 - Make AudioBufferAddWithScale handle unaligned buffers; r=padenot

https://reviewboard.mozilla.org/r/51959/#review49085

::: dom/media/webaudio/AudioNodeEngine.cpp:85
(Diff revision 1)
>  #ifdef USE_SSE2
>    if (mozilla::supports_sse2()) {
> -    AudioBufferAddWithScale_SSE(aInput, aScale, aOutput, aSize);
> -    return;
> +    if (aScale == 1.0f) {
> +      while (aSize && (!IS_ALIGNED16(aInput) || !IS_ALIGNED16(aOutput))) {
> +        *aOutput += *aInput;
> +        ++aOutput; ++aInput; --aSize;

Each on its line, please, and below.

Comment 4

[Dan Minor [:dminor]]

Comment on attachment 8751335 [details]
MozReview Request: Bug 1263910 - Make AudioBufferAddWithScale handle unaligned buffers; r=padenot

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51959/diff/1-2/

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/24e883121fcd

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/24e883121fcd