GeezerVison

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All commit authors signed the Contributor License Agreement.

15-bit builds

Do we actually support such builds?

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

I suspect this has been entirely generated by an LLM (at least the summary has been generated by that). It's unclear what the issue is so please first wait for some feedbcak on the issue before opening the PR. This is the common process as highlighted in our devguide.

I suspect this has been entirely generated by an LLM (at least the summary has been generated by that). It's unclear what the issue is so please first wait for some feedbcak on the issue before opening the PR. This is the common process as highlighted in our devguide.

Hi, that's incorrect, I'm talking to a core dev directly on the the PR and as noted by the status, it's still a draft and WIP

Do we actually support such builds?

15-bit builds are still supported: --enable-big-digits=15 is a documented configure option, and CPython still has both 15-bit and 30-bit PyLong layouts. I called it out because widening the JIT path to full int64_t exposed a real correctness issue on that

https://github.com/python/cpython/blob/main/Doc/using/configure.rst#L229-L237
https://github.com/python/cpython/blob/main/configure.ac#L6452-L6466

Hi, that's incorrect, I'm talking to a core dev directly on the the PR and as noted by the status

Who's the core dev?

Ok for the 15-bit but please, first share a reproducer on the issue before jumping onto PRs. I don't know if you want to address correctness or performance. Those are different. And the described scope is not helpful as there are too many points on the issue.

I don't know if you want to address correctness or performance.

I'm addressing performance, the performance changes surfaced correctness issues on 15-bit by the test suite.

the described scope is not helpful as there are too many points on the issue.

I'm working on updating the title and bodies, as I said, this is a draft / WIP, I still haven't settled the final body as I'm doing some cleanup

Before doing any work, we should first understand the issue. It doesn't make sense to make PRs and update the issue accordingly. This is not how our worklow is. So please, first discuss the change on the issue by including a reproducer.

Before doing any work, we should first understand the issue.

I don't think what I'm doing is an "issue", though arguably performance bottlenecks are issues (which is something that I personally believe).

It doesn't make sense to make PRs and update the issue accordingly. This is not how our worklow is.

it is however, Github native workflow (opening PRs as a draft, a rough PR where as a draft it is iterated and improved upon), where cpython happens to be hosted in.

it is however, Github native workflow (opening PRs as a draft, a rough PR where as a draft it is iterated and improved upon), where cpython happens to be hosted in.

We have a policy written on the devguide. Contributors need to follow that policy. The purpose is to avoid back-and-forth so knowing first what the issue is about is the first step. PRs come after.

Nothing forbids you to open something on your fork though. But any change you make in a PR against CPython takes CI resources which affects other contributors.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Considering @Fidget-Spinner's stance, I'm reopening it. But please follow what the devguide says. We have too many AI-generated PRs that take both triagers and reviewers time.

When it comes to performance PRs, we really want to see benchmarks before PRs even if in a draft state. This gives more intution on whether the change is worth or not.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Follow-up from review:

• Tightened _PyLong_MightFitInt64() so it rejects max-digit values outside the signed 64-bit range instead of using digit count alone.
• Fixed tier-2 optimizer facts so _GUARD_*_INT_WIDE records PyLong_Type, not compact-int state.
• Added regression coverage for 1 << 64 staying on generic BINARY_OP instead of specializing to *_INT_WIDE.

I've updated the benchmarks, with JIT on and off.

Thanks! Results look good. I am wondering where the performance gain is coming from. I can think of at least three options:

• Covering more ints in the specializing code, e.g. avoiding the slow path via __add__
• Covering more ints, causing less deopts
• The fast path for int64 (e.g. ndigits <= _PY_LONG_MAX_DIGITS_FOR_INT64 , the _PyLong_TryAsInt64Exact and _Py_i64_add_overflow combination)

(probably it is a combination of the above). If the fast path for int64 is not contributing to the performance, we could drop it to simplify the code, if it is a significant contribution we could first create a PR adding the _PyLong_TryAsInt64Exact to long_add (in that way we can split this PR in multiple smaller PRs, each easier to review)

I see in one of the recent commits you splitted _BINARY_OP_SUBTRACT_INT into _BINARY_OP_SUBTRACT_INT and _BINARY_OP_SUBTRACT_INT_WIDE. What was the reason for that? It would be nice to keep the number of opcodes small. Even stronger: could we just have a single _BINARY_OP_SUBTRACT_INT that covers all ints? (I suspect it would be good for the int paths, but maybe interacts poorly with float+int addition)

Could you explore the above? (in separate branches to keep this PR a bit stable)

And to make life of reviewers easier: either mark the PR as draft, or keep changes to a minimum. Reviewing a moving target is not easy

@picnixz I marked it as ready to review because I considered it ready to review, however once I did that, I noticed the CI started to fail which is why I made more changes, let me switch it back to draft, and after that, I'll only mark it as ready for review once we both agree it is in fact, ready for that?

I think you can put your PR up to review when it is ready and let Ken Jin shepherd it as he was supportive of the idea (if he wants to)

oops sorry, I tagged the wrong person here @picnixz , I meant to tag @eendebakpt , got my thoughts messed up, apologies!

@eendebakpt

I see in one of the recent commits you splitted _BINARY_OP_SUBTRACT_INT into _BINARY_OP_SUBTRACT_INT and _BINARY_OP_SUBTRACT_INT_WIDE. What was the reason for that? It would be nice to keep the number of opcodes small. Even stronger: could we just have a single _BINARY_OP_SUBTRACT_INT that covers all ints? (I suspect it would be good for the int paths, but maybe interacts poorly with float+int addition)

the compact and wide variants are separate opcodes so that the compact arithmetic path has no ERROR_IF branch in JIT traces. _PyCompactLong_Subtract (compact-only) returns PyStackRef_NULL for both overflow and OOM, so the compact uop body is just:

https://github.com/KRRT7/cpython/blob/4d495669265/Python/executor_cases.c.h#L4772-L4777

res = _PyCompactLong_Subtract(...);
EXIT_IF(PyStackRef_IsNull(res));

The wide variant (_PyCompactLong_SubtractWide) additionally has an ERROR_IF for the overflow case that compact arithmetic can never trigger:

https://github.com/KRRT7/cpython/blob/4d495669265/Python/executor_cases.c.h#L4938-L4955

res = _PyCompactLong_SubtractWide(...);
EXIT_IF(PyStackRef_IsNull(res));
// ...
ERROR_IF(PyStackRef_IsError(res));

If they were one op, every JIT trace for compact int subtraction would carry that dead ERROR_IF edge → larger traces → more icache pressure.

this was causing a perf regression in my micro benchmarks which is how I caught it

I see in one of the recent commits you splitted _BINARY_OP_SUBTRACT_INT into _BINARY_OP_SUBTRACT_INT and _BINARY_OP_SUBTRACT_INT_WIDE. What was the reason for that? It would be nice to keep the number of opcodes small. Even stronger: could we just have a single _BINARY_OP_SUBTRACT_INT that covers all ints?

I explored the merged approach you suggested and replaced the split compact/wide design with a single _BINARY_OP_{ADD,SUBTRACT,MULTIPLY}_INT that handles all int64-range ints. The _PyCompactLong_{Add,Subtract,Multiply} functions branch internally on compactness — fast compact path when both are single-digit, transparent int64 fallback otherwise. All _WIDE opcodes and their corresponding tier-1 macros have been removed.

also I am worried about the following scenario: we have a part of the code working with ints both compact (99%) and wide(1%)... it specializes on the compact ones... then a single wide int arrives and we deopt.

With the merged design this doesn't happen — the single guard (PyLong_CheckExact) accepts all exact ints, and the merged arithmetic handles both compact and wide transparently. I also benchmarked the scenario: the original split handled it fine too (side-exits are fast, no thrashing).

I am wondering where the performance gain is coming from.

I created experiment branches to decompose this. The gain is almost entirely from the custom inline int64 fast path (_PyLong_TryAsInt64Exact + _wide_op_result). Replacing those with standard PyLong_AsLongLongAndOverflow + PyLong_FromLongLong was 1.3x slower overall. The compact-only fast path contributes ~6%. The opcode split itself contributes <1%.

Benchmarks (PGO+LTO, clang/LLVM, Apple M-series) against origin/main:

Targeted wide-int microbenchmarks (operands in [2^30, 2^63), the range the fast path covers):

import pyperf

BASE = 2**32        # 4_294_967_296 — solidly in wide-int range
DELTA = 2**32 + 1

def bench_wide_add(loops):
    x = BASE
    y = DELTA
    for _ in range(loops):
        x = x + y - y
    return x

def bench_wide_mul(loops):
    x = BASE
    y = 3
    for _ in range(loops):
        x = x * y // y
    return x

TS_BASE = 1_748_000_000   # representative Unix timestamp (wide-int range)
TS_DELTA = 86400          # one day in seconds (compact)

def bench_timestamp_arith(loops):
    t = TS_BASE
    d = TS_DELTA
    for _ in range(loops):
        t = t + d - d
    return t

runner = pyperf.Runner()
runner.bench_func('wide_add',          bench_wide_add,        1_000_000)
runner.bench_func('wide_mul',          bench_wide_mul,        1_000_000)
runner.bench_func('timestamp_arith',   bench_timestamp_arith, 1_000_000)

General pyperformance benchmarks (crypto_pyaes, deltablue, go, hexiom, nqueens): no significant difference on any — all within 1–2% noise, as expected for workloads that don't produce wide-range integers.

One correctness fix also landed in this branch: _BINARY_OP_SUBSCR_LIST_INT and _STORE_SUBSCR_LIST_INT were calling _PyLong_CompactValue unconditionally, but their runtime guard (_GUARD_TOS_INT) only checks PyLong_CheckExact — so a wide int could pass the guard and hit the assertion. The specializer correctly gates on _PyLong_IsNonNegativeCompact at specialization time, but that invariant wasn't enforced at execution time. Fixed by adding EXIT_IF/DEOPT_IF(!_PyLong_IsNonNegativeCompact(...)) before each _PyLong_CompactValue call. The bug was latent before this branch because BINARY_OP_ADD_INT previously deopted on wide operands, so wide results rarely reached an already-specialised subscript bytecode.

Do we actually support such builds?

@picnixz, 15-bit builds are supported, of course, but much less tested. We have just one build bot, added recently, see #138336.