Wasmtime GitHub notifications bot (Jan 13 2025 at 11:18): Wasmtime GitHub notifications bot (Jan 13 2025 at 11:18):JonasKruckenberg opened PR #9992 from JonasKruckenberg:jonas/static-machine-env to bytecodealliance:main:
This PR is another attempt at #8489 (superseding it) this time by making the necessary changes in
regalloc2(see here) to allowMachineEnvto be created in const expressions and therefore placed into statics without the need for lazy initialization (as mentioned in this comment)
This PR also makes the necessary changes in register creation functions to make themconst.While I agree with @cfallin that this is the "correct" move, it is slightly annoying that Rusts const limitation makes the static initializers so verbose where a
1..=16range iterator was used previously :/
Wasmtime GitHub notifications bot (Jan 13 2025 at 11:18):JonasKruckenberg requested fitzgen for a review on PR #9992.
Wasmtime GitHub notifications bot (Jan 13 2025 at 11:18):JonasKruckenberg requested wasmtime-compiler-reviewers for a review on PR #9992.
Wasmtime GitHub notifications bot (Jan 13 2025 at 11:18):JonasKruckenberg requested wasmtime-default-reviewers for a review on PR #9992.
Wasmtime GitHub notifications bot (Jan 13 2025 at 11:19):JonasKruckenberg edited PR #9992:
This PR is another attempt at #8489 (superseding it) this time by making the necessary changes in
regalloc2(see here) to allowMachineEnvto be created in const expressions and therefore placed into statics without the need for lazy initialization (as mentioned in this comment)
This PR also makes the necessary changes in register creation functions to make themconst.While I agree with @cfallin that this is the "correct" move, it is slightly annoying that Rusts const limitation makes the static initializers so verbose where a
1..=16range iterator was used previously, any ideas here are welcome :/Edit: marked the PR as draft until the regalloc2 upstream PR is merged
Wasmtime GitHub notifications bot (Jan 13 2025 at 11:20):JonasKruckenberg updated PR #9992.
Wasmtime GitHub notifications bot (Jan 13 2025 at 19:54):cfallin requested cfallin for a review on PR #9992.
Wasmtime GitHub notifications bot (Jan 13 2025 at 20:22):cfallin created PR review comment:
This probably goes in the
PINNED_MACHINE_ENVabove, no? (It's load-bearing there that we get it right, because we excluded the pinned register from allocation; the default environment is used when pinning is disabled so doesn't care what the pinned reg is)
Wasmtime GitHub notifications bot (Jan 13 2025 at 20:22):cfallin created PR review comment:
Can you say more here -- I'm not that familiar with the current state of constification of everything; is there some feature this is waiting on to stabilize or...?
Wasmtime GitHub notifications bot (Jan 13 2025 at 20:22):cfallin submitted PR review:
Thanks a bunch for working on this! This all looks like the shape I expected; I'm glad there were no other hidden snags. A few minor comments below but otherwise happy to see this merged once we get the RA2 PR merged and released.
Wasmtime GitHub notifications bot (Jan 13 2025 at 22:22):JonasKruckenberg submitted PR review.
Wasmtime GitHub notifications bot (Jan 13 2025 at 22:22):JonasKruckenberg created PR review comment:
well, the feature that would be waiting for is const associated functions in traits (therefore allowing
From::fromto be const) but that afaik is in "never type land". I can remove the note as it doesn't make much sense to hold our breath on it
Wasmtime GitHub notifications bot (Jan 13 2025 at 22:24):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jan 13 2025 at 22:24):cfallin created PR review comment:
Yeah, I think that's best -- usually when we have a FIXME we want it to be actionable, or at least give sufficient context so that we know what we're waiting on (i.e.: if someone else walked up to this code tomorrow, would they know what they need in order to fix it?). It's fine to evaluate questions like this once new Rust features land in the future.
Wasmtime GitHub notifications bot (Jan 13 2025 at 23:05):JonasKruckenberg submitted PR review.
Wasmtime GitHub notifications bot (Jan 13 2025 at 23:05):JonasKruckenberg created PR review comment:
the issue with constness is that theres no good way to detect constness workarounds after the fact (the whole reason for the const-hack comments) but usually you'd tie them to specific RFCs yeah
Wasmtime GitHub notifications bot (Jan 13 2025 at 23:07):JonasKruckenberg edited PR review comment.
Wasmtime GitHub notifications bot (Jan 13 2025 at 23:07):JonasKruckenberg edited PR review comment.
Wasmtime GitHub notifications bot (Jan 14 2025 at 00:18):hanna-kruppe submitted PR review.
Wasmtime GitHub notifications bot (Jan 14 2025 at 00:18):hanna-kruppe created PR review comment:
Relevant RFC fresh off the press: https://github.com/rust-lang/rfcs/pull/3762
Last updated: Jan 24 2025 at 00:11 UTC