Skip to content

Add a binaryen-shell pass to fold load/store index offsets into the wasm load/store immediate offsets. #19

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

Closed
ghost opened this issue Dec 21, 2015 · 3 comments

Comments

@ghost
Copy link

ghost commented Dec 21, 2015

In most cases it is fine to fold an asm.js load/store offset into the wasm load/store offset. It will fail on some rare code when the pre-offset index is a signed value (wasm interprets it as unsigned with infinite precision), but to help us get some benchmarks out and develop the binary encoding could this be a command line option that defaults to off?

You might recall a big sage a year ago in emscripten with this being rejected there as 'not wanted', and I had to maintain an emscripten fork with this support, but perhaps people have conceded it's worth the burden now, or that at least it could help us get some testing done until the llvm wasm backend pipeline is more useful. The rebased emscripten patches are still at https://github.com/JSStats/emscripten if they are of any use.

@kripken
Copy link
Member

kripken commented Dec 21, 2015

We could write a simple pass that does this, then it would be easy to do testing on it (just run binaryen-shell -offsetize input.wast).

@ghost ghost changed the title asm2wasm: load/store offsets are not being translated to wasm load/store immediate offsets. Add a binaryen-shell pass to fold load/store index offsets into the wasm load/store immediate offsets. Dec 21, 2015
@sunfishcode
Copy link
Member

Note that it's not semantically valid in all cases to do this transformation at the asm.js level, because asm.js code may depend on the value being wrapped, while wasm's offsets are defined to be non-wrapping. That said, this may be a useful thing to implement for now, because it'll probably work in most cases, and it'll be more representative of what we hope to have in production in the future.

I have patches in progress which implement offset folding with non-wrapping semantics in the LLVM wasm backend using the inbounds property on getelementptr.

@kripken
Copy link
Member

kripken commented Dec 21, 2015

Yeah, definitely not valid to do in general at the asm level. But a pass might be nice for testing, as 99% of the changes are valid to do, so e.g. it would give a good picture of binary size changes even with 1% wrong semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants