Resolve value aliases in disas tests#8239
Merged
jameysharp merged 1 commit intobytecodealliance:mainfrom Mar 26, 2024
Merged
Conversation
jameysharp
requested review from
alexcrichton and
elliottt
and removed request for
a team
github-actions
bot
added
the
cranelift
This reverts commit 6c51848, "Cranelift: resolve value aliases when printing CLIF functions (bytecodealliance#8214)", then applies the same effect a different way. In discussion on bytecodealliance#8223, we decided to handle this a different way. So I've introduced a method on DataFlowGraph which eliminates all value aliases, and we can call it when necessary. If that function is not called then the CLIF printer should print value aliases the same way it did before bytecodealliance#8214. In this PR, I've specifically called it in disas tests. The changes to write.rs and frontend.rs are from the revert, while the changes to disas.rs are new. In these tests, value aliases are just clutter that distracts from understanding what code will actually be generated. They may change due to changes in legalization, but that doesn't signal anything about whatever the test was intended to check. Because the new helper deletes aliases after it's done resolving them, those aliases now disappear from the test expectations. Aside from that, the test expectations are unchanged compared to what bytecodealliance#8214 did.
jameysharp
force-pushed
the
resolve-disas-aliases
branch
from
231dae2 to
0160919
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit 6c51848, "Cranelift: resolve value aliases when printing CLIF functions (#8214)", then applies the same effect a different way.
In discussion on #8223, we decided to handle this a different way. So I've introduced a method on DataFlowGraph which eliminates all value aliases, and we can call it when necessary. If that function is not called then the CLIF printer should print value aliases the same way it did before #8214.
In this PR, I've specifically called it in disas tests. The changes to write.rs and frontend.rs are from the revert, while the changes to disas.rs are new.
In these tests, value aliases are just clutter that distracts from understanding what code will actually be generated. They may change due to changes in legalization, but that doesn't signal anything about whatever the test was intended to check.
Because the new helper deletes aliases after it's done resolving them, those aliases now disappear from the test expectations. Aside from that, the test expectations are unchanged compared to what #8214 did.