Skip to content

bpo-45885: Specialize COMPARE_OP #29734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Dec 3, 2021
Merged

bpo-45885: Specialize COMPARE_OP #29734

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a301c92
specialize compare_op
sweeneyde Nov 23, 2021
9b5e15a
Add COMPARE_OP_STR for string equality
sweeneyde Nov 23, 2021
26cc70c
📜🤖 Added by blurb_it.
blurb-it[bot] Nov 23, 2021
6bfe99a
Use 3 combined instructions
sweeneyde Nov 28, 2021
3531f95
Merge branch 'compare' of https://github.com/sweeneyde/cpython into c…
sweeneyde Nov 28, 2021
a0f558e
remove unused variables
sweeneyde Nov 28, 2021
124693c
Remove adaptivity when specialization is impossible
sweeneyde Dec 2, 2021
91fc62a
Only consider Py_ABS(Py_SIZE(int)) <= 1
sweeneyde Dec 2, 2021
24c4def
Pack the mask into _PyAdaptiveEntry.index
sweeneyde Dec 2, 2021
24ba945
Introduce and use _PyUnicode_Equal
sweeneyde Dec 2, 2021
bb979de
Don't check for readiness in specialization
sweeneyde Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Include/internal/pycore_code.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ int _Py_Specialize_StoreSubscr(PyObject *container, PyObject *sub, _Py_CODEUNIT
int _Py_Specialize_CallFunction(PyObject *callable, _Py_CODEUNIT *instr, int nargs, SpecializedCacheEntry *cache, PyObject *builtins);
void _Py_Specialize_BinaryOp(PyObject *lhs, PyObject *rhs, _Py_CODEUNIT *instr,
SpecializedCacheEntry *cache);
void _Py_Specialize_CompareOp(PyObject *lhs, PyObject *rhs, _Py_CODEUNIT *instr, SpecializedCacheEntry *cache);

#define PRINT_SPECIALIZATION_STATS 0
#define PRINT_SPECIALIZATION_STATS_DETAILED 0
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_long.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ static inline PyObject* _PyLong_GetOne(void)
PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right);
PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right);
PyObject *_PyLong_Subtract(PyLongObject *left, PyLongObject *right);
PyObject *_PyLong_RichCompare(PyLongObject *left, PyLongObject *right, int op);

/* Used by Python/mystrtoul.c, _PyBytes_FromHex(),
_PyBytes_DecodeEscape(), etc. */
Expand Down
78 changes: 41 additions & 37 deletions Include/opcode.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Lib/opcode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ def jabs_op(name, op):
"BINARY_OP_MULTIPLY_FLOAT",
"BINARY_OP_SUBTRACT_INT",
"BINARY_OP_SUBTRACT_FLOAT",
"COMPARE_OP_ADAPTIVE",
"COMPARE_OP_FLOAT",
"COMPARE_OP_INT",
"COMPARE_OP_STR",
"BINARY_SUBSCR_ADAPTIVE",
"BINARY_SUBSCR_GETITEM",
"BINARY_SUBSCR_LIST_INT",
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2021-11-23-21-01-56.bpo-45885.3IxeCX.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Specialized the ``COMPARE_OP`` opcode using the PEP 659 machinery.
31 changes: 25 additions & 6 deletions Objects/longobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2956,16 +2956,35 @@ long_compare(PyLongObject *a, PyLongObject *b)
return sign;
}

PyObject *
_PyLong_RichCompare(PyLongObject *left, PyLongObject *right, int op)
{
Py_ssize_t diff;
if (left == right) {
diff = 0;
}
else {
diff = long_compare(left, right);
}
int cmp;
switch (op) {
case Py_LT: cmp = (diff < 0); break;
case Py_LE: cmp = (diff <= 0); break;
case Py_EQ: cmp = (diff == 0); break;
case Py_NE: cmp = (diff != 0); break;
case Py_GT: cmp = (diff > 0); break;
case Py_GE: cmp = (diff >= 0); break;
default: Py_UNREACHABLE();
}
return PyBool_FromLong(cmp);
}

static PyObject *
long_richcompare(PyObject *self, PyObject *other, int op)
{
Py_ssize_t result;
CHECK_BINOP(self, other);
if (self == other)
result = 0;
else
result = long_compare((PyLongObject*)self, (PyLongObject*)other);
Py_RETURN_RICHCOMPARE(result, 0, op);
return _PyLong_RichCompare((PyLongObject *)self,
(PyLongObject *)other, op);
}

static Py_hash_t
Expand Down
101 changes: 101 additions & 0 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3670,6 +3670,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
}

TARGET(COMPARE_OP) {
PREDICTED(COMPARE_OP);
STAT_INC(COMPARE_OP, unquickened);
assert(oparg <= Py_GE);
PyObject *right = POP();
PyObject *left = TOP();
Expand All @@ -3684,6 +3686,104 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
DISPATCH();
}

TARGET(COMPARE_OP_ADAPTIVE) {
assert(cframe.use_tracing == 0);
SpecializedCacheEntry *cache = GET_CACHE();
if (cache->adaptive.counter == 0) {
PyObject *right = TOP();
PyObject *left = SECOND();
next_instr--;
_Py_Specialize_CompareOp(left, right, next_instr, cache);
DISPATCH();
}
else {
STAT_INC(COMPARE_OP, deferred);
cache->adaptive.counter--;
oparg = cache->adaptive.original_oparg;
STAT_DEC(COMPARE_OP, unquickened);
JUMP_TO_INSTRUCTION(COMPARE_OP);
}
}

