Repository navigation
Conversation
|
@eendebakpt this is the rework you suggested on #151290: the compact specializations are left exactly as on main, and the multi-digit case goes through |
|
One trade-off worth flagging. Because the I also built the alternative: a guard function that only accepts exact ints that are not both compact, so compact pairs deopt and respecialize. Instructions per op ( Extra cost per compact op at a site that was specialized on a wide value:
Instructions saved per wide add/sub (
The guard function fixes the pinning without the JIT, but it cuts the gain on wide ints by half or more: calling the guard through the pointer and checking compactness costs more. Under the JIT it is worse than both main and this PR in the pinned case, most likely because each compact pair fails the guard inside the trace and exits it (I haven't confirmed that with the JIT's stats). It also stops the optimizer from dropping the guard when the operand types are known. So I've kept the type-only guard. The pinning only affects sites that specialize while seeing a wide value and later see mostly compact ones, and the cost there is bounded. If you'd rather never be slower than main in the default build, the guard-function version is ready and I can switch to it. |
| static PyObject * | ||
| longs_add(PyObject *lhs, PyObject *rhs) |
There was a problem hiding this comment.
It seems you actually want this type of API. So, why not make
PyObject *
_PyLong_Add(PyObject *a, PyObject *b)
{
return (PyObject *)long_add((PyLongObject *)a, (PyLongObject *)b);
}like _PyLong_DivmodNear()? Same for _PyLong_Subtract.
BTW, it seems that _PyCompactLong_Add() could accept PyObject's as well: in all current use cases current API needs type casts:
Modules/_testinternalcapi/test_cases.c.h: res = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
Objects/longobject.c:_PyCompactLong_Add(PyLongObject *a, PyLongObject *b)
Python/bytecodes.c: res = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
Python/bytecodes.c: INT_INPLACE_OP(left, right, left, +, _PyCompactLong_Add);
Python/bytecodes.c: INT_INPLACE_OP(left, right, right, +, _PyCompactLong_Add);
Python/ceval_macros.h:// is from FUNC (_PyCompactLong_Add etc.).
Python/ceval_macros.h:// FUNC is the fallback function (_PyCompactLong_Add etc.)
Python/executor_cases.c.h: res = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
Python/executor_cases.c.h: res = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
Python/executor_cases.c.h: res = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
Python/executor_cases.c.h: INT_INPLACE_OP(left, right, left, +, _PyCompactLong_Add);
Python/executor_cases.c.h: INT_INPLACE_OP(left, right, left, +, _PyCompactLong_Add);
Python/executor_cases.c.h: INT_INPLACE_OP(left, right, left, +, _PyCompactLong_Add);
Python/executor_cases.c.h: INT_INPLACE_OP(left, right, right, +, _PyCompactLong_Add);
Python/executor_cases.c.h: INT_INPLACE_OP(left, right, right, +, _PyCompactLong_Add);
Python/executor_cases.c.h: INT_INPLACE_OP(left, right, right, +, _PyCompactLong_Add);
Python/generated_cases.c.h: res = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
Python/optimizer_cases.c.h: res_stackref = _PyCompactLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
| PyAPI_FUNC(PyObject*) _PyLong_Add(PyLongObject *left, PyLongObject *right); | ||
| PyAPI_FUNC(PyObject*) _PyLong_Subtract(PyLongObject *left, PyLongObject *right); |
There was a problem hiding this comment.
Please also add comment, explaining why this added.
int + intandint - intare specialized only when both operands are compact. When either operand is a multi-digit int,BINARY_OPstays unspecialized and goes through the genericbinary_op1dispatch every time.This adds
BINARY_OP_EXTENDdescriptors for+,-,+=and-=on exactintoperands. The specializer still tries the compact path first, so the descriptors only apply to sites where at least one operand was non-compact when the site was specialized. The action callslong_add/long_subdirectly through new internal_PyLong_Add/_PyLong_Subtractwrappers.Design notes:
BINARY_OP_ADD_INT,BINARY_OP_SUBTRACT_INT, their guards and the optimizer are unchanged.guard == NULL,lhs_type/rhs_type=int), so the optimizer can already drop it when the operand types are known, e.g. in(a - b) - c.intbut not as compact, so later compact-int ops keep their guards. This avoids the wrong-result bug found in gh-151289: optimize shrinking wide int subtraction #151290, where a non-compact result was modelled as compact. That case is now a regression test intest_opt.result_unique = 1:long_add/long_subreturn either a new object or an immortal small int, which is what the debug assertion in_BINARY_OP_EXTENDchecks.PyLong_SHIFT.This follows the same direction as gh-155486, which allowed non-compact ints in
TO_BOOL_INT.Supersedes #151290.
Performance
Wall-clock microbenchmarks on a cloud VM were too noisy for a change this size. PGO layout alone moved untouched compact ops by ±5–8%, and repeated runs of the same binary drifted by up to ~9%. So these are user-space instructions per op (
perf stat -e instructions:u, difference between 3M and 1M iterations ofa OP bin a loop, empty loop subtracted, 5 repeats each), on 30-bit builds of current main. "sub wide" is10_000_000_000 - 1, "add wide" is10_000_000_000 + 1.That's 30–39 fewer instructions per wide add/sub (~10–12%) in every configuration, while compact ops are unchanged. Measured on 7dbc797 and this PR rebased on it.
pyperformance (LTO-only builds of the earlier base c22e9c9, one run each, pinned to an isolated core) shows no measurable change. The geometric mean is 1.03x faster, but benchmarks that do no wide-int arithmetic moved ±10–20% in both directions, so that's noise. This is expected: very little of the suite does non-compact int add/sub. The point of the change is that such code no longer stays on the unspecialized
BINARY_OPpath, without affecting the compact fast paths.