Wasmtime GitHub notifications bot (Nov 02 2024 at 03:36): Wasmtime GitHub notifications bot (Nov 02 2024 at 03:36):alexcrichton requested abrown for a review on PR #9543.
Wasmtime GitHub notifications bot (Nov 02 2024 at 03:37):alexcrichton opened PR #9543 from alexcrichton:push-memory-type-down-in-compilers to bytecodealliance:main:
This commit has more refactoring to simplify handling of linear memories in Wasmtime. The end goal is to remove the static/dynamic distinction and simplify the internal implementation and external interface. This is along the same lines of #9528, #9532, #9531, and #9530. The goal of this PR is a series of standalone commits which work towards futher "pushing down" the usage of
MemoryStyledown to the consumers of bounds check code. The goal here is to remove layers of abstraction and instead unify everything around the same abstraction ofTunablesandwasmtime_environ::Memory. Many fields in Winch/CraneliftHeapDatahave been removed in favor of storingMemorydirectly.This PR affects generated golden tests because previously the global value for the heap bound was not generated if the heap was a "static" heap. With the eventual removal of static/dynamic distinction it's easier to just always generate this and have it not get used most of the time. Thus while the clif output tests are changing none of the actual generated code itself should be changing.
Wasmtime GitHub notifications bot (Nov 02 2024 at 03:37):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9543.
Wasmtime GitHub notifications bot (Nov 02 2024 at 03:37):alexcrichton requested pchickey for a review on PR #9543.
Wasmtime GitHub notifications bot (Nov 02 2024 at 03:37):alexcrichton requested wasmtime-core-reviewers for a review on PR #9543.
Wasmtime GitHub notifications bot (Nov 04 2024 at 17:12):abrown submitted PR review.
Wasmtime GitHub notifications bot (Nov 04 2024 at 17:12):abrown created PR review comment:
It's this extra global that is causing the changes in the disassembly tests, right? E.g.,
gv5?
Wasmtime GitHub notifications bot (Nov 04 2024 at 17:14):abrown submitted PR review.
Wasmtime GitHub notifications bot (Nov 04 2024 at 17:14):abrown created PR review comment:
It would appear that
gv4is not being used in those tests, so we're adding an unused global here and I'm debating the pros and cons of that.
Wasmtime GitHub notifications bot (Nov 04 2024 at 18:15):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Nov 04 2024 at 18:15):alexcrichton created PR review comment:
Right yeah this is the global being added. I wrote up a bit above about this but it's essentially easier given the current code shape to assume that the global value is always there rather than calculate if it'll be needed in a function body and create it and/or lazily creating it on-demand. My hope is that the "overhead" here is just a few bytes in a vector per-function which in theory doesn't have too much impact, but I'll admit I haven't measured.
Wasmtime GitHub notifications bot (Nov 04 2024 at 23:03):alexcrichton requested fitzgen for a review on PR #9543.
Wasmtime GitHub notifications bot (Nov 05 2024 at 01:01):abrown submitted PR review:
Well, beyond my question about the additional global, which I'm not terribly opposed to anyways, I think this makes sense.
Wasmtime GitHub notifications bot (Nov 05 2024 at 15:26):alexcrichton updated PR #9543.
Wasmtime GitHub notifications bot (Nov 05 2024 at 15:26):alexcrichton commented on PR #9543:
@fitzgen mind giving a once-over confirmation here as well?
Wasmtime GitHub notifications bot (Nov 05 2024 at 20:35):fitzgen submitted PR review:
LGTM!
Wasmtime GitHub notifications bot (Nov 05 2024 at 21:05):fitzgen merged PR #9543.
Last updated: Nov 22 2024 at 16:03 UTC