Wasmtime GitHub notifications bot (Feb 20 2025 at 23:00): Wasmtime GitHub notifications bot (Feb 20 2025 at 23:00):frank-emrich requested alexcrichton for a review on PR #10265.
Wasmtime GitHub notifications bot (Feb 20 2025 at 23:00):frank-emrich requested wasmtime-core-reviewers for a review on PR #10265.
Wasmtime GitHub notifications bot (Feb 20 2025 at 23:00):frank-emrich opened PR #10265 from frank-emrich:vmcontext-layout to bytecodealliance:main:
This PR is part of a series that adds support for the Wasm stack switching proposal. The explainer document for the proposal is here. There's a tracking issue for the overall progress: #10248
This particular PR does one small change: It adds a single pointer-sized field, called
stack_chainto the fixed-width part ofVMContext.
Note that this PR only adds the field in the layout of theVMContext, but does not actually read or write any data in that field. The reason for doing this change in a separate PR is simply that the change to the layout of theVMContextrequires updates to 700 disas tests.In subsequent PRs, the field is used as follows: In the future, the
StoreOpaquewill contain astack_chainfield as well, which indicates the currently running continuation, and logically represents a linked list continuations.The
stack_chainfield in theVMContextadded in this PR will merely be a pointer to that stack chain in theStoreOpaque. This is due to the fact that the stack chain is shared by all instances running inside a store, and we need access to the stack chain in theStoreOpaquefrom Cranelift-generated code. This is similar to how theVMRuntimeLimitsare accessed.
Wasmtime GitHub notifications bot (Feb 20 2025 at 23:14):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Feb 20 2025 at 23:14):alexcrichton has enabled auto merge for PR #10265.
Wasmtime GitHub notifications bot (Feb 20 2025 at 23:36):alexcrichton merged PR #10265.
Wasmtime GitHub notifications bot (Feb 27 2025 at 19:23):fitzgen commented on PR #10265:
@frank-emrich
The
stack_chainfield in theVMContextadded in this PR will merely be a pointer to that stack chain in theStoreOpaque. This is due to the fact that the stack chain is shared by all instances running inside a store, and we need access to the stack chain in theStoreOpaquefrom Cranelift-generated code. This is similar to how theVMRuntimeLimitsare accessed.Would it make sense to put the stack chain in the
VMRuntimeLimits(which I have been meaning to rename to something likeVMStoreContextfor forever) rather than inStoreOpaque? That is generally the location where we put all shared-across-the-whole-store-and-accessed-by-JIT-code data (as opposed to instance-specific data that is accessed by JIT code, which goes inVMContext). There is no precedent, afaik, of JIT code directly accessing anything inStoreOpaquethus far, and I'd sort of like to keep it that way so that we have that clean separation between what is vs isn't accessed by JIT code and becauseStoreOpaqueis notrepr(C).(Apologies for all these late comments, I've been at the CG and then on vacation and I'm just catching up now)
Wasmtime GitHub notifications bot (Feb 27 2025 at 19:29):frank-emrich commented on PR #10265:
@fitzgen
I don't have strong feelings about this, but from what you describe it sounds like a good idea to move the stack chain there. It would also mean that the new field added in this PR can be removed again (its only purpose is to achieve being able to access that particular field in theStoreOpaquefrom jitted code. If we move the stack chain into theVMRuntimeLimits, we can just rely on the fact that we already got a pointer from theVMContextto theVMRuntimeLimits).
Last updated: Feb 28 2025 at 03:10 UTC