GeezerVison is showing github.com — gh-131798: Small improvements to `remove_unneeded_uops` by tomasr8 · Pull Request #134554 · python/cpython
Conversation
tomasr8
commented
May 22, 2025
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
- Add
_CHECK_PERIODIC to the list of skipped instructions. I believe this is safe to do since this instruction does not modify the stack. It is also relatively common so I think it's worth it to skip it.
- Add an extra guard to the optimizer.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
if (op_without_push[last->opcode]) {
|
|
|
if (op_without_push[last->opcode] && op_without_pop[opcode]) {
|
|
|
last->opcode = op_without_push[last->opcode];
|
|
|
opcode = buffer[pc].opcode = op_without_pop[opcode];
|
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.
When we are here, we know that op_without_pop[opcode] || op_without_pop_null[opcode] is true. But we should also make sure that op_without_pop[opcode] itself is true, otherwise it is possible to accidentally write 0 as the opcode.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Add
_CHECK_PERIODIC to the list of skipped instructions. I believe this is safe to do since this instruction does not modify the stack. It is also relatively common so I think it's worth it to skip it.
I read the code but just checking that I understand it right: op_skip means the instruction is still emitted, just that we "skip" it when trying to optimize away redundant push/pops right? If so that's ok the PR LGTM.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Add
_CHECK_PERIODIC to the list of skipped instructions. I believe this is safe to do since this instruction does not modify the stack. It is also relatively common so I think it's worth it to skip it.
I read the code but just checking that I understand it right: op_skip means the instruction is still emitted, just that we "skip" it when trying to optimize away redundant push/pops right? If so that's ok the PR LGTM.
Yes, that's exactly the case! This allows us to optimize for instance:
_LOAD_CONST
_CHECK_PERIODIC
_POP_TOP
into just
_NOP
_CHECK_PERIODIC
_NOP
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot AMD64 Fedora Stable Refleaks 3.x (tier-1) has failed when building commit 71dea74.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/320/builds/2497) and take a look at the build logs.
- Check if the failure is related to this commit (71dea74) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/#/builders/320/builds/2497
Failed tests:
- test.test_multiprocessing_fork.test_processes
Failed subtests:
- test_interrupt - test.test_multiprocessing_fork.test_processes.WithProcessesTestProcess.test_interrupt
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/process.py", line 320, in _bootstrap
self.run()
~~~~~~~~^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/process.py", line 108, in run
self._target(*self._args, **self._kwargs)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 518, in _sleep_some_event
time.sleep(100)
~~~~~~~~~~^^^^^
KeyboardInterrupt
k
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 588, in test_interrupt
exitcode = self._kill_process(multiprocessing.Process.interrupt)
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 570, in _kill_process
self.assertEqual(join(), None)
~~~~^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 250, in __call__
return self.func(*args, **kwds)
~~~~~~~~~^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/process.py", line 156, in join
res = self._popen.wait(timeout)
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/popen_fork.py", line 44, in wait
return self.poll(os.WNOHANG if timeout == 0.0 else 0)
~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/popen_fork.py", line 28, in poll
pid, sts = os.waitpid(self.pid, flag)
~~~~~~~~~~^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 566, in handler
raise RuntimeError('join took too long: %s' % p)
RuntimeError: join took too long: <Process name='Process-3' pid=1325402 parent=1325398 started daemon>
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/process.py", line 320, in _bootstrap
self.run()
~~~~~~~~^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/process.py", line 108, in run
self._target(*self._args, **self._kwargs)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 517, in _sleep_some_event
event.set()
~~~~~~~~~^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/synchronize.py", line 344, in set
with self._cond:
^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/synchronize.py", line 242, in __exit__
return self._lock.__exit__(*args)
~~~~~~~~~~~~~~~~~~~^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/synchronize.py", line 100, in __exit__
return self._semlock.__exit__(*args)
~~~~~~~~~~~~~~~~~~~~~~^^^^^^^
KeyboardInterrupt
k
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 588, in test_interrupt
exitcode = self._kill_process(multiprocessing.Process.interrupt)
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 570, in _kill_process
self.assertEqual(join(), None)
~~~~^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 250, in __call__
return self.func(*args, **kwds)
~~~~~~~~~^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/process.py", line 156, in join
res = self._popen.wait(timeout)
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/popen_fork.py", line 44, in wait
return self.poll(os.WNOHANG if timeout == 0.0 else 0)
~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/multiprocessing/popen_fork.py", line 28, in poll
pid, sts = os.waitpid(self.pid, flag)
~~~~~~~~~~^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/_test_multiprocessing.py", line 566, in handler
raise RuntimeError('join took too long: %s' % p)
RuntimeError: join took too long: <Process name='Process-1706' pid=1247987 parent=1230274 started daemon>
|
brandtbucher reacted with thumbs down emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|