wasmtime / PR #11321 Cranelift: support dynamic contexts ... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11321 Cranelift: support dynamic contexts ...

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:02):

cfallin requested fitzgen for a review on PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:02):

cfallin opened PR #11321 from cfallin:cranelift-exception-dynamic-context to bytecodealliance:main:

In #11285, we realized that Wasm semantics require us to match on dynamic instances of exception tags, rather than static tag types. This fundamentally requires the unwinder to be able to resolve the current Wasm instance for each Wasm frame on the stack that has any handlers, and our frame format does not provide this today.

We discussed many options, some of which solve the more general problem (Wasm vmctx for any frame), but ultimately landed on a notion of "dynamic context for evaluating tags", specific to Cranelift's exception-catch metadata; and storing that context and carrying it through to a place that is named in the unwind metadata. The reasoning is fairly straightforward: we cannot afford a more general approach that stores vmctx in every frame (I measured this at 20% overhead for a recursive-Fibonacci benchmark that is call-intensive); and inlining means that we may have multiple contexts at any given program point, each associated with a different slice of the handler tags; so we need a mechanism that, just for a try-call, intersperses contexts with tags (or puts a context on each tag) and stores these somewhere that the exception-unwind ABI doesn't clobber (e.g., on the stack).

This PR implements "option 4" from that issue, namely, dynamic exception contexts. The idea is that this is the dual to exception payload: while payload lets the unwinder communicate state to the catching code, context lets the unwinder take state from the catching code that lets it decide whether the tag is a match. Because of inlining, we need to either associate (optional) context with every tag, or intersperse context-updates with handler tags. I've opted for the latter for efficiency at the CLIF level (in most cases there will be multiple tags per context), though they are isomorphic.

The new tag-matching semantics are: when walking up the stack, upon reaching a try_call, evaluate catch-clauses in listed order. A context clause sets the current context. A tagN: block(...) clause attempts to match the throwing exception against tagN, evaluated in the current context, and branches to the named block if it matches. A default: block(...) always branches to the named block.

Note that this lets us assume less about tags than before, and this particularly manifests in the changes to the inliner. Whereas before, tagN is tagN and an inner handler for that tag shadows an outer handler (that is, tags always alias if identical indices); and whereas before, tagN is not tagM and so we can order the tags arbitrarily (that is, tags never alias if non-identical indices); now any two static tag indices may or may not alias depending on the dynamic context of each. Or, even in the same context, two may alias, because we leave the match-predicate as an unspecified (user-chosen) algorithm during unwinding. (This mirrors the reality that, for example, a Wasm instance may import two tags, and dynamically these tags may be equal or different at runtime, even instantiation-to-instantiation.) Cranelift's only job is to faithfully carry the list of contexts and tags through to the compiled-code metadata; and to ensure that they remain in the order they were specified in the CLIF.

This PR introduces the Cranelift-level feature, and it will be used in a subsequent PR that introduces Wasm exception handling. Because of that, I've opted not to update the clif-utils runtest "runtime" to read out contexts and do something with them -- we will have plenty of test coverage via a bunch of Wasm tests for corner cases such as the above. This PR does include filetests that show that contexts are carried through to spillslots and those appear in the metadata.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:02):

cfallin requested wasmtime-compiler-reviewers for a review on PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:02):

cfallin requested wasmtime-core-reviewers for a review on PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:02):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:03):

cfallin edited PR #11321:

In #11285, we realized that Wasm semantics require us to match on dynamic instances of exception tags, rather than static tag types. This fundamentally requires the unwinder to be able to resolve the current Wasm instance for each Wasm frame on the stack that has any handlers, and our frame format does not provide this today.

We discussed many options, some of which solve the more general problem (Wasm vmctx for any frame), but ultimately landed on a notion of "dynamic context for evaluating tags", specific to Cranelift's exception-catch metadata; and storing that context and carrying it through to a place that is named in the unwind metadata. The reasoning is fairly straightforward: we cannot afford a more general approach that stores vmctx in every frame (I measured this at 20% overhead for a recursive-Fibonacci benchmark that is call-intensive); and inlining means that we may have multiple contexts at any given program point, each associated with a different slice of the handler tags; so we need a mechanism that, just for a try-call, intersperses contexts with tags (or puts a context on each tag) and stores these somewhere that the exception-unwind ABI doesn't clobber (e.g., on the stack).

This PR implements "option 4" from that issue, namely, dynamic exception contexts. The idea is that this is the dual to exception payload: while payload lets the unwinder communicate state to the catching code, context lets the unwinder take state from the catching code that lets it decide whether the tag is a match. Because of inlining, we need to either associate (optional) context with every tag, or intersperse context-updates with handler tags. I've opted for the latter for efficiency at the CLIF level (in most cases there will be multiple tags per context), though they are isomorphic.

The new tag-matching semantics are: when walking up the stack, upon reaching a try_call, evaluate catch-clauses in listed order. A context clause sets the current context. A tagN: block(...) clause attempts to match the throwing exception against tagN, evaluated in the current context, and branches to the named block if it matches. A default: block(...) always branches to the named block.

Note that this lets us assume less about tags than before, and this particularly manifests in the changes to the inliner. Whereas before, tagN is tagN and an inner handler for that tag shadows an outer handler (that is, tags always alias if identical indices); and whereas before, tagN is not tagM and so we can order the tags arbitrarily (that is, tags never alias if non-identical indices); now any two static tag indices may or may not alias depending on the dynamic context of each. Or, even in the same context, two may alias, because we leave the match-predicate as an unspecified (user-chosen) algorithm during unwinding. (This mirrors the reality that, for example, a Wasm instance may import two tags, and dynamically these tags may be equal or different at runtime, even instantiation-to-instantiation.) Cranelift's only job is to faithfully carry the list of contexts and tags through to the compiled-code metadata; and to ensure that they remain in the order they were specified in the CLIF.

This PR introduces the Cranelift-level feature, and it will be used in a subsequent PR that introduces Wasm exception handling. Because of that, I've opted not to update the clif-utils runtest "runtime" to read out contexts and do something with them -- we will have plenty of test coverage via a bunch of Wasm tests for corner cases such as the above. This PR does include filetests that show that contexts are carried through to spillslots and those appear in the metadata.

Fixes #11285.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:04):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:11):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:15):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:33):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:40):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:40):

bjorn3 created PR review comment:

Is this using the same numbering scheme as DWARF debuginfo? If so can this reuse the enum we use for variable location debuginfo? If not what numbering scheme is used?

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:45):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:45):

cfallin created PR review comment:

Right now it's a little ad-hoc -- it propagates through whatever numbering scheme we use in PReg, which on x86 is the ISA encoding's numbering (here) and on all others mirrors the GPR numbering directly (N is xN on aarch64, xN on riscv64, rN on s390x, xN on pulley). I was worried about putting an unwind dependency in a core type but perhaps I can pull the enum the other way -- good idea.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:46):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 21:46):

cfallin created PR review comment:

(err, here, rather)

Wasmtime GitHub notifications bot (Jul 24 2025 at 22:11):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 22:11):

cfallin created PR review comment:

Hmm, actually, this creates a bunch of new complexity: it requires pulling in the register mapper from the unwind implementation which ordinarily only exists on SysV platforms, conditionalizing this enum arm existence on the unwind feature, and some other mess as well. I only added this for genericity based on yesterday's Cranelift meeting (support context in regs/stack in general, assert stack only on the Wasmtime side), but I would rather not build out a bunch of functionality for register-based contexts if it's not actually used; and if it is eventually in Wasmtime, we will likely define our own register mappings. So I am going to go ahead and declare what we have now as Cranelift's "native" dynamic exception context register mapping, and we can refine this later as needed -- or if folks would prefer (@alexcrichton?) I can remove the GPR arm here and put the assert and stack-only constraint back into Cranelift.

Wasmtime GitHub notifications bot (Jul 24 2025 at 22:43):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 22:43):

alexcrichton created PR review comment:

Would it be possible to use a literal PReg here? That way embedders could remap if they like, but there's a canonical definition for each register (the various backend helpers). Otherwise I'd agree it's not worth too much infrastructure just yet and it's ok to assert with a todo that a register doesn't pop out

Wasmtime GitHub notifications bot (Jul 24 2025 at 23:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 23:01):

cfallin created PR review comment:

Perhaps; but we don't export regalloc2 types from the public API anywhere else, nor those backend helpers? The latter especially mean that the PReg isn't much more meaningful than the raw hw_enc -- that's the integer we are returning currently.

In fullness of time this seems like something an assembler layer should own -- canonical register names and ISA-defined numbering, then we can use this mapping in our public APIs -- but I guess no matter which type we wrap around that index today, the issue is that we don't have any public API surface that defines it.

Wasmtime GitHub notifications bot (Jul 24 2025 at 23:02):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 24 2025 at 23:02):

cfallin created PR review comment:

(Except for the unwind stuff, of course, but that has the above platform-dependence and feature-dependence issues)

Wasmtime GitHub notifications bot (Jul 25 2025 at 03:24):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 25 2025 at 03:24):

alexcrichton created PR review comment:

Ok makes sense, personally I'm ambivalent about leaving this as u8 or just removing the variant for now, could go either way.

Wasmtime GitHub notifications bot (Jul 25 2025 at 04:58):

cfallin created PR review comment:

Cool; maybe the deciding factor then is, as long as this PR has the change to rely on the ABI clobbers to force us to the stack case for the tail-call ABI (a good suggestion!), let's keep the register case, so we don't have to fall back to a compile panic or disallow contexts for try-calls in SysV ABI (which are already supported).

Wasmtime GitHub notifications bot (Jul 25 2025 at 04:58):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 25 2025 at 18:50):

fitzgen submitted PR review:

Looks great! A couple nitpicks below, r=me with them addressed

Wasmtime GitHub notifications bot (Jul 25 2025 at 18:50):

fitzgen created PR review comment:

This length invariant no longer holds because context entries do not have an associated target, right?

Wasmtime GitHub notifications bot (Jul 25 2025 at 18:50):

fitzgen created PR review comment:

This method would previously give only exceptional edges, but now gives all kinds of table items. That seems fine if callers were not relying on that property, but we should definitely update the method documentation and probably the method name as well.

Wasmtime GitHub notifications bot (Jul 25 2025 at 18:50):

fitzgen created PR review comment:

Double checking my understanding: we don't need to check basic properties of the context value here (like its definition dominates this use) because these checks are handled via the generic methods of getting all uses and defs of a particular instruction elsewhere in the verifier, right?

Wasmtime GitHub notifications bot (Jul 25 2025 at 18:50):

fitzgen created PR review comment:

    #[expect(
so that it turns into a warning (and CI failure) if we forget to remove this once it is actually used

Wasmtime GitHub notifications bot (Jul 25 2025 at 18:50):

fitzgen created PR review comment:

Maybe makes sense to define add_call_site such that it doesn't take any handlers, and then a new add_try_call_site that does take them? Would cut down on the empty iterator argument stuff to clean up all these call sites a little.

Actually, even better would be if we pushed the try call info into the caller, so that call sites just looked like
sink.add_call_site(info.try_call_info())
and then add_call_site would internally do the if let Some(...).

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin created PR review comment:

Went with the former option -- interpreting the TryCallInfo to get handlers requires a FrameLayout as well, and pushing that into the MachBuffer felt like too much coupling (vs. taking an iterator over MachExceptionHandlers, which are a buffer-defined concept).

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin created PR review comment:

Yep, exactly -- the context values are included in the instruction's values iterator (DataFlowGraph::inst_values()) now, so are verified to exist and be valid SSA uses that way.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:02):

cfallin created PR review comment:

Updated, thanks!

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:03):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:03):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:03):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:05):

cfallin has enabled auto merge for PR #11321.

Wasmtime GitHub notifications bot (Jul 26 2025 at 01:16):

cfallin updated PR #11321.

Wasmtime GitHub notifications bot (Jul 26 2025 at 02:00):

cfallin merged PR #11321.

Last updated: Dec 06 2025 at 07:03 UTC