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
cfallin commented on Issue #2538:
Thanks for the detailed review!
One top-level point about special-casing the one-reg case vs. generalizing with
ValueRegseverywhere: I did a quick measurement of compilation instruction count before/after this PR (clif-util wasm --target x86_64 bz2.wasm) as measured byperf 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 cycleIn 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
Reginto 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!
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