-
Notifications
You must be signed in to change notification settings - Fork 6
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
[libmad] Fix WASM build by disabling asm-based optimizations (#37088) #135
Conversation
…ft#37088) Fixes microsoft#37087 This disables architecture-specific optimizations when targeting WASM. The root issue is that `libmad`'s CMake configuration seems to detect WASM as an Intel CPU (via `CMAKE_SYSTEM_PROCESSOR`) and then tries to use arch-specific assembly, which results in the build errors from the linked issue. - [x] Changes comply with the [maintainer guide](https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md). - [x] SHA512s are updated for each updated download. - [x] The "supports" clause reflects platforms that may be fixed by this new version. - [x] Any fixed [CI baseline](https://github.com/microsoft/vcpkg/blob/master/scripts/ci.baseline.txt) entries are removed from that file. - [x] Any patches that are no longer applied are deleted from the port's directory. - [x] The version database is fixed by rerunning `./vcpkg x-add-version --all` and committing the result. - [x] Only one version is added to each modified port's versions file.
# we have to override these flags: | ||
# https://codeberg.org/tenacityteam/libmad/src/commit/84ba587793d61caadf6d1f6c0d94c3e165874a50/CMakeLists.txt | ||
if(VCPKG_TARGET_IS_EMSCRIPTEN) | ||
list(APPEND EXTRA_OPTIONS "-DFPM_64BIT=OFF -DFPM_INTEL=OFF -DFPM_DEFAULT=ON") |
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 consider FPM_DEFAULT as broken:
it loses significant accuracy
This is due to a limited 32 bit precision compared to 64.
Does FPM_64BIT compile?
It might be accurate, but slow:
https://emscripten.org/docs/porting/guidelines/portability_guidelines.html#code-that-compiles-but-might-run-slowly
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.
Interestingly, wasm32 does support 64-bit integer operations: https://groups.google.com/g/emscripten-discuss/c/nWmO3gi8_Jg
Does FPM_64BIT compile?
It seems to. Haven't really had the time to dig in whether this works and/or whether we need WASM_BIGINT
though. Currently prioritizing getting audio to work at all before digging into optimization...
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.
For my understanding WASM_BIGINT supports native 64 bit integers. Without, it is emulated by using 32 bit integers which is slow.
Can we check for |
A small follow-up to microsoft#37088, which unintentionally disabled asm optimizations on non-WASM platforms. This PR adds `aso` to the set of default features, which should fix this. - [x] Changes comply with the [maintainer guide](https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md). - [x] SHA512s are updated for each updated download. - [x] The "supports" clause reflects platforms that may be fixed by this new version. - [x] Any fixed [CI baseline](https://github.com/microsoft/vcpkg/blob/master/scripts/ci.baseline.txt) entries are removed from that file. - [x] Any patches that are no longer applied are deleted from the port's directory. - [x] The version database is fixed by rerunning `./vcpkg x-add-version --all` and committing the result. - [x] Only one version is added to each modified port's versions file.
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.
LGTM, Thank you.
Cherry-pick of microsoft#37088 onto 2.5.