gh-98831: Use opcode metadata for stack_effect() by gvanrossum · Pull Request #101704 · python/cpython · GitHub

In order to do this, I had to change the generated opcode_metadata.h file to be sensitive to whether NEED_OPCODE_TABLES is defined -- if it is, it defines the functions and data with public linkage, if it isn't it only declares them. That way the file can be included in multiple places without worry about duplicating static data or functions.

Also, the pushed/popped functions will now accept any integer opcode argument and return -1 for unknown opcodes.

This also changes the command line options for generate_cases.py to write all output files in a single run, and updates Makefile.pre.in accordingly. This halves the time to run the cases generator (since most of the time goes into parsing bytecodes.c).

Issue: Generate the interpreter #98831

This halves the time to run the cases generator (most of the time goes into parsing the input).

We have these:

#define MAX_REAL_OPCODE 254
 
#define IS_WITHIN_OPCODE_RANGE(opcode) \
        (((opcode) >= 0 && (opcode) <= MAX_REAL_OPCODE) || \
         IS_PSEUDO_OPCODE(opcode))

Okay, I'll use opcode <= MAX_REAL_OPCODE. I feel we don't need IS_PSEUDO_OPCODE() because the switch will take care of that.

Wouldn't it be correct to return the stack effect of _PyOpcode_Deopt[opcode]?

Wouldn't it be correct to return the stack effect of _PyOpcode_Deopt[opcode]?

I thought so, but there are unit tests that insist that this is an error, and I didn't feel like changing the tests:

        # All not defined opcodes
        for code in set(range(256)) - set(dis.opmap.values()):
            with self.subTest(opcode=code):
                self.assertRaises(ValueError, stack_effect, code)
                self.assertRaises(ValueError, stack_effect, code, 0)

Ah, those tests probably predate specialization. We don't need to change this now.

opcode.py goes to great lengths to hide the existence of specialized instructions -- they don't show in either opname or opmap, even though pseudo ops do exist there. So I think it's by design. Though we could argue for changing that design.

I found the algorithm in dis.py on L46-52. I think it's duplicated in Tools/build/generate_opcode_h.py on L145-151.

I think dis.py:46-52 is actually allocating unused opcodes for the specialised instructions, while generate_opcode_h.py:145-151 is building the full deopt lookup table from opcode["_specializations"], plus mapping each normal opcode to itself.

Oh wait, in generate_opcode_h.py it's L101-107. These two algorithms ought to match, otherwise results will be hilarious. :-)

Anyway, I'm going to merge now. On to figuring out whether mark_stacks() is in reach yet (I doubt it).

Ah yes. Wow, we need to fix that.

* main: (82 commits) pythongh-101670: typo fix in PyImport_ExtendInittab() (python#101723) pythonGH-99293: Document that `Py_TPFLAGS_VALID_VERSION_TAG` shouldn't be used. (#pythonGH-101736) no-issue: Add Dong-hee Na as the cjkcodecs codeowner (pythongh-101731) pythongh-101678: Merge math_1_to_whatever() and math_1() (python#101730) pythongh-101678: refactor the math module to use special functions from c11 (pythonGH-101679) pythongh-85984: Remove legacy Lib/pty.py code. (python#92365) pythongh-98831: Use opcode metadata for stack_effect() (python#101704) pythongh-101283: Version was just released, so should be changed in 3.11.3 (pythonGH-101719) pythongh-101283: Fix use of unbound variable (pythonGH-101712) pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286) pythongh-101277: Port more itertools static types to heap types (python#101304) pythongh-98831: Modernize CALL and family (python#101508) pythonGH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (python#101697) pythongh-100221: Fix creating dirs in `make sharedinstall` (pythonGH-100329) pythongh-101670: typo fix in PyImport_AppendInittab() (pythonGH-101672) pythongh-101196: Make isdir/isfile/exists faster on Windows (pythonGH-101324) pythongh-101614: Don't treat python3_d.dll as a Python DLL when checking extension modules for incompatibility (pythonGH-101615) pythongh-100933: Improve `check_element` helper in `test_xml_etree` (python#100934) pythonGH-101578: Normalize the current exception (pythonGH-101607) pythongh-47937: Note that Popen attributes are read-only (python#93070) ...

bedevere-bot added the awaiting core review label Feb 8, 2023

bedevere-bot mentioned this pull request Feb 8, 2023

Generate the interpreter #98831

Closed

gvanrossum added the skip news label Feb 8, 2023

iritkatriel reviewed Feb 8, 2023

View reviewed changes

Comment thread Makefile.pre.in

gvanrossum added 7 commits February 8, 2023 15:15

Write output and metadata in a single run

f59751d

This halves the time to run the cases generator (most of the time goes into parsing the input).

Declare or define opcode metadata based on NEED_OPCODE_TABLES

383c1fc

Use generated metadata for stack_effect()

c5db6bc

compile.o depends on opcode_metadata.h

683d8f7

Return -1 for unknown opcode

a271ebc

Support jump<0 in stack_effect()

977e639

stack_effect() for specialized opcode is an error

bd19d79

gvanrossum force-pushed the cases-infra branch from 8d5a6c4 to bd19d79 Compare February 8, 2023 23:16

gvanrossum marked this pull request as ready for review February 8, 2023 23:16

gvanrossum requested a review from markshannon as a code owner February 8, 2023 23:16

gvanrossum requested a review from iritkatriel February 8, 2023 23:17

iritkatriel approved these changes Feb 8, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Feb 8, 2023

Use MAX_REAL_OPCODE as upper opcode limit

658ea17

gvanrossum merged commit 65b7b6b into python:main Feb 9, 2023

bedevere-bot removed the awaiting merge label Feb 9, 2023

gvanrossum deleted the cases-infra branch February 9, 2023 00:23

+                  if (0 <= opcode && opcode < 256) {
+                      if (_PyOpcode_Deopt[opcode] != opcode) {
+                          // Specialized instructions are not supported.
+                          return PY_INVALID_STACK_EFFECT;

gh-98831: Use opcode metadata for stack_effect() by gvanrossum · Pull Request #101704 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-98831: Use opcode metadata for stack_effect() - #101704

gh-98831: Use opcode metadata for stack_effect()#101704
gvanrossum merged 8 commits into
python:mainfrom
gvanrossum:cases-infra

gvanrossum commented Feb 8, 2023 •
edited
Loading

Uh oh!

Uh oh!

Uh oh!

iritkatriel Feb 8, 2023

Uh oh!

gvanrossum Feb 8, 2023

Uh oh!

iritkatriel Feb 8, 2023

Uh oh!

gvanrossum Feb 8, 2023

Uh oh!

iritkatriel Feb 8, 2023

Uh oh!

gvanrossum Feb 8, 2023

Uh oh!

iritkatriel Feb 9, 2023

Uh oh!

gvanrossum Feb 9, 2023

Uh oh!

iritkatriel Feb 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-                      case LOAD_BUILD_CLASS:
-                          return 1;
+                  if (0 <= opcode && opcode < 256) {

gh-98831: Use opcode metadata for stack_effect() by gvanrossum · Pull Request #101704 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-98831: Use opcode metadata for stack_effect() - #101704

gh-98831: Use opcode metadata for stack_effect()#101704gvanrossum merged 8 commits intopython:mainpython/cpython:mainfrom gvanrossum:cases-infragvanrossum/cpython:cases-infraCopy head branch name to clipboard

Conversation

gvanrossum commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gh-98831: Use opcode metadata for stack_effect()#101704
gvanrossum merged 8 commits into
python:mainfrom
gvanrossum:cases-infra

gvanrossum commented Feb 8, 2023 •
edited
Loading