Skip to content

egraph: Resolve all aliases at once#8240

Merged
jameysharp merged 2 commits intobytecodealliance:mainfrom
jameysharp:resolve-egraph-aliases
Mar 28, 2024
Merged

egraph: Resolve all aliases at once#8240
jameysharp merged 2 commits intobytecodealliance:mainfrom
jameysharp:resolve-egraph-aliases

Conversation

@jameysharp
Copy link
Contributor

This way we can use the linear-time alias rewriting pass, and then avoid having to think about value aliases ever again.

@jameysharp jameysharp requested a review from a team as a code owner March 26, 2024 07:14
@jameysharp jameysharp requested review from abrown and removed request for a team March 26, 2024 07:14
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Mar 26, 2024
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@cfallin cfallin added this pull request to the merge queue Mar 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 26, 2024
This way we can use the linear-time alias rewriting pass, and then avoid
having to think about value aliases ever again.
When resolving aliases in values_labels, this discards debug info on
values which are replaced by aliases. However, that is equivalent to the
existing behavior in `Lower::get_value_labels`, which resolves value
aliases first and only then looks for attached debug info.
@jameysharp jameysharp force-pushed the resolve-egraph-aliases branch from 82ab57d to 70b0827 Compare March 28, 2024 21:16
@jameysharp
Copy link
Contributor Author

@cfallin, could you have a look at my additional changes in 70b0827 and make sure they make sense?

CI informed me that values deleted due to this alias-rewrite pass were still referenced. While auditing everything in the DataFlowGraph for references to Values, I realized I think it makes sense to merge PCC facts onto the targets of value aliases. So that may affect your work on PCC, hopefully in good ways.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New changes look good!

@jameysharp jameysharp added this pull request to the merge queue Mar 28, 2024
Merged via the queue into bytecodealliance:main with commit b338f92 Mar 28, 2024
@jameysharp jameysharp deleted the resolve-egraph-aliases branch March 28, 2024 22:41
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