Fix bad code generation on wasm simd butterflies - #185
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While benchmarking a new simd radix4, I noticed very bad performance regressions on wasm. After investigating, I found that the butterflies were being generated with fallback functions for all the simd instrinsics.
I narrowed it down to our "wasm simd fft helper" functions: They take lambdas to process each chunk of input data, and lambdas inherit the target feature of their owning function. Since they were declared in non-target-feature functions, they didn't have it either. The compiler for Wasm simd seems to heavily pessimize inlining, and lambdas are declared with just
#[inline], not#[inline(always)], so we can't force the compiler to inline them. So the final location for the butterfly code was a function that did not have the simd target feature.So this does the next best thing, it changes paradigms so that the "fft helper" is in the body of the butterfly struct (still in the macro of course), so that the lambdas we declare to process data inherit the target feature. This results in the code being much smaller, so it gets reliably inlined, and it will still be correct even if it isn't inlined.
Finally, wasm simd lets you put the target feature attribute directly on the trait methods, so they don't neven need the seperate helper functions.