Improve vectorization in SincResampler.
Created attachment 415438 [details] Patch
Created attachment 415442 [details] Patch
Comment on attachment 415442 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415442&action=review How did you test all four versions? > Source/WebCore/platform/audio/SincResampler.cpp:348 > + vDSP_dotpr(inputP, 1, k1, 1, &sum1, kernelSize); > + vDSP_dotpr(inputP, 1, k2, 1, &sum2, kernelSize); Is there a reason that we don’t write a 4 different implementations of a "dot product" function instead of having the algorithm differ a level above that? > Source/WebCore/platform/audio/SincResampler.cpp:351 > + return static_cast<float>((1.0 - kernelInterpolationFactor) * sum1 + kernelInterpolationFactor * sum2); Is it important do do this as double and then convert back to float? I notice we do that in both versions.
(In reply to Darin Adler from comment #3) > Comment on attachment 415442 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=415442&action=review > > How did you test all four versions? I have an Intel Mac so I was able to test ACCELERATE, SSE and slow implementations. The only implementation I could not test was NEON but this implementation is a straight import from Chromium. CPU(ARM_NEON) does not appear to be true on iPhone unless I tested wrong. > > > Source/WebCore/platform/audio/SincResampler.cpp:348 > > + vDSP_dotpr(inputP, 1, k1, 1, &sum1, kernelSize); > > + vDSP_dotpr(inputP, 1, k2, 1, &sum2, kernelSize); > > Is there a reason that we don’t write a 4 different implementations of a > "dot product" function instead of having the algorithm differ a level above > that? I guess we could do that. I was trying to stay in sync with Chromium here. ACCELERATE() is the only implementation that is not present in Chromium but that I chose to add for better performance on Cocoa platforms. > > > Source/WebCore/platform/audio/SincResampler.cpp:351 > > + return static_cast<float>((1.0 - kernelInterpolationFactor) * sum1 + kernelInterpolationFactor * sum2); > > Is it important do do this as double and then convert back to float? I > notice we do that in both versions. Probably not since our vectorized versions only work with floats. I will test and fix.
Created attachment 415453 [details] Patch
Committed r270457: <https://trac.webkit.org/changeset/270457> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415453 [details].
<rdar://problem/71993191>
Maybe related: https://bugs.webkit.org/show_bug.cgi?id=219676?