Skip to content

bpo-39156: Break up COMPARE_OP into four logically distinct opcodes. #17754

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.

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 4 commits into from
Jan 14, 2020

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Dec 30, 2019

Breaks up COMPARE_OP into four logically distinct opcodes:

  • COMPARE_OP for rich comparisons
  • IS_OP for 'is' and 'is not' tests
  • CONTAINS_OP for 'in' and 'is not' tests
  • JUMP_IF_NOT_EXC_MATCH for checking exceptions in 'try-except' statements.

This improves the clarity of the interpreter and should provide a modest speedup.

https://bugs.python.org/issue39156

Copy link
Contributor

@ZackerySpytz ZackerySpytz left a comment

Choose a reason for hiding this comment

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

I have some minor comments.

@@ -961,6 +975,13 @@ All of the following opcodes use their arguments.

.. versionadded:: 3.1

.. opcode:: JUMP_IF_NOT_EXC_MATCH (target)

Tests whether the second value on the stack is an exception matching TOS, and jumps if it is not.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be wrapped at 80 chars.

Python/ceval.c Outdated
PyObject *left = POP();
if (PyTuple_Check(right)) {
Py_ssize_t i, length;
length = PyTuple_Size(right);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think PyTuple_GET_SIZE() should be used here.

Python/ceval.c Outdated
if (PyTuple_Check(right)) {
Py_ssize_t i, length;
length = PyTuple_Size(right);
for (i = 0; i < length; i += 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i++

int res = PyErr_GivenExceptionMatches(left, right);
Py_DECREF(left);
Py_DECREF(right);
if (res > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, this check for res > 0 should not be included.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why?

   COMPARE_OP for rich comparisons
   IS_OP for 'is' and 'is not' tests
   CONTAINS_OP for 'in' and 'is not' tests
   JUMP_IF_NOT_EXC_MATCH for checking exceptions in 'try-except' statements.
@markshannon markshannon reopened this Jan 10, 2020
@markshannon markshannon requested a review from a team as a code owner January 13, 2020 12:39
@markshannon markshannon merged commit 9af0e47 into python:master Jan 14, 2020
@markshannon markshannon deleted the breakup-compare-op branch January 14, 2020 10:12
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
…ythonGH-17754)

Break up COMPARE_OP into four logically distinct opcodes:
* COMPARE_OP for rich comparisons
* IS_OP for 'is' and 'is not' tests
* CONTAINS_OP for 'in' and 'is not' tests
* JUMP_IF_NOT_EXC_MATCH for checking exceptions in 'try-except' statements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants