wasmtime
wasmtime copied to clipboard
Improve usability of constants
This adds a higher-level method, LowerCtx::emit_constant for lowering a constant in one of two ways:
- using a
DataValue(instead of the previousu64), a value can be generated into a register usingMachInst::gen_constant - alternately, if
MachInst::should_genenerate_constant_in_poolis true, then a value is emitted into the constant pool/island and loaded from memory at runtime; this path usesMachInst::gen_constant_in_pool.
@cfallin, @bnjbvr, I think this almost wraps up what I had proposed in #1385. Several caveats:
- I chose to not replace all of the
ctx.emit(PXOR...)uses withctx.emit_constant([0; 16]...)because I was concerned there might be an extra allocation or extra logic that might slightly slow down compile times; if you don't think this is the case and want a consistent way of generating constants, I can do the replacement here - I have not dug into removing
LowerCtx::get_constant(its uses can all be replaced byLowerCtx::get_immediate) and theLowerInput::constantfield--I can do that as a part of this PR but wanted to get feedback on the current changes first
(And apparently CI will fail until I figure out the LowerCtx::get_constant and LowerInput::constant issues: do we want to go down the route to remove those?)
@abrown I'm cleaning up old PRs and came upon this; do you think there are still improvements here that are useful to salvage? Constant-handling has changed quite a bit in the meantime (and much of it has been purged from my cache too) so it's hard for me to judge exactly which of the issues here are still relevant, but there may be some! :-)
I'm cleaning up old PRs and came upon this; do you think there are still improvements here that are useful to salvage? Constant-handling has changed quite a bit in the meantime (and much of it has been purged from my cache too) so it's hard for me to judge exactly which of the issues here are still relevant, but there may be some! :-)
As I remember it, the API could have been nicer; this was an attempt to do that. I think at this point, though, one would have to re-evaluate the new state of the API and try an almost-new refactoring. Let's close this!