Wasmtime GitHub notifications bot (Jun 22 2025 at 17:11): Wasmtime GitHub notifications bot (Jun 22 2025 at 17:11):PhantomInTheWire requested alexcrichton for a review on PR #11098.
Wasmtime GitHub notifications bot (Jun 22 2025 at 17:11):PhantomInTheWire requested wasmtime-compiler-reviewers for a review on PR #11098.
Wasmtime GitHub notifications bot (Jun 22 2025 at 17:11):PhantomInTheWire opened PR #11098 from PhantomInTheWire:feat/use-dominator-tree-preorder to bytecodealliance:main:
Motivation
This PR is to audit and reduce usage ofDominatorTreeand itsdominates()method, as tracked in https://github.com/bytecodealliance/wasmtime/issues/7954.The goal is to replace these withDominatorTreePreorderwherever applicable.Context
Recent related work includes:
Wasmtime GitHub notifications bot (Jun 23 2025 at 15:58):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #11098.
Wasmtime GitHub notifications bot (Jun 23 2025 at 15:58):alexcrichton requested cfallin for a review on PR #11098.
Wasmtime GitHub notifications bot (Jun 23 2025 at 15:58):alexcrichton commented on PR #11098:
I'm going to reroll this on the compiler-reviewer rotation as I'm not familiar with this data structure, but I suspect others are!
Wasmtime GitHub notifications bot (Jun 23 2025 at 18:10):cfallin created PR review comment:
I'm surprised you needed to add this case -- it should be implied by the below (
pre_numberandpost_numberwill both be equal pairwise to the other nodes' values ifa == b). Is this a performance optimization or...?
Wasmtime GitHub notifications bot (Jun 23 2025 at 18:10):cfallin created PR review comment:
Could we have a doc-comment on this field, like the others? In particular let's note what's different ("dominator tree with dominance stored implicitly via visit-order indices").
Wasmtime GitHub notifications bot (Jun 23 2025 at 18:10):cfallin submitted PR review:
Thanks for this! A number of thoughts below, but overall looks fine.
Wasmtime GitHub notifications bot (Jun 23 2025 at 18:10):cfallin created PR review comment:
Was this needed to address a failure otherwise? I'd prefer keeping the simple definition -- the original should also return false (I think?) if either is unreachable and the other is not, since the RHS of a
<=will be zero.
Wasmtime GitHub notifications bot (Jun 23 2025 at 18:10):cfallin created PR review comment:
Let's use
expect()here with a string indicating why this is infallible (we're traversing code that is in a block).
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:48):PhantomInTheWire updated PR #11098.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:48):PhantomInTheWire submitted PR review.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:48):PhantomInTheWire created PR review comment:
done.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:49):PhantomInTheWire created PR review comment:
no it was actually to deal with some debug statements that the previous pr added, but since I removed them this is no longer necessary
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:49):PhantomInTheWire submitted PR review.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:49):PhantomInTheWire submitted PR review.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:49):PhantomInTheWire created PR review comment:
sorry i missed it, removed.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:51):PhantomInTheWire submitted PR review.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:51):PhantomInTheWire created PR review comment:
my bad, i forgot to replace it. we should avoid unwrap() as much as we can.
Wasmtime GitHub notifications bot (Jun 24 2025 at 08:52):PhantomInTheWire updated PR #11098.
Wasmtime GitHub notifications bot (Jun 24 2025 at 15:20):cfallin submitted PR review:
Looks great -- thanks!
Wasmtime GitHub notifications bot (Jun 24 2025 at 15:42):cfallin merged PR #11098.
Last updated: Dec 06 2025 at 07:03 UTC