bpo-42615: Delete redundant jump instructions that only bypass empty blocks #23733
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: @OmG-117 For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Just to be clear, I'll need to merge in other recent changes to |
|
Great. |
Using the is_jump function results in the inclusion of instructions like returns for which this optimization is not really valid. So, instead explicitly check that the instruction is an unconditional jump.
OxyMagnesium
force-pushed
the
fix-redundant-jumps
branch
from
701bcb3 to
2f4e5d3
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @markshannon: please review the changes made to this pull request. |
|
That's great. Thanks for the contribution. |
…blocks (pythonGH-23733) * Delete jump instructions that bypass empty blocks * Add news entry * Explicitly check for unconditional jump opcodes Using the is_jump function results in the inclusion of instructions like returns for which this optimization is not really valid. So, instead explicitly check that the instruction is an unconditional jump. * Handle conditional jumps, delete jumps gracefully * Ensure b_nofallthrough and b_reachable are valid * Add test for redundant jumps * Regenerate importlib.h and edit Misc/ACKS * Fix bad whitespace
Currently, the deletion of unreachable blocks results in jump instructions that used to jump over said blocks, but now simply point to the next line (the next non-empty block in the CFG). These jump instructions do nothing and should be deleted.
This can be achieved by checking if the jump instruction at the end of any given block points to the next non-empty block reached through normal flow control (i.e. by following the
b_nextpointers until a non-empty block is found). If it does, then that means it is redundant, and can be deleted by decrementingb_usedfor that block.Confirmed that the extra jump in the proof of concept code is gone after this change. Casual non-scientific performance testing of the proof of concept code showed about a 3% performance increase averaged over 5 runs.
Also ran the test module, and all tests that could be run passed. Some random scripts I had lying around also worked flawlessly.
Proof of concept: https://pastebin.com/eRe7WThj
Disassembler output: https://imgur.com/a/jIAtoGt
https://bugs.python.org/issue42615