Wasmtime GitHub notifications bot (Jan 31 2022 at 15:38): Wasmtime GitHub notifications bot (Jan 31 2022 at 15:38):uweigand commented on issue #3724:
Can this be merged now or are you waiting on any action from my side? I have other patches I'd like to submit and this is causing merge conflicts ...
As to the question of moving
emit_safepointto the common prelude, my preference would still be for other architecture maintainers to implement this for their platform as fits them best, and once it is done for all, we can still merge any definitions that end up the same back into the common prelude ...
Wasmtime GitHub notifications bot (Jan 31 2022 at 18:05):fitzgen commented on issue #3724:
Can this be merged now or are you waiting on any action from my side?
I think just the nitpicks that we didn't already discuss and agree could wait for follow ups:
- moving
emit_safepointinto the prelude, and- moving its external rust implementation into
wasmtime/cranelift/codegen/src/machinst/isle.rs.
Wasmtime GitHub notifications bot (Jan 31 2022 at 18:42):uweigand commented on issue #3724:
Can this be merged now or are you waiting on any action from my side?
I think just the nitpicks that we didn't already discuss and agree could wait for follow ups:
* moving `emit_safepoint` into the prelude, and * moving its external rust implementation into `wasmtime/cranelift/codegen/src/machinst/isle.rs`.But that was just what we discussed: As I understand, x64 will need a _different_
emit_safepointimplementation due the move mitosis stuff, so it's not clear this can be shared.
Wasmtime GitHub notifications bot (Jan 31 2022 at 19:26):fitzgen commented on issue #3724:
Ah right. Then just the
declandexternbits need to move toprelude.isleand the implementations can be pre-backend. We can stub out the other backends withunimplemented!()for now.
Wasmtime GitHub notifications bot (Jan 31 2022 at 20:48):uweigand commented on issue #3724:
Ah right. Then just the
declandexternbits need to move toprelude.isleand the implementations can be pre-backend. We can stub out the other backends withunimplemented!()for now.I'm happy to do that, but I'd really prefer this to be a separate PR. That will still touch all backends and their generated files, and I'd prefer not to entangle this with this back-end change, which is already quite large as-is ...
Wasmtime GitHub notifications bot (Jan 31 2022 at 21:59):uweigand commented on issue #3724:
Okay, we can do that in follow ups then.
Thanks! The follow up is now here: https://github.com/bytecodealliance/wasmtime/pull/3745
Last updated: Nov 22 2024 at 16:03 UTC