GH-125837: Split `LOAD_CONST` into three. by markshannon · Pull Request #125972 · python/cpython · GitHub

Splits LOAD_CONST into three instructions

LOAD_INT for ints in range(256). Avoids the need for a space in the co_consts tuple and avoids an incref
LOAD_CONST_IMMORTAL for other immortal objects. Avoids an incref
LOAD_CONST for the remaining constants

Issue: Refactor RETURN_VALUE and RETURN_CONST #125837

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

…_GEN

Co-authored-by: Tomas R. <tomas.roun8@gmail.com>

LOAD_INT for ints in range(256)

I think the range is -5 -> 256 (included) right? (at least _PyLong_SMALL_INTS works that way, but maybe 256 should be excluded even though it's stored in the _PyLong_SMALL_INTS).

I think the range is -5 -> 256 (included) right? (at least _PyLong_SMALL_INTS works that way, but maybe 256 should be excluded even though it's stored in the _PyLong_SMALL_INTS).

I want LOAD_INT 0 to push 0 not -5 to keep things simple.
So LOAD_INT will only cover range(256) even though the range of statically allocated ints is larger.

We can always add -5,-4,-3,-2 and -1 to LOAD_COMMON_CONST later if we want.

Performance for tier 1 appears to be about 0.5% faster.

Stats show the following fraction of constants loaded:

Opcode	Fraction
LOAD_INT	47%
LOAD_CONST_IMMORTAL	42%
LOAD_CONST	11%

With a reduction in increfs of immortal objects of about 20%.

hasconst is defined in the dis docs as "Sequence of bytecodes that access a constant". I think it will make more sense if we clarify that it means "access a constant from co_consts" and then remove LOAD_SMALL_INT from hasconst and OPCODE_HAS_CONST.

That makes sense. LOAD_COMMON_CONST is not in hasconst.

Maybe it's worth testing whether creating integers within the range(256) will not affect co_consts?

I don't want to go overboard on testing internal details of the bytecode compiler.
LOAD_CONST with a small int is still correct, just a bit less efficient than LOAD_SMALL_INT.
We already have plenty of tests in test_dis that test this

…om code.co_consts

* Add LOAD_CONST_IMMORTAL opcode * Add LOAD_SMALL_INT opcode * Remove RETURN_CONST opcode

markshannon and others added 16 commits October 22, 2024 15:57

Replace RETURN_VLAUE/RETURN_CONST with RETURN_VALUE_FUNC/RETURN_VALUE…

2a61cfc

…_GEN

Add LOAD_CONST_IMMORTAL

42a526c

Fix up all the tests apart from test_dis

2a3b1e2

Update optimizer

031f320

Fix up test_dis

cb5252b

Fix up a couple of comments

892edbc

Apply suggestions from code review

5ce99ca

Co-authored-by: Tomas R. <tomas.roun8@gmail.com>

Clarify comment

325c5fe

Remove unused function

6149153

Update test

59a554a

Update dis docs

bcb4e65

Remove link to remove instruction

f3383e5

Restore RETURN_VALUE

68e97ba

Add LOAD_INT instruction

0b8a5ad

Fix up dis and some tests

1bfdd70

Fix up test dis

f934559

bedevere-app Bot mentioned this pull request Oct 25, 2024

Refactor RETURN_VALUE and RETURN_CONST #125837

Closed

picnixz reviewed Oct 25, 2024

View reviewed changes

Comment thread Python/bytecodes.c Outdated

Comment thread Python/compile.c Outdated

Comment thread Python/flowgraph.c Outdated

markshannon added 2 commits October 25, 2024 15:15

Address review comments

f135b69

Fix __annotate__ docstring

8f2a781

Teach optimizer about LOAD_INT

97c6cdc

markshannon marked this pull request as ready for review October 25, 2024 14:39

markshannon requested review from Eclips4, brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners October 25, 2024 14:39

Eclips4 reviewed Oct 25, 2024

View reviewed changes

Comment thread Lib/test/test_ast/test_ast.py

Make functions static again to avoid warning

79305d2

Eclips4 reviewed Oct 25, 2024

View reviewed changes

Comment thread Lib/test/test_ast/test_ast.py Outdated

implement review suggestion

7e4b2a8

iritkatriel reviewed Oct 25, 2024

View reviewed changes

brettcannon removed their request for review October 25, 2024 20:01

JelleZijlstra reviewed Oct 27, 2024

View reviewed changes

Comment thread Python/codegen.c Outdated

markshannon added 2 commits October 28, 2024 10:06

Address review comments

57e891e

Fix typo

207e35e

iritkatriel reviewed Oct 28, 2024

View reviewed changes

Comment thread Doc/library/dis.rst

markshannon added 4 commits October 28, 2024 10:47

Remove extra argument

7c88f69

Rename LOAD_INT to LOAD_SMALL_INT

9d4e575

Regenerate test file

ae11453

Fix test_dis

3a50437

markshannon mentioned this pull request Oct 28, 2024

Add instructions for creating small objects. faster-cpython/ideas#609

Open

iritkatriel reviewed Oct 28, 2024

View reviewed changes

Comment thread Python/codegen.c

iritkatriel reviewed Oct 28, 2024

View reviewed changes

Don't mark LOAD_SMALL_INT as having a constant, as it doesn't load fr…

47927cf

…om code.co_consts

Eclips4 reviewed Oct 28, 2024

View reviewed changes

Comment thread Lib/test/test_ast/test_ast.py

Add back int to test

3caac1a

markshannon mentioned this pull request Oct 29, 2024

gh-126072: do not add None to co_consts if no docstring #126101

Merged

iritkatriel approved these changes Oct 29, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 29, 2024

markshannon merged commit faa3272 into python:main Oct 29, 2024

bedevere-app Bot removed the awaiting merge label Oct 29, 2024

markshannon deleted the split-load-const branch November 18, 2024 14:37

picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024

pythonGH-125837 : Split LOAD_CONST into three. (pythonGH-125972 )

7487220

* Add LOAD_CONST_IMMORTAL opcode * Add LOAD_SMALL_INT opcode * Remove RETURN_CONST opcode

ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025

pythonGH-125837 : Split LOAD_CONST into three. (pythonGH-125972 )

a02dbb3

* Add LOAD_CONST_IMMORTAL opcode * Add LOAD_SMALL_INT opcode * Remove RETURN_CONST opcode

eendebakpt mentioned this pull request Apr 26, 2025

Increase _PY_NSMALLPOSINTS size faster-cpython/ideas#725

Closed

                       for (int i = 0; i < b->b_iused; i++) {
-                          if (OPCODE_HAS_CONST(b->b_instr[i].i_opcode)) {
+                          int opcode = b->b_instr[i].i_opcode;
+                          if (OPCODE_HAS_CONST(opcode) && opcode != LOAD_SMALL_INT) {

GH-125837: Split `LOAD_CONST` into three. by markshannon · Pull Request #125972 · python/cpython · GitHub

Repository navigation

GH-125837: Split LOAD_CONST into three. - #125972

GH-125837: Split LOAD_CONST into three.#125972markshannon merged 34 commits intopython:mainpython/cpython:mainfrom faster-cpython:split-load-constfaster-cpython/cpython:split-load-constCopy head branch name to clipboard

Conversation

markshannon commented Oct 25, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

picnixz commented Oct 25, 2024

Uh oh!

markshannon commented Oct 25, 2024

Uh oh!

markshannon commented Oct 25, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iritkatriel Oct 28, 2024

Choose a reason for hiding this comment

Uh oh!

markshannon Oct 28, 2024

Choose a reason for hiding this comment

Uh oh!

Eclips4 Oct 28, 2024

Choose a reason for hiding this comment

Uh oh!

markshannon Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

GH-125837: Split LOAD_CONST into three.#125972
markshannon merged 34 commits into
python:mainfrom
faster-cpython:split-load-const

markshannon commented Oct 25, 2024 •
edited by github-actions Bot
Loading