gh-106581: Split `CALL_PY_EXACT_ARGS` into uops#107760

gvanrossum

This is only the first step for doing CALL in Tier 2. The next step involves tracing into the called code object. After that we'll have to do the remaining CALL specialization. Finally we'll have to tweak various things like KW_NAMES, and possibly move the NULL (for method calls) above the callable (that's 107788). But those are things for future PRs.

Note: this moves setting frame->return_offset directly in front of DISPATCH_INLINED(), to make it easier to move it into _PUSH_FRAME.

Rebase on GH-105848: Simplify the arrangement of CALL's stack #107788
Redo using Mark's ideas
Add back a call to _Py_EnterRecursivePy
Add tests

Issue: Call design for Tier 2 (uops) interpreter #106581

brandtbucher

Thanks for tackling this, it definitely doesn't look easy. It's sort of a bummer that we need to special-case this much stuff, but I also don't see a nicer way of handling these issues than what you have here.

A few comments and questions, mostly for my own understanding:

gvanrossum

I'll add that assert; then I'll review your PR, and hopefully you can then merge that, and I can handle the merge fallout.

gvanrossum

Made this back into a draft; I need to (a) wait for Brandt's gh-107788, then (b) redo the split and tooling changes using Mark's ideas.

brandtbucher

The CALL PR has been merged.

This is only the first step for doing `CALL` in Tier 2. The next step involves tracing into the called code object. After that we'll have to do the remaining `CALL` specialization. Finally we'll have to tweak various things like `KW_NAMES`, and possibly move the `NULL` (for method calls) *above* the callable. But those are things for future PRs. Note: this moves setting `frame->return_offset` directly in front of `DISPATCH_INLINED()`, to make it easier to move it into `_PUSH_FRAME`.

ambv

Closing and re-opening to retrigger CLA checks. Sorry for the noise.

Instead, the special case is an opcode using SAVE_FRAME_STATE(). Introducing #if TIER_ONE and #if TIER_TWO so we can implement _PUSH_FRAME differently for both tiers.

Instead, we special-case SAVE_IP: - Its Tier 2 expansion sets oparg to the instruction offset - In Tier 1 it is a no-op (and skipped if present in a macro)

gvanrossum

@markshannon I was hoping you'd review this. I added _Py_EnterRecursivePy which was the last thing on my TODO list.

Unless you'd rather review #107925, which includes this (and #107793, which is the intermediate stage).

markshannon

I'm uneasy about the introduction of the TIER_ONE and TIER_TWO macros.
It is a principle of the overall design that there is a single source of truth for the semantics of bytecodes.

It might appear that I'm being dogmatic, but the need for something like those macros often indicates an underlying problem that should be fixed independently.

In this case the problem is the cframe. Loading and saving the IP needs to handled specially anyway and saving and loading the SP should be the same for both interpreters (but will need to be handled specially by the copy-and-patch compiler, so should be its own micro-op).
It is pushes the frame that differs. Removing cframe will fix that.

The cframe only exists as a performance hack to minimize the impact of tracing prior to PEP 669.

gvanrossum

Benchmark: 1.00x faster: https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20230816-3.13.0a0-05af848

IOW it doesn't slow CALL_PY_EXACT_ARGS down, which is all I care about.

bedevere-bot

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

Hi! The buildbot wasm32-emscripten node (pthreads) 3.x has failed when building commit dc8fdf5.

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/all/#builders/1050/builds/2796) and take a look at the build logs.
Check if the failure is related to this commit (dc8fdf5) 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/all/#builders/1050/builds/2796

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

329 tests OK.

10 slowest tests:

test_math: 2 min 9 sec
test_hashlib: 1 min 59 sec
test_tarfile: 1 min 9 sec
test_unparse: 49.6 sec
test_io: 41.3 sec
test_tokenize: 40.0 sec
test_unicodedata: 28.3 sec
test_capi: 27.8 sec
test_fstring: 24.5 sec
test_pickle: 23.0 sec

1 test altered the execution environment:
test_capi

117 tests skipped:
test.test_asyncio.test_base_events
test.test_asyncio.test_buffered_proto
test.test_asyncio.test_context
test.test_asyncio.test_eager_task_factory
test.test_asyncio.test_events test.test_asyncio.test_futures
test.test_asyncio.test_futures2 test.test_asyncio.test_locks
test.test_asyncio.test_pep492
test.test_asyncio.test_proactor_events
test.test_asyncio.test_protocols test.test_asyncio.test_queues
test.test_asyncio.test_runners
test.test_asyncio.test_selector_events
test.test_asyncio.test_sendfile test.test_asyncio.test_server
test.test_asyncio.test_sock_lowlevel test.test_asyncio.test_ssl
test.test_asyncio.test_sslproto test.test_asyncio.test_streams
test.test_asyncio.test_subprocess
test.test_asyncio.test_taskgroups test.test_asyncio.test_tasks
test.test_asyncio.test_threads test.test_asyncio.test_timeouts
test.test_asyncio.test_transports
test.test_asyncio.test_unix_events test.test_asyncio.test_waitfor
test.test_asyncio.test_windows_events
test.test_asyncio.test_windows_utils test__xxinterpchannels
test__xxsubinterpreters test_asyncgen test_clinic test_cmd_line
test_concurrent_futures test_contextlib_async test_ctypes
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_doctest
test_docxmlrpc test_dtrace test_embed test_epoll test_faulthandler
test_fcntl test_file_eintr test_fork1 test_ftplib test_gdb
test_generated_cases test_grp test_httplib test_httpservers
test_idle test_imaplib test_interpreters test_ioctl test_kqueue
test_launcher test_lzma test_mmap test_multiprocessing_fork
test_multiprocessing_forkserver test_multiprocessing_main_handling
test_multiprocessing_spawn test_openpty test_pdb
test_perf_profiler test_perfmaps test_poll test_poplib test_pty
test_pwd test_readline test_regrtest test_repl test_resource
test_select test_selectors test_smtplib test_smtpnet test_socket
test_socketserver test_ssl test_stable_abi_ctypes test_startfile
test_subprocess test_sys_settrace test_syslog test_tcl
test_tkinter test_tools test_ttk test_ttk_textonly test_turtle
test_urllib2 test_urllib2_localnet test_urllib2net test_urllibnet
test_venv test_wait3 test_wait4 test_webbrowser test_winconsoleio
test_winreg test_winsound test_wmi test_wsgiref test_xmlrpc
test_xxlimited test_zipfile64 test_zipimport_support test_zoneinfo

Total duration: 26 min 4 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/Lib/test/test_capi/test_watchers.py", line 532, in watcher
    raise MyError("testing 123")

This finishes the work begun in gh-107760. When, while projecting a superblock, we encounter a call to a short, simple function, the superblock will now enter the function using `_PUSH_FRAME`, continue through it, and leave it using `_POP_FRAME`, and then continue through the original code. Multiple frame pushes and pops are even possible. It is also possible to stop appending to the superblock in the middle of a called function, when running out of space or encountering an unsupported bytecode.

gvanrossum added the skip news label Aug 8, 2023

gvanrossum requested review from brandtbucher and markshannon August 8, 2023 04:48

bedevere-bot mentioned this pull request Aug 8, 2023

Call design for Tier 2 (uops) interpreter #106581

Closed

bedevere-bot added the awaiting core review label Aug 8, 2023

Eclips4 added the interpreter-core label Aug 8, 2023

brandtbucher approved these changes Aug 8, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Aug 8, 2023

gvanrossum commented Aug 8, 2023

View reviewed changes

brandtbucher approved these changes Aug 8, 2023

View reviewed changes

gvanrossum mentioned this pull request Aug 9, 2023

gh-106581: Start projecting through calls #107793

Closed

2 tasks

markshannon reviewed Aug 9, 2023

View reviewed changes

brandtbucher reviewed Aug 9, 2023

View reviewed changes

gvanrossum marked this pull request as draft August 9, 2023 17:41

bedevere-bot removed the awaiting merge label Aug 9, 2023

gvanrossum added 2 commits August 9, 2023 11:41

Fix merge so it works again (I think)

907ff95

gvanrossum force-pushed the call-uops branch from 9784cc2 to 907ff95 Compare August 9, 2023 19:01

Split into finer-grained uops

2c6be6d

gvanrossum force-pushed the call-uops branch from 91ebd3f to 2c6be6d Compare August 9, 2023 23:03

gvanrossum added 6 commits August 9, 2023 17:24

Fix type error in stacking.py

6d78ff2

Add test

0d8e66c

Add comment explaining _PUSH_FRAME's unused output effect

b75f30e

Make PUSH_FRAME special case a little less myterious

61c2822

Rename Instruction.write to write_case_body

f73ea90

Move next_instr update to a more logical place

12910fc

gvanrossum added 5 commits August 10, 2023 16:19

Don't recompute macro cache offset

2fafa2c

Fold and refactor long line in stacking.py

2717b07

Fold long lines in generate_cases.py

e487908

Don't emit static assert to executor cases

1d549af

Factor away write_case_body (formerly Instruction.write)

f40fb1f

ambv closed this Aug 11, 2023

ambv reopened this Aug 11, 2023

gvanrossum added 3 commits August 11, 2023 14:59

Fold long lines

4f6f8f8

Make less of a special case of _PUSH_FRAME …

6facc8d

Instead, the special case is an opcode using SAVE_FRAME_STATE(). Introducing #if TIER_ONE and #if TIER_TWO so we can implement _PUSH_FRAME differently for both tiers.

Stop special-casing _PUSH_FRAME altogether …

94630d4

Instead, we special-case SAVE_IP: - Its Tier 2 expansion sets oparg to the instruction offset - In Tier 1 it is a no-op (and skipped if present in a macro)

gvanrossum marked this pull request as ready for review August 13, 2023 03:31

bedevere-bot added the awaiting core review label Aug 13, 2023

gvanrossum mentioned this pull request Aug 14, 2023

gh-107557: Setup abstract interpretation #107847

Merged

2 tasks

gvanrossum added 2 commits August 15, 2023 13:20

Call _Py_EnterRecursivePy in _FRAME_PUSH

cf8e2c0

Merge remote-tracking branch 'upstream/main' into call-uops

1e62876

gvanrossum mentioned this pull request Aug 15, 2023

IGNORE: Call uops forever #107952

Closed

markshannon reviewed Aug 16, 2023

View reviewed changes

gvanrossum mentioned this pull request Aug 16, 2023

GH-108035: Remove the _PyCFrame struct as it is no longer needed for performance. #108036

Merged

Introduce SAVE_CURRENT_IP uop per Mark's proposal

05af848

gvanrossum merged commit dc8fdf5 into python:main Aug 16, 2023

bedevere-bot removed the awaiting core review label Aug 16, 2023

gvanrossum deleted the call-uops branch August 16, 2023 23:31

gvanrossum mentioned this pull request Aug 16, 2023

gh-106581: Project through calls #108067

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-106581: Split `CALL_PY_EXACT_ARGS` into uops#107760

gh-106581: Split `CALL_PY_EXACT_ARGS` into uops#107760
gvanrossum merged 20 commits into
python:mainfrom
gvanrossum:call-uops

gvanrossum commented Aug 8, 2023 •

edited

Loading

Uh oh!

brandtbucher left a comment

Uh oh!

gvanrossum left a comment

Uh oh!

gvanrossum commented Aug 9, 2023

Uh oh!

brandtbucher commented Aug 9, 2023

Uh oh!

ambv commented Aug 11, 2023

Uh oh!

gvanrossum commented Aug 16, 2023

Uh oh!

markshannon left a comment

Uh oh!

gvanrossum commented Aug 16, 2023

Uh oh!

bedevere-bot commented Aug 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

gvanrossum commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandtbucher left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Aug 9, 2023

Uh oh!

brandtbucher commented Aug 9, 2023

Uh oh!

ambv commented Aug 11, 2023

Uh oh!

gvanrossum commented Aug 16, 2023

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Aug 16, 2023

Uh oh!

bedevere-bot commented Aug 17, 2023

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

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gvanrossum commented Aug 8, 2023 •

edited

Loading