-
Notifications
You must be signed in to change notification settings - Fork 18
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
Enhance "fast unsafe" compilation option #62
Comments
Based on https://github.com/sunfishcode/wasm-reference-manual/blob/master/WebAssembly.md#integer-remainder-signed, the wasm spec differs from modulo operations. There is an llvm srem intrinsic, but it seems to track C and have undefined behavior with INT32_MIN and -1. My logic is just that since division/multiplication can be performed by repeated subtraction/addition, we can add the absolute value of the denominator to be sure that the numerator doesn't overflow, and that results in a numerator between {INT32_MIN, 0}. Since we can't produce a positive number, I don't think that should cause any sign issues. Here's a REPL with some experiments: Perhaps we might want to enable to a "quirks mode" where we drop strict compliance for the wasm spec in certain cases? Sort of like how bash takes a flag for strict POSIX shell standards compliance. I suggest we kick that discussion to an issue. It seems like a good idea to try to target strict compliance first and then consciously downshift if there are perf wins for doing so. |
There are some corners of the WebAssembly spec that are strict and explicit where the C and LLVM specifications remain undefined. An example of this is UINT32_MIN % -1, which overflows with undefined semantics in C and LLVM, but which correctly returns 0 in WebAssembly. Achieving strict compliance thus requires special shims beyond LLVM intrinsics. There may be a performance cost to this.
We can enhance our fast unsafe math compiler option to use a "close enough" LLVM intrinsic in place of a totally compliant implementation in certain cases.
The text was updated successfully, but these errors were encountered: