Skip to content

GH-135379: Remove types from stack items in code generator. #135384

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 11, 2025
Merged

GH-135379: Remove types from stack items in code generator. #135384

merged 4 commits into from
Jun 11, 2025

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Jun 11, 2025 •
edited
Loading

This removes the types from stack items in the code generator.
E.g

op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame: _PyInterpreterFrame *)) {

becomes

op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame) {

and the cast from _PyStackRef to _PyInterpreterFrame * is made explicit in the op definition.

This is a net zero change in terms of code size, but removes fiddly code generator code and replaces it with simple casts.
This also means that any future changes to the code generator, specifically for TOS caching, do not have to worry about inserting casts.

@bedevere-app bedevere-app bot mentioned this pull request Jun 11, 2025
Open
@Fidget-Spinner
Copy link
Member

Looks good. However, can you add a news entry please? Downstream forks use the DSL now (like Cinder), so I think this should have a news entry. It's also big enough that it warrants it.

@markshannon
Copy link
Member Author

I know Cinder uses it, but it isn't part of any public API and we don't want it to be.
I guess a change note is fine, as long as it doesn't go in "what's new".

@Fidget-Spinner
Copy link
Member

Yeah I don't think it should be documented or in What's New either.

@markshannon markshannon merged commit c87b5b2 into python:main Jun 11, 2025
74 checks passed
@markshannon markshannon deleted the remove-stackitem-types branch June 11, 2025 14:52
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x RHEL9 3.x (tier-3) has failed when building commit c87b5b2.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1565/builds/2457) and take a look at the build logs.
  4. Check if the failure is related to this commit (c87b5b2) or if it is a false positive.
  5. 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/1565/builds/2457

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-rhel9-s390x/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-rhel9-s390x/build/Lib/test/_test_multiprocessing.py", line 570, in _kill_process
    self.assertEqual(join(), None)
                     ~~~~^^
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/build/Lib/test/_test_multiprocessing.py", line 250, in __call__
    return self.func(*args, **kwds)
           ~~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/build/Lib/multiprocessing/process.py", line 156, in join
    res = self._popen.wait(timeout)
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/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-rhel9-s390x/build/Lib/multiprocessing/popen_fork.py", line 28, in poll
    pid, sts = os.waitpid(self.pid, flag)
               ~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/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-1' pid=3964973 parent=3964971 started daemon>


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/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-rhel9-s390x/build/Lib/test/_test_multiprocessing.py", line 570, in _kill_process
    self.assertEqual(join(), None)
                     ~~~~^^
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/build/Lib/test/_test_multiprocessing.py", line 250, in __call__
    return self.func(*args, **kwds)
           ~~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/build/Lib/multiprocessing/process.py", line 156, in join
    res = self._popen.wait(timeout)
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/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-rhel9-s390x/build/Lib/multiprocessing/popen_fork.py", line 28, in poll
    pid, sts = os.waitpid(self.pid, flag)
               ~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-rhel9-s390x/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-158' pid=3956945 parent=3954168 started daemon>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@markshannon @Fidget-Spinner @bedevere-bot