-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[wasm] Do not build mono libs with -msimd128
#89502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Make it optional, build only minimal set of code witch required `-msimd128` to separate library. Also provide "stub" nosimd version of this library. Choose the appropriate library during linking.
|
could we have WBT for this ? |
yeah, I plan to add tests later in another PR, with wa-info involved. |
kg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% confident in this, but it looks mergeable to me. One big caveat:
The result of this is that the EnableSIMD != 'true' scenario will be much slower than it was before I added interp PackedSimd support. Before, we had support for all the non-packedsimd SIMD APIs in the interpreter, even if the actual implementations were not using native WebAssembly SIMD. Those APIs resulted in sizable performance improvements. Because you're now disabling all SIMD instead of just PackedSimd, users with EnableSIMD set to false will probably experience performance worse than net7.
AFAIK the only part of the interpreter that actually relies on msimd128 is PackedSimd.
|
/backport to release/8.0-rc1 |
|
Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5892974848 |
|
We should add a test to build, and run an app with simd turned off. |
Make it optional, move minimal set of code, which requires
-msimd128, to separate library. Also provide "stub" nosimd version of that library.Choose the appropriate library during app linking.
Part of solution for #89302