conv() function in Accelerate.framework can be used.
Accelerate.framework conv() function may be optimized in DirectConvolver.cpp, but unlikely critical.
Created attachment 137657 [details] Patch
Comment on attachment 137657 [details] Patch Attachment 137657 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12429019
Created attachment 137659 [details] Patch
Comment on attachment 137659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137659&action=review Xingnan, thanks for doing this - looks reasonable, but I'd like to have a chance to try out the patch... > Source/WebCore/platform/audio/DirectConvolver.h:40 > +#endif Can we move this include to the .cpp file?
Comment on attachment 137659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137659&action=review Seems to sound fine, passes layout tests, etc. just the two nits with the #include and the #if stuff: > Source/WebCore/platform/audio/DirectConvolver.cpp:88 > +#if OS(DARWIN) Instead of nesting the #if OS(DARWIN) inside of the #if USE(WEBAUDIO_IPP) can we instead use #elif #if USE(WEBAUDIO_IPP) #elif OS(DARWIN) #else #endif
Created attachment 137815 [details] Patch
Comment on attachment 137659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137659&action=review Updated the patch, thanks. >> Source/WebCore/platform/audio/DirectConvolver.cpp:88 >> +#if OS(DARWIN) > > Instead of nesting the #if OS(DARWIN) inside of the #if USE(WEBAUDIO_IPP) > can we instead use #elif > > #if USE(WEBAUDIO_IPP) > #elif OS(DARWIN) > #else > #endif Done. >> Source/WebCore/platform/audio/DirectConvolver.h:40 >> +#endif > > Can we move this include to the .cpp file? Done.
Xingnan, I'm very sorry I didn't see this earlier. I now see that your original approach was better since it's able to share more common code with the un-optimized case. Can we try a new patch similar to patch #2, but with the header file fix? Sorry about that. Also, we *might* need to handle the macro insanity similar to what we do with some of the other vDSP functions in VectorMath.cpp - adding Jer Noble to verify...
Jer, can you please have a look to see if we need the: #if defined(__ppc__) || defined(__i386__) stuff that some other vecLib functions need (in VectorMath.cpp Thanks!
(In reply to comment #10) > Jer, can you please have a look to see if we need the: > > #if defined(__ppc__) || defined(__i386__) > > stuff that some other vecLib functions need (in VectorMath.cpp > > Thanks! Yep. vDSP_conv() is defined as a macro in vDSP_translate.h on x86_64.
Created attachment 137852 [details] Patch
(In reply to comment #12) > Created an attachment (id=137852) [details] > Patch Updated the patch.
Comment on attachment 137852 [details] Patch Thanks Xingnan!
Comment on attachment 137852 [details] Patch Clearing flags on attachment: 137852 Committed r114621: <http://trac.webkit.org/changeset/114621>
All reviewed patches have been landed. Closing bug.