Add a workaround for Safari 9 ARM iOS right shift by non-immediate zero JIT bug by juj · Pull Request #7191 · emscripten-core/emscripten (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation7 Commits1 Checks0 Files changed
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
[ Show hidden characters]({{ revealButtonHref }})
Perhaps this could be part of LEGACY_VM_SUPPORT?
cc @Brion
I'd like not to put this as part of LEGACY_VM_SUPPORT, since it would be nicer to keep a more fine grained control. E.g. developer might be interesting in supporting old desktop Internet Explorer 9/10/11 users, but not care about Safari on iOS, or other way around. Instead it would be nice to go to opposite direction of a "catch-all" LEGACY_VM_SUPPORT setting, and use feature and/or browser specific things instead. That way developers can tackle code size in a fine grained way even when they need backward compatibility.
@juj I do still think there is a benefit to a catch-all option, people that just want to support all older browsers. But I agree that it shouldn't be the only way for users that want specific things out of that. So how about adding this new option as you suggested, and also enabling it when LEGACY_VM_SUPPORT?
Yeah, having a enable-all sounds good as well. I'll update this PR and add an option for that.
Pushed updated version of this. @kripken does this look good now?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with that name fixed
| @@ -1099,6 +1099,8 @@ def check(input_file): |
|---|
| if shared.Settings.LEGACY_VM_SUPPORT: |
| # legacy vms don't have wasm |
| assert not shared.Settings.WASM, 'LEGACY_VM_SUPPORT is only supported for asm.js, and not wasm. Build with -s WASM=0' |
| shared.Settings.POLYFILL_OLD_MATH_FUNCTIONS = 1 |
| shared.Settings.SUPPORT_IOS_9 = 1 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IOS option has a different name here than elsewhere, which is WORKAROUND_IOS_9_RIGHT_SHIFT_BUG
2 participants
