wasm2c: Remove unnecessary force_read in bounds check mode for performance - #2814
Conversation
|
I'm fine with this for now, but for the longer-term I would love to reduce the forest of preprocessor macros (especially, in this case, preprocessor macros that have variable expansions) used to configure the runtime behavior... this is getting pretty hard to reason about! Are you okay if a future version of wasm2c makes these kinds of choices a transpile-time configuration (when running wasm2c) instead of via a "compile-time" configuration (by setting preprocessor macros)? |
Yup, I agree this needs a cleanup and at least a basic cleanup is on my medium term to-do list as well. Re transpile time vs compile time --- It may make sense to bike shed the exact design a bit prior to any implementation, but in principle, this sounds fine to me. @keithw for the current change, could you sign off on the code review as well, so I can land this? |
3 participants
When using wasm2c with guard pages, the force_read macro (containing an asm block) is used to make sure the C compiler doesn't optimize away memory reads which are dead or move them around in Wasm functions. This is because reads in Wasm have a side-effect --- they can trap if they are out-of-bounds of the linear memory. This macro is however unnecessary for bounds check mode (as the bounds check explicitly captures this side effect), and keeping the asm block around has a large performance impact as it hinders various compiler optimizations. For example, when using a Wasm2c sandboxed libwoff2 font compression library in bounds check mode, removing the force_read macro reduces overhead from 55% to 44%. This PR defines the force_read macro to empty for bounds check mode.