Make tableswitch trap on out of bounds, rather than having a default.#482
Make tableswitch trap on out of bounds, rather than having a default.#482sunfishcode wants to merge 1 commit intomasterfrom
Conversation
|
Would it be OK to wait for some usage data before implementing this change? I find the disadvantage of the redundant branch to be pretty significant. In the case of the application introducing its own bounds check before the switch (to avoid the trap), the compiler analysis required to eliminate the redundant second branch amounts to an ad-hoc range analysis. It's far easier for an application to put in an |
|
Even a consumer compiler that can derive the range might be better off if the |
|
@titzer's |
|
@titzer Yes, I'm ok taking some time to properly explore this. @jfbastien That wouldn't satisfy my stated goals of making it "simpler and conceptually closer to being "just a jump table"", because when the default isn't unreachable it's still an auxiliary branch tacked on the side, or "consistent with other constructs in WebAssembly (trap on out of bounds, instead of having default behavior)", because it's a default behavior. |
|
Either way, A default target directly exposes this implicit branch-on-range of the You could apply the same argument to traps on memory access out-of-bounds, and indirect call index out-of-bounds, but for memory access at least, it would make implementations that use hardware memory protection to catch out-of-bounds memory accesses harder. |
|
Memory access out-of-bounds and indirect call index out-of-bounds both do trap, so making tableswitch trap instead of having a default would be consistent with them. And, I am contemplating a future where we use hardware mechanisms to bounds-check tableswitch. I'll grant that with reasonably clever optimizers, there's very little effective difference between tableswitch trapping on out-of-bounds, and a tableswitch with a default+unreachable. We can optimize both into the same code. When there is a default and it isn't |
|
There's currently no consensus for this change. |
This is the idea described in WebAssembly/spec#163 , making
tableswitchtrap if the index is out of bounds rather than having an explicit default. This makestableswitchsimpler and conceptually closer to being "just a jump table", and is consistent with other constructs in WebAssembly (trap on out of bounds, instead of having default behavior).The main downside is that in cases when an application emits a range check branch for itself, and the JIT uses a branch to implement the bounds check, we'd get two branches checking the range rather than one. Clever JITs could optimize this away, but simple JITs might not.
Despite my general interest in simple JITs, the factor that I think tips the balance in this case is that
tableswitchis very infrequent relative to most other instructions, so a simple JIT aiming to be as fast as possible should be able to do extra work to recognize the common patterns where it can elide its own branch without a significant overall slowdown.