Switch wasm emission to a custom encoder #30
Conversation
alexcrichton
force-pushed
the
manually-serialize
branch
from
8ec9613
to
cf91ebd
Compare
src/encode.rs
Outdated
| let mut done = false; | ||
| while !done { | ||
| let mut byte = (val as i8) & 0x7f; | ||
| val >>= 7; |
There was a problem hiding this comment.
So the leb128 crate has an existing writer that we could use here, and (at least the reader part of) that crate has had a good amount of optimization work done. Would be worth investigating at minimum.
There was a problem hiding this comment.
There was a problem hiding this comment.
I actually initially tried to use that, but I ran into one snag. When this implementation encodes sections it reserves 5 bytes for the section length, encodes the section, and then goes back and fills in the size of the section. That may not be the best idea in general, but the leb128 crate didn't have a way (I think?) to encode an integer to the maximum width, generating unnecessary trailer bytes.
Do you think we should abandon this strategy entirely and just slosh around buffers? (I haven't benchmarked anything yet). Or do you think we should add that functionality to leb128 crate?
There was a problem hiding this comment.
Yeah let's just work with this for now
There was a problem hiding this comment.
Ok sure thing, I've filed an issue at gimli-rs/leb128#6 to track it for now, and I'll put a FIXME in the code to stop hardcoding.
This commit moves emission of the wasm module away from the `parity-wasm` crate to instead using custom code within this crate. Similar to parsing with `wasmparser`, this is motivated twofold: * First, we want the ability to record binary offsets of where functions and instructions are located. This allows us encode dwarf debug information eventually. * Second, this avoids a "lowering to a different IR" problem where we will be able to implement more efficient emission than if we go to parity-wasm first. Ideally this would all be separated to an external crate and/or maybe even sharing `wasmparser` types or something like that, but for now it should be relatively easy enough to inline it and with the spec tests we can have a pretty high degree of confidence it's not full of bugs at least. Some other changes included here are: * Functions are now serialized in parallel * The handling of mapping a local id to an index is now done in a per-function fashion rather than through `IdsToIndices`. This way the maps can be built in parallel and then aggregated at the end into the one global map serially.
Turns out we forgot that `Used` was already collecting used locals, so there's no need to do it again in `emit_locals`!
alexcrichton
force-pushed
the
manually-serialize
branch
from
206d6f9
to
125d2c0
Compare
This commit moves emission of the wasm module away from the
parity-wasmcrate to instead using custom code within this crate.Similar to parsing with
wasmparser, this is motivated twofold:First, we want the ability to record binary offsets of where functions
and instructions are located. This allows us encode dwarf debug
information eventually.
Second, this avoids a "lowering to a different IR" problem where we
will be able to implement more efficient emission than if we go to
parity-wasm first.
Ideally this would all be separated to an external crate and/or maybe
even sharing
wasmparsertypes or something like that, but for now itshould be relatively easy enough to inline it and with the spec tests we
can have a pretty high degree of confidence it's not full of bugs at
least.
Some other changes included here are:
per-function fashion rather than through
IdsToIndices. This way themaps can be built in parallel and then aggregated at the end into the
one global map serially.