GH-105848: Replace `KW_NAMES` + `CALL` with `LOAD_CONST` + `CALL_KW` by brandtbucher · Pull Request #109300 · python/cpython · GitHub

Get rid of the magic kwnames side-channel and split out calls with a tuple of keyword names on the stack (CALL_KW) from those with no keyword arguments (the existing CALL family). Stats on an earlier version of this branch suggested that specializing CALL_KW just isn't worth it.

Other cleanup:

There was some incorrect code in mark_stacks that was missed in GH-105848: Simplify the arrangement of CALL's stack #107788 (some docs, too).
Any CALL specialization with the name CALL_NO_KW_* has the NO_KW part dropped, since that should be obvious with the new split.

The new CALL_KW instruction is mostly copied and lightly modified from the existing CALL instruction. They can probably share code using uops, but that's a project for another day.

Issue: Clean up the CALL sequence #105848

📚 Documentation preview 📚: https://cpython-previews--109300.org.readthedocs.build/

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 53917a5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 53917a5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

1% faster.

Can you add CALL_KW to the list of specializable instructions that we don't specialize for the stats. https://github.com/python/cpython/blob/main/Python/specialize.c#L135

I just love seeing this! Mostly some docs nits.

Is the crash fix inseparable from the new opcode? Is there no issue for it?

It's just bugs in mark_stacks left over from #107788. So same issue number, unrelated to the new opcode though.

Break long line

Suggested change

      
               or :opcode:`CALL_KW` when calling the unbound method. Otherwise, ``NULL`` and the object returned by
          
               or :opcode:`CALL_KW` when calling the unbound method.
          
               Otherwise, ``NULL`` and the object returned by

Suggested change

      
               * ``self`` (or ``NULL``)
          
               * ``self`` or ``NULL``

(Same thing about whether "not" should be removed.)

Make explicit that if there are N positional and M keyword arguments, oparg is N+M, and len(tuple) must be == M.

This is already a merge conflict. :-) Might need to manually merge #109269.

Maybe add (here too) that oparg counts arg1 etc., but not self? (It has to be this way, of course, but still bears repeating IMO.)

I think it's clearer to just say "excluding self":

Suggested change

      
               ``argc`` is the total of the positional arguments, excluding
          
               ``self`` when a ``NULL`` is not present.
          
               ``argc`` is the total of the positional arguments, excluding ``self``.

Suggested change

      
               * ``self`` (or ``NULL``)
          
               * ``self`` or ``NULL``

Suggested change

      
               ``argc`` is the total of the positional and named arguments, excluding
          
               ``self`` when a ``NULL`` is not present.
          
               ``argc`` is the total of the positional and named arguments, excluding ``self``.

Maybe this deserves a comment (like CALL) about the stack contents on entry and exit? Or if not, maybe CALL doesn't need it either? (Because it should be clear from the DSL.)

Yeah, I'd rather just remove it from both. It's not that complex anymore.

Ignoring the buildbot failures since they're all either failing on main or on other PRs also. Not a very informative run... :(

Can you add CALL_KW to the list of specializable instructions that we don't specialize for the stats. https://github.com/python/cpython/blob/main/Python/specialize.c#L135

That's only useful if we're recording failure kinds, right? We're not in this case, so I don't think it would actually add anything other than an empty section.

That is definitely easier to follow. :)

brandtbucher added 14 commits August 11, 2023 16:53

Split things up (sorta)

5e43544

Fix all specialization

b7820ee

Catch up with main

0ad7112

Catch up with main

72d22a4

Quick hack to fix stats

9c8ccf6

Catch up with main

4c247af

Don't specialize CALL_KW

367e7db

More cleanup

b0e33b6

Catch up with main

6e6a3b4

Catch up with main

135a499

CALL_NO_KW -> CALL

6cd0318

blurb add

53917a5

Feedback from code review

21b7fef

Catch up with main

79f93cd

brandtbucher added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Sep 12, 2023

brandtbucher requested a review from gvanrossum September 12, 2023 01:01

brandtbucher self-assigned this Sep 12, 2023

brandtbucher requested review from brettcannon, ericsnowcurrently, iritkatriel, markshannon, ncoghlan and warsaw as code owners September 12, 2023 01:01

