Stream: git-wasmtime

Topic: wasmtime / PR #10988 Cranelift: Rewrite conditional branc...

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:29):

fitzgen opened PR #10988 from fitzgen:conditional-branch-to-unconditional-trap to bytecodealliance:main:

Given this instruction:

brif v0, block1, block2

If we know that block1 does nothing but immediately trap then we can rewrite
that brif into the following:

trapz v0, <trapcode>
jump block2

(And we can do the equivalent with trapz if block2 immediately traps).

This transformation allows for the conditional trap instructions to be GVN'd and
for our egraphs mid-end to generally better optimize the program. We
additionally have better codegen in our backends for trapz than branches to
unconditional traps.

Fixes https://github.com/bytecodealliance/wasmtime/issues/10941

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:29):

fitzgen requested abrown for a review on PR #10988.

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:29):

fitzgen requested wasmtime-compiler-reviewers for a review on PR #10988.

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:29):

fitzgen requested alexcrichton for a review on PR #10988.

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:29):

fitzgen requested wasmtime-core-reviewers for a review on PR #10988.

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:46):

alexcrichton commented on PR #10988:

Could you detail more why this is part of legalization and not a separate function/pass? It seems a bit odd to me to have this be intertwined with legalization as it's basically not realized to legalization at all and is more of an optimization pass. Was it a compile-speed-related concern?

Also, do you know why so many tests are changing? Is it also due to the nature of the first commit where it's switching to a backwards walk from a forwards walk which renumbers legalization-produced things which GVN might pick differently between now?

We additionally have better codegen in our backends for trapz than branches to unconditional traps.

FWIW with cold blocks I don't think this is true, I believe if the trapping block is cold the codegen should basically be the same. That being said the GVN point carries this PR alone IMO in terms of motivation.

Wasmtime GitHub notifications bot (Jun 09 2025 at 17:55):

fitzgen commented on PR #10988:

Could you detail more why this is part of legalization and not a separate function/pass? It seems a bit odd to me to have this be intertwined with legalization as it's basically not realized to legalization at all and is more of an optimization pass. Was it a compile-speed-related concern?

Yes, I did not want to introduce another IR traversal, and would consider that a non-starter for solving this issue. We have in general been trying to remove passes (eg we've talked a lot about how to fuse phi-removal into the egraph pass) rather than add them.

You can think of legalization as a type of canonicalization, and this PR is canonicalizing conditional-branch-to-unconditional-trap into conditional-trap, if that helps.

Also, do you know why so many tests are changing? Is it also due to the nature of the first commit where it's switching to a backwards walk from a forwards walk which renumbers legalization-produced things which GVN might pick differently between now?

Yes, GVN is a forwards pass will choose the first value it sees as the canonical version. This used to be the legalized value with the lowest number because legalization was also a forwards pass so values at the start of the function would be legalized before those at the end. After the change to legalize in reverse, the legalized values at the end of the function are lower and at the start of the function are higher. So even if the IR is the "same" except for value numbers, the value numbers do change in the golden output.

FWIW with cold blocks I don't think this is true, I believe if the trapping block is cold the codegen should basically be the same.

We don't mark any Wasm blocks cold during translation today, so we won't actually get to that scenario. But maybe we should start making translation smarter and marking blocks cold based on heuristics at some point.

Wasmtime GitHub notifications bot (Jun 09 2025 at 18:51):

alexcrichton submitted PR review:

Sounds reasonable to me :+1:

Wasmtime GitHub notifications bot (Jun 09 2025 at 20:37):

fitzgen merged PR #10988.

Last updated: Dec 06 2025 at 06:05 UTC