Stream: git-wasmtime

Topic: wasmtime / Issue #2328 WIP: initial emission of constants

Wasmtime GitHub notifications bot (Oct 28 2020 at 17:55):

abrown commented on Issue #2328:

At a high level: I think we can actually do things with one less level of indirection

If we want to retain the de-duplication of constants from the constant pool AND avoid comparing long byte sequences, we need to keep track of what Constant handles have been used during lowering. Even if I reserve the next range of MachLabels after the number of blocks for constants, we still need to keep track of a map of Constant to MachLabel so I can see if we've used a constant previously.

Additionally, some structure needs to map those MachLabels back to their constant data so they can actually be emitted once we have a MachBuffer available during VCode::emit. So we need another map of MachLabel to ConstantData so that we can then call MachBuffer::defer_constant after all instructions have been emitted. This map must be separate from the Constant to MachLabel map because during lowering we can generate new constants that will not have a Constant handle (since we can't modify the ConstantPool in self.f.dfg).

Now, where we put these two maps is debatable (they could go in the same struct, different structs, be attached to Lower, etc.) but I don't see how we get rid of either of them. What do you think?

Wasmtime GitHub notifications bot (Oct 29 2020 at 22:58):

cfallin commented on Issue #2328:

Hmm, yes, you're right; I hadn't been considering how deduplication would work. I agree that we need some sort of remapping in order to redirect duplicated constants to the original copy.

I do think that it would be good to find a way to make the deduplication optional; I want to keep the "only do the barebones fast code emission" path as true to that original spirit as possible. Given that many constants will be coming from the CLIF anyway, where they're deduplicated already, I would hope not to lose too much by omitting this (though we can certainly measure!). From your design here, it seems that we could do this with a hybrid data-structure of some sort; maybe an enum with VCodeConstantTable::List { constants: Vec<ConstantData> } and VCodeConstantTable::Deduped { constants: HashMap<ConstantData, VCodeConstant> } ?

Last updated: Nov 22 2024 at 17:03 UTC