bedevere-app Bot added the awaiting review label Sep 12, 2023

bedevere-app Bot mentioned this pull request Sep 12, 2023

Clean up the CALL sequence #105848

Closed

brandtbucher added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels Sep 12, 2023

bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Sep 12, 2023

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 12, 2023

brettcannon removed their request for review September 12, 2023 17:25

gvanrossum reviewed Sep 12, 2023

View reviewed changes

ericsnowcurrently reviewed Sep 13, 2023

View reviewed changes

brandtbucher merged commit 22e65ee into python:main Sep 13, 2023

bedevere-app Bot removed the awaiting review label Sep 13, 2023

markshannon mentioned this pull request Oct 24, 2023

Remove eval_breaker and kwnames local variables from interpreter definition. #104580

Closed

coderabbitai Bot mentioned this pull request Apr 3, 2026

#49 Enhance bytecode decompiler with slices and tuples sahebbiswas/pycrefine#50

Merged

coderabbitai Bot mentioned this pull request Aug 1, 2026

Proof: вынести общую BUILD-границу Labpics-Team/lab-colors#508

Closed

                  * The callable
-                 * The positional arguments
-                 * The named arguments
+                 * ``self`` (or ``NULL``)

                       // or
-                      //   [method, self, arg1, arg2, ...]
-                      // (Some args may be keywords, see KW_NAMES, which sets 'kwnames'.)
+                      //   [callable, self, arg1, arg2, ...]

-                 :opcode:`KW_NAMES`, if any.
-                 On the stack are (in ascending order), either:
+                 Calls a callable object with the number of arguments specified by ``argc``.
+                 On the stack are (in ascending order):
-                 * NULL
                  * The callable
-                 * The positional arguments
-                 * The named arguments
-                 or:
-                 * The callable
-                 * ``self``
+                 * ``self`` or ``NULL``
                  * The remaining positional arguments
-                 * The named arguments
-                 ``argc`` is the total of the positional and named arguments, excluding
-                 ``self`` when a ``NULL`` is not present.
+                 ``argc`` is the total of the positional arguments, excluding ``self``.

		arguments. Also, fix a possible crash when jumping over method calls in a
		debugger.

	or :opcode:`CALL_KW` when calling the unbound method. Otherwise, ``NULL`` and the object returned by
	or :opcode:`CALL_KW` when calling the unbound method.
	Otherwise, ``NULL`` and the object returned by

		``argc`` is the total of the positional and named arguments, excluding
		``self`` when a ``NULL`` is not present.

               # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
               # in PC/launcher.c must also be updated.
-              MAGIC_NUMBER = (3561).to_bytes(2, 'little') + b'\r\n'

+                          GO_TO_INSTRUCTION(CALL_KW);
+                      }
+                      inst(CALL_KW, (callable, self_or_null, args[oparg], kwnames -- res)) {

GH-105848: Replace `KW_NAMES` + `CALL` with `LOAD_CONST` + `CALL_KW` by brandtbucher · Pull Request #109300 · python/cpython · GitHub

Uh oh!

Uh oh!

GH-105848: Replace KW_NAMES + CALL with LOAD_CONST + CALL_KW - #109300

GH-105848: Replace KW_NAMES + CALL with LOAD_CONST + CALL_KW#109300brandtbucher merged 14 commits intopython:mainpython/cpython:mainfrom brandtbucher:call-kwbrandtbucher/cpython:call-kwCopy head branch name to clipboard

Conversation

brandtbucher commented Sep 12, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Sep 12, 2023

Uh oh!

bedevere-bot commented Sep 12, 2023

Uh oh!

brandtbucher commented Sep 12, 2023

Uh oh!

markshannon commented Sep 12, 2023

Uh oh!

gvanrossum left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandtbucher commented Sep 13, 2023

Uh oh!

brandtbucher commented Sep 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

GH-105848: Replace KW_NAMES + CALL with LOAD_CONST + CALL_KW#109300
brandtbucher merged 14 commits into
python:mainfrom
brandtbucher:call-kw

brandtbucher commented Sep 12, 2023 •
edited by github-actions Bot
Loading

gvanrossum left a comment •
edited
Loading