-
-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
gh-106581: Fix two bugs in the code generator's copy optimization #108380
Conversation
I was comparing the last preceding poke with the *last* peek, rather than the *first* peek. Unfortunately this bug obscured another bug, which I have yet to fix: When the last preceding poke is UNUSED, the first peek disappears, leaving the variable unassigned.
Rename CopyEffect to CopyItem. Change CopyItem to contain StackItems instead of StackEffects. Update those StackItems when adjusting the manager higher or lower. Assert that those StackItems' offsets are equivalent.
I apologize, when using this to split |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor tweaks.
(I had different code in one of the branches during debugging. :-) Co-authored-by: Irit Katriel <[email protected]>
When replacing matched pairs of push/pop (poke/peek), I was matching up the last poke in the previous "manager" with the wrong peek in the new one, causing some copying opportunities to be missed.
Fixing this revealed a more serious bug where we could end up skipping the copy of an 'unused' poke to a used peek, leaving the variable uninitialized. The fix for this required me to keep track of the original StackItems in the CopyEffect object, and use that to reconstruct the source when copying from 'unused'.
The net result so far is that this found two redundant initializations.