Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PCC: merge/propagate facts through egraph opts. #7280

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Oct 18, 2023

This turns out to be quite simple: the fundamental operation during egraph-based optimization is to merge eclasses, which is an assertion that their value is equal. Since the values of either side of the merge are equal, a fact about one side is a fact about the other, and vice-versa.

Since we only support one fact at most per value, we can't take the union of all facts; instead, if one side is missing a fact, or if both sides have exactly the same fact, we keep that one; otherwise we go to a special "conflict" fact that can't support any check. This is edging closer to a fact-lattice, but I opted not to go there with a full meet-function merge yet, for simplicity. (It's a little complex because of the "minimum fact" we can know about a value based on its type -- if we're going to do something better, I think we should account for that too.)

In any case, that complexity mostly isn't needed, because the two cases that happen in reality are (i) opt rules rewrite to a new node, which will have no facts at all, so facts just propagate; or (ii) GVN merges two values in the input program into one, but if both are the same value, in practice the Wasm PCC frontend (for example) should be producing the same facts on both values, so the merge is trivial.

This turns out to be quite simple: the fundamental operation during
egraph-based optimization is to *merge* eclasses, which is an assertion
that their value is equal. Since the values of either side of the merge
are equal, a fact about one side is a fact about the other, and
vice-versa.

Since we only support *one* fact at most per value, we can't take the
union of all facts; instead, if one side is missing a fact, or if both
sides have exactly the same fact, we keep that one; otherwise we go to a
special "conflict" fact that can't support any check. This is edging
closer to a fact-lattice, but I opted not to go there with a full
meet-function merge yet, for simplicity. (It's a little complex because
of the "minimum fact" we can know about a value based on its type -- if
we're going to do something better, I think we should account for that
too.)

In any case, that complexity mostly isn't needed, because the two cases
that happen in reality are (i) opt rules rewrite to a new node, which
will have no facts at all, so facts just propagate; or (ii) GVN merges
two values in the input program into one, but if both are the same
value, in practice the Wasm PCC frontend (for example) should be
producing the same facts on both values, so the merge is trivial.
@cfallin cfallin requested a review from fitzgen October 18, 2023 04:23
@cfallin cfallin requested a review from a team as a code owner October 18, 2023 04:23
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Oct 18, 2023
@fitzgen fitzgen added this pull request to the merge queue Oct 18, 2023
Merged via the queue into bytecodealliance:main with commit 5c7ed43 Oct 18, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants