Stream: git-wasmtime

Topic: wasmtime / issue #4024 Cranelift: VCode: complete the tra...

Wasmtime GitHub notifications bot (Apr 13 2022 at 19:48):

cfallin opened issue #4024:

In #3989 with the switchover to regalloc2, VCode::emit is almost completely immutable (needs only &self), due to the way that we now keep regalloc2 results on-the-side and use the pre-regalloc code plus regalloc results on the fly, rather than editing in-place as before.

However, the ABI implementation is still slightly stateful during emission: gen_prologue takes a &mut self because it saves some computed info about the frame to use in gen_epilogue. Because of this, VCode::emit takes a self instead (consumes the VCode), and uses the ABI object mutably.

Fixing this last little bit of mutability should in principle only need us to split out that state that the ABI impl wants to save and pass it from gen_prologue to gen_epilogue manually. This is a bit tricky because of the trait-genericity though: we need a type projection to tell us what that type is, but we use a &dyn ABICallee (to avoid further type parameters and monomorphization everywhere) and so we can't just use a A::PrologueState from A: ABICallee because we don't have a parameter A.

We could do some further boxing and downcasting trickery to work around this, or (my preferred solution) finally do away with the distinction between ABICallee and ABICalleeImpl, because in practice it seems we were able to share implementations and didn't need the pluggability. Then we can just use a concrete type to pass the state, and as a bonus, we remove a few vtable invocations as well (though this has not appeared in any hotpaths in the past).

Wasmtime GitHub notifications bot (May 10 2022 at 11:35):

akirilov-arm labeled issue #4024:

In #3989 with the switchover to regalloc2, VCode::emit is almost completely immutable (needs only &self), due to the way that we now keep regalloc2 results on-the-side and use the pre-regalloc code plus regalloc results on the fly, rather than editing in-place as before.

However, the ABI implementation is still slightly stateful during emission: gen_prologue takes a &mut self because it saves some computed info about the frame to use in gen_epilogue. Because of this, VCode::emit takes a self instead (consumes the VCode), and uses the ABI object mutably.

Fixing this last little bit of mutability should in principle only need us to split out that state that the ABI impl wants to save and pass it from gen_prologue to gen_epilogue manually. This is a bit tricky because of the trait-genericity though: we need a type projection to tell us what that type is, but we use a &dyn ABICallee (to avoid further type parameters and monomorphization everywhere) and so we can't just use a A::PrologueState from A: ABICallee because we don't have a parameter A.

We could do some further boxing and downcasting trickery to work around this, or (my preferred solution) finally do away with the distinction between ABICallee and ABICalleeImpl, because in practice it seems we were able to share implementations and didn't need the pluggability. Then we can just use a concrete type to pass the state, and as a bonus, we remove a few vtable invocations as well (though this has not appeared in any hotpaths in the past).

Wasmtime GitHub notifications bot (May 10 2022 at 11:35):

akirilov-arm labeled issue #4024:

In #3989 with the switchover to regalloc2, VCode::emit is almost completely immutable (needs only &self), due to the way that we now keep regalloc2 results on-the-side and use the pre-regalloc code plus regalloc results on the fly, rather than editing in-place as before.

However, the ABI implementation is still slightly stateful during emission: gen_prologue takes a &mut self because it saves some computed info about the frame to use in gen_epilogue. Because of this, VCode::emit takes a self instead (consumes the VCode), and uses the ABI object mutably.

Fixing this last little bit of mutability should in principle only need us to split out that state that the ABI impl wants to save and pass it from gen_prologue to gen_epilogue manually. This is a bit tricky because of the trait-genericity though: we need a type projection to tell us what that type is, but we use a &dyn ABICallee (to avoid further type parameters and monomorphization everywhere) and so we can't just use a A::PrologueState from A: ABICallee because we don't have a parameter A.

We could do some further boxing and downcasting trickery to work around this, or (my preferred solution) finally do away with the distinction between ABICallee and ABICalleeImpl, because in practice it seems we were able to share implementations and didn't need the pluggability. Then we can just use a concrete type to pass the state, and as a bonus, we remove a few vtable invocations as well (though this has not appeared in any hotpaths in the past).

Last updated: Dec 23 2024 at 12:05 UTC