Stream: git-wasmtime

Topic: wasmtime / Issue #2258 machinst x64: check SSE requiremen...

Wasmtime GitHub notifications bot (Oct 05 2020 at 14:54):

bnjbvr commented on Issue #2258:

would it make sense to hold the EmitInfo in the VCode container itself, and plumb it into the instruction emission from there?

I'm happy to do this, but that would break the symmetry with EmitState... which could also live in Vcode in fact. Maybe also moving EmitState would help reducing the traits complexity... Any preference here? (Happy to chat about this on Zulip)

Wasmtime GitHub notifications bot (Oct 05 2020 at 17:19):

cfallin commented on Issue #2258:

I suppose I see the difference between EmitState and EmitInfo as: the vcode should be emittable multiple times (the instructions are immutable during emit), so we want EmitState to be an external thing -- it's transient, per emission; while EmitInfo encapsulates some information that is tied to the lowering and should not be changed, so its lifetime should be as long as the VCode. (And given that it shouldn't change, there is really only one "correct value" to use when emitting a given VCode body, so it doesn't seem right to require it as a parameter.) So from that perspective, the asymmetry doesn't bother me too much. That said, I'm happy to go with your original design as well if you prefer it :-)

Wasmtime GitHub notifications bot (Oct 05 2020 at 17:19):

cfallin edited a comment on Issue #2258:

I suppose I see the difference between EmitState and EmitInfo as: the VCode should be emittable multiple times (the instructions are immutable during emit), so we want EmitState to be an external thing -- it's transient, per emission; while EmitInfo encapsulates some information that is tied to the lowering and should not be changed, so its lifetime should be as long as the VCode. (And given that it shouldn't change, there is really only one "correct value" to use when emitting a given VCode body, so it doesn't seem right to require it as a parameter.) So from that perspective, the asymmetry doesn't bother me too much. That said, I'm happy to go with your original design as well if you prefer it :-)

Wasmtime GitHub notifications bot (Oct 06 2020 at 10:07):

bnjbvr commented on Issue #2258:

That's a good argument, thanks! I hadn't thought about the fact that we could emit several times (I wonder what such a use case would be though, since an emitter who'd like to emit in several places would emit in a buffer, and then would carry a cheap copy rather than the full-blown emission.) I'll rework my patch.

Wasmtime GitHub notifications bot (Oct 07 2020 at 09:43):

bnjbvr commented on Issue #2258:

@cfallin Updated the patch, please let me know if you want to take another look!

Last updated: Nov 22 2024 at 16:03 UTC