jameysharp opened PR #8506 from jameysharp:compress-range-lists
to bytecodealliance:main
:
These lists of ranges always cover contiguous ranges of an index space, meaning the start of one range is the same as the end of the previous range, so we can cut storage in half by only storing one endpoint of each range.
This in turn means we don't have to keep track of the other endpoint while building these lists, reducing the state we need to keep while building vcode and simplifying the various build steps.
jameysharp requested elliottt for a review on PR #8506.
jameysharp requested wasmtime-compiler-reviewers for a review on PR #8506.
jameysharp updated PR #8506.
fitzgen submitted PR review.
fitzgen submitted PR review.
fitzgen created PR review comment:
Why ignore?
fitzgen created PR review comment:
Can we add some typed indices for
Ranges
? Or reusecranelift_entity::Entity
?I've always been slightly nervous about all of our raw indices in the backends.
jameysharp submitted PR review.
jameysharp created PR review comment:
Because doc-tests don't work in non-public types, since they're compiled in a separate crate. I couldn't figure out a better way to document what these methods do than with code, so doc-test style seemed good, but didn't quite work how I wanted. I did build this with the module made public and the doc-tests un-ignored, and verified that they pass.
jameysharp submitted PR review.
jameysharp created PR review comment:
I guess we could add some sort of type-system tagging, but what we have now is
usize
almost everywhere this is used, so it would be noisy.That said, I had wanted to somehow associate each instance of
Ranges
with theVec
orRanges
that it indexes. I couldn't figure out a good way to do that, but putting a newtype wrapper around the indexes could work, with some helpers.I'd prefer to land this without that change, since I believe it's easier to understand and no less safe than the _status quo_, but I do think this is worth thinking about more.
elliottt submitted PR review:
This is a really nice change!
jameysharp merged PR #8506.
Last updated: Dec 23 2024 at 12:05 UTC