Stream: git-wasmtime

Topic: wasmtime / issue #3989 Switch Cranelift over to regalloc2.


view this post on Zulip Wasmtime GitHub notifications bot (Apr 04 2022 at 21:03):

cfallin commented on issue #3989:

@fitzgen I've split this into somewhat more reasonably-sized chunks; I think this should be more manageable, but let me know if you want me to try to factor the core changes more finely still.

I'll switch this out of 'draft' mode once bytecodealliance/regalloc2#38 is reviewed and merged and I can switch the dep back to a crate version here.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2022 at 19:50):

cfallin commented on issue #3989:

@fitzgen I've split the work into more-or-less separate chunks, and cleaned up the last CI nits, so I think this is ready for review now!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 07 2022 at 19:15):

bjorn3 commented on issue #3989:

I just tried this with cg_clif. On the rustc benchmark suite it improves compile time performance by a couple percent. The biggest improvement is on the deep-vector benchmark which has a huge function. There it improves perf by up to 7%. On the primary benchmarks the biggest improvement is cargo at 1.7%.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2022 at 18:49):

fitzgen commented on issue #3989:

Ideally VCode::emit would take a &self. Right now it consumes the VCode (takes self) only because the ABICallee saves some state when generating the prologue (it only computes clobbers and hence frame size at that point) and I didn't want to play tricks with cells, or clone it or whatnot, to make this work.

Can you file a follow up issue for this?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 19:49):

cfallin commented on issue #3989:

Ideally VCode::emit would take a &self. Right now it consumes the VCode (takes self) only because the ABICallee saves some state when generating the prologue (it only computes clobbers and hence frame size at that point) and I didn't want to play tricks with cells, or clone it or whatnot, to make this work.

Can you file a follow up issue for this?

Yep, filed #4024.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2022 at 01:06):

cfallin commented on issue #3989:

I believe I've resolved all pending review comments now, with a few followup issues filed as well. Thanks @fitzgen for the speedy and helpful review!

I'll hold off on merging this until my morning (PDT) so I'm around just out of caution, but given all of the fuzzing and testing I am cautiously optimistic this should be uneventful...


Last updated: Nov 22 2024 at 17:03 UTC