-
-
Notifications
You must be signed in to change notification settings - Fork 810
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
fix[venom]: fix MakeSSA
with existing phis
#4423
fix[venom]: fix MakeSSA
with existing phis
#4423
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4423 +/- ##
===========================================
- Coverage 91.76% 51.79% -39.97%
===========================================
Files 119 119
Lines 16615 16615
Branches 2796 2796
===========================================
- Hits 15247 8606 -6641
- Misses 936 7364 +6428
- Partials 432 645 +213 ☔ View full report in Codecov by Sentry. |
i added more tests (1728bee runs venom passes in the round-trip tests) and there still seems to be some bugs, e.g. https://github.com/vyperlang/vyper/actions/runs/12483208026/job/34838622968?pr=4423 throws things like at first glance, it looks like the fix fails if there are multiple phis in multiple basic blocks referencing the same variable name, but not sure |
MakeSSA
with existing phis
run without --experimental-codegen
What I did
Fixed the memssa pass to support already existing
phi
instructions.How I did it
How to verify it
Commit message
Description for the changelog
Cute Animal Picture