TARGET(COMPARE_OP_FLOAT) {
SpecializedCacheEntry *caches = GET_CACHE();
_PyAdaptiveEntry *cache0 = &caches[0].adaptive;
PyObject *right = TOP();
PyObject *left = SECOND();
DEOPT_IF(!PyFloat_CheckExact(left), COMPARE_OP);
DEOPT_IF(!PyFloat_CheckExact(right), COMPARE_OP);
STAT_INC(COMPARE_OP, hit);
double dleft = PyFloat_AS_DOUBLE(left);
double dright = PyFloat_AS_DOUBLE(right);
int cmp;
switch (cache0->original_oparg) {
case Py_LT: cmp = (dleft < dright); break;
case Py_LE: cmp = (dleft <= dright); break;
case Py_EQ: cmp = (dleft == dright); break;
case Py_NE: cmp = (dleft != dright); break;
case Py_GT: cmp = (dleft > dright); break;
case Py_GE: cmp = (dleft >= dright); break;
default: Py_UNREACHABLE();
}
// This cannot fail
PyObject *res = PyBool_FromLong(cmp);
assert(!PyErr_Occurred());
SET_SECOND(res);
STACK_SHRINK(1);
Py_DECREF(left);
Py_DECREF(right);
PREDICT(POP_JUMP_IF_FALSE);
PREDICT(POP_JUMP_IF_TRUE);
DISPATCH();
}

TARGET(COMPARE_OP_INT) {
SpecializedCacheEntry *caches = GET_CACHE();
_PyAdaptiveEntry *cache0 = &caches[0].adaptive;
PyObject *right = TOP();
PyObject *left = SECOND();
DEOPT_IF(!PyLong_CheckExact(left), COMPARE_OP);
DEOPT_IF(!PyLong_CheckExact(right), COMPARE_OP);
STAT_INC(COMPARE_OP, hit);
// This cannot fail.
PyObject *res = _PyLong_RichCompare(
(PyLongObject *)left, (PyLongObject *)right,
cache0->original_oparg);
assert(res != NULL);
assert(!PyErr_Occurred());
SET_SECOND(res);
STACK_SHRINK(1);
Py_DECREF(left);
Py_DECREF(right);
PREDICT(POP_JUMP_IF_FALSE);
PREDICT(POP_JUMP_IF_TRUE);
DISPATCH();
}

TARGET(COMPARE_OP_STR) {
SpecializedCacheEntry *caches = GET_CACHE();
_PyAdaptiveEntry *cache0 = &caches[0].adaptive;
PyObject *right = TOP();
PyObject *left = SECOND();
DEOPT_IF(!PyUnicode_CheckExact(left), COMPARE_OP);
DEOPT_IF(!PyUnicode_CheckExact(right), COMPARE_OP);
DEOPT_IF(!PyUnicode_IS_READY(left), COMPARE_OP);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? If left == right then they are equal regardless of whether the string is ready or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect that almost all strings are ready. I suppose the alternative is

            DEOPT_IF(!PyUnicode_CheckExact(left), COMPARE_OP);
            DEOPT_IF(!PyUnicode_CheckExact(right), COMPARE_OP);
            int res = 1;
            if (left != right) {
                DEOPT_IF(!PyUnicode_IS_READY(left), COMPARE_OP);
                DEOPT_IF(!PyUnicode_IS_READY(left), COMPARE_OP);
                res = comare_the_strings_assuming_theyre_ready(left, right);
            }

is that what you mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Assuming that the identity shortcut is worthwhile, then it makes sense to shortcut as much work as possible.
You are also missing out of the quick inequality test using lengths len(a) != len(b) implies that a != b.

Maybe wrap unicode_compare_eq in an equality function that readies the strings (I'm surprised that no such thing exists) and use that?

int res = 1;
if (left != right) {
    res = _PyUnicode_Equal(left, right);
}

DEOPT_IF(!PyUnicode_IS_READY(right), COMPARE_OP);
STAT_INC(COMPARE_OP, hit);
assert(cache0->original_oparg == Py_EQ || cache0->original_oparg == Py_NE);
int cmp = Py_Is(left, right) || _PyUnicode_EQ(left, right);
cmp ^= (cache0->original_oparg == Py_NE);
PyObject *res = PyBool_FromLong(cmp);
assert(!PyErr_Occurred());
SET_SECOND(res);
STACK_SHRINK(1);
Py_DECREF(left);
Py_DECREF(right);
PREDICT(POP_JUMP_IF_FALSE);
PREDICT(POP_JUMP_IF_TRUE);
DISPATCH();
}

TARGET(IS_OP) {
PyObject *right = POP();
PyObject *left = TOP();
Expand Down Expand Up @@ -4970,6 +5070,7 @@ MISS_WITH_CACHE(LOAD_GLOBAL)
MISS_WITH_CACHE(LOAD_METHOD)
MISS_WITH_CACHE(CALL_FUNCTION)
MISS_WITH_CACHE(BINARY_OP)
MISS_WITH_CACHE(COMPARE_OP)
MISS_WITH_CACHE(BINARY_SUBSCR)
MISS_WITH_OPARG_COUNTER(STORE_SUBSCR)

Expand Down
46 changes: 23 additions & 23 deletions Python/opcode_targets.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading