Stream: git-wasmtime

Topic: wasmtime / Issue #2538 Multi-register value support: fram...


view this post on Zulip Wasmtime GitHub notifications bot (Jan 04 2021 at 06:31):

cfallin commented on Issue #2538:

CI error seems unrelated (perhaps caused by recent Rust 1.49 release?):

 note: rust-lld: error while loading shared libraries: libLLVM-11-rust-1.49.0-stable.so: cannot open shared object file: No such file or directory

view this post on Zulip Wasmtime GitHub notifications bot (Jan 06 2021 at 01:43):

cfallin commented on Issue #2538:

Thanks for the detailed review!

One top-level point about special-casing the one-reg case vs. generalizing with ValueRegs everywhere: I did a quick measurement of compilation instruction count before/after this PR (clif-util wasm --target x86_64 bz2.wasm) as measured by perf stat:

Before:
     1,960,194,985      instructions              #    1.99  insn per cycle
     1,956,941,087      instructions              #    1.98  insn per cycle
     1,944,661,945      instructions              #    1.95  insn per cycle

After:
     1,951,673,951      instructions              #    1.98  insn per cycle
     1,951,074,339      instructions              #    1.98  insn per cycle
     1,945,951,877      instructions              #    1.98  insn per cycle

In other words, it seems to be in the measurement noise. I sort-of expected this given that (on the 64-bit platform case) we turned a 32-bit Reg into a 64-bit (Reg, Reg) (basically) with a bunch of nonzero checks of the top half and almost-never-taken branches.

Given that tiny/no overhead, I would strongly prefer to not special-case the one-register case, as it adds significant complexity; IMHO we're starting to get a slight case of "too much nested-if disease" in some places and I'd really prefer to keep just one code path where we can :-)

Anyway, lots of updates here so PTAL again if you like!

view this post on Zulip Wasmtime GitHub notifications bot (Jan 06 2021 at 17:58):

cfallin commented on Issue #2538:

Just spoke with @bnjbvr and will go ahead and merge this with existing approval (thanks!).


Last updated: Jan 10 2026 at 02:36 UTC