wasmtime icon indicating copy to clipboard operation
wasmtime copied to clipboard

Improve usability of constants

Open abrown opened this issue 4 years ago • 2 comments
trafficstars

This adds a higher-level method, LowerCtx::emit_constant for lowering a constant in one of two ways:

  • using a DataValue (instead of the previous u64), a value can be generated into a register using MachInst::gen_constant
  • alternately, if MachInst::should_genenerate_constant_in_pool is true, then a value is emitted into the constant pool/island and loaded from memory at runtime; this path uses MachInst::gen_constant_in_pool.

abrown avatar Dec 02 '20 22:12 abrown

@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 with ctx.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 by LowerCtx::get_immediate) and the LowerInput::constant field--I can do that as a part of this PR but wanted to get feedback on the current changes first

abrown avatar Dec 02 '20 22:12 abrown

(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 avatar Dec 02 '20 23:12 abrown

@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! :-)

cfallin avatar Feb 09 '23 00:02 cfallin

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!

abrown avatar Feb 10 '23 18:02 abrown