Stream: git-wasmtime

Topic: wasmtime / PR #11596 refactor(cranelift): merge Dominator...

Wasmtime GitHub notifications bot (Sep 03 2025 at 15:51):

pnodet opened PR #11596 from pnodet:pnodet-6 to bytecodealliance:main:

This PR aims at merging DominatorTreePreorder functionality directly into DominatorTree, eliminating duplicate computation and providing O(1) block dominance checks by default.

Previously, Cranelift had two separate dominator tree interfaces:

- DominatorTree: Basic dominance with O(depth) block dominance checks
- DominatorTreePreorder: Fast O(1) block dominance via preorder numbering

Changes

- Added preorder fields to DominatorTreeNode (child, sibling, dom_pre_number, dom_pre_max)
- Integrated preorder computation into main compute() method
- Replaced O(n) block_dominates with O(1) implementation using preorder numbers
- Added children() iterator for dominator tree traversal
- Removed DominatorTreePreorder struct entirely

Closes https://github.com/bytecodealliance/wasmtime/issues/7954

Wasmtime GitHub notifications bot (Sep 03 2025 at 15:51):

pnodet requested alexcrichton for a review on PR #11596.

Wasmtime GitHub notifications bot (Sep 03 2025 at 15:51):

pnodet requested wasmtime-compiler-reviewers for a review on PR #11596.

Wasmtime GitHub notifications bot (Sep 03 2025 at 17:21):

alexcrichton requested cfallin for a review on PR #11596.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:27):

cfallin submitted PR review:

Thanks very much for doing this cleanup!

A few comments below but nothing major...

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:27):

cfallin created PR review comment:

Why clone the postorder vec here? It looks like it's borrowed only while mutating other fields (nodes, dfs_worklist) below so the borrow checker should be able to understand the lack of borrow conflict...

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:27):

cfallin created PR review comment:

To avoid polluting the diff, could you keep this at the same point in the file? It looks like it's otherwise unmodified (modulo the type renaming).

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:27):

cfallin created PR review comment:

No need to use the inst_block method here -- pred.block is available (as shown a few lines below).

Likewise above around line 205.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:27):

cfallin created PR review comment:

nit -- rustdoc format is usually a single summary line, then a blank /// line, then the details.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:38):

pnodet submitted PR review.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:38):

pnodet created PR review comment:

Indeed I may have been overly cautious there

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:42):

pnodet updated PR #11596.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:45):

pnodet submitted PR review.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:45):

pnodet created PR review comment:

Thanks, changes are now at https://github.com/bytecodealliance/wasmtime/pull/11596/files#diff-b490f64fce5d44b4375aa866ddbb675869cb30c3044e818e6533271f409f5bc5L543 and way more readable

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:50):

pnodet commented on PR #11596:

@cfallin Thanks for the review, made a few changes for the things you pointed out

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:54):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 03 2025 at 18:54):

cfallin has enabled auto merge for PR #11596.

Wasmtime GitHub notifications bot (Sep 03 2025 at 19:18):

cfallin merged PR #11596.

Last updated: Dec 06 2025 at 07:03 UTC