Repository: python/cpython
Pull request: GH-103963: Make dis display names of args for intrinsics opcodes
Make the dis module display the names of the args for opcodes `CALL_INTRINSIC_*`.

Fixes #103963

<!-- gh-issue-number: gh-103963 -->
* Issue: gh-103963
<!-- /gh-issue-number -->


Problem statement:
dis: Show names of intrinsics
For `CALL_INTRINSIC_*` opcodes, `dis` currently just shows the number, which makes it hard to figure out what the code actually does:

```
              2 LOAD_CONST               0 ('T')
              4 CALL_INTRINSIC_1         7
```

It would be nice if this instead said

```
              2 LOAD_CONST               0 ('T')
              4 CALL_INTRINSIC_1         7 (INTRINSIC_TYPEVAR)
```

Possible implementation strategy:
- instrinsic names are defined in opcode.py
- `pycore_intrinsics.h` is generated from opcode.py (bonus: we no longer have to manually update MAX_INTRINSIC_1)
- `dis` learns to read the intrinsic names from opcode.py


<!-- gh-linked-prs -->
### Linked PRs
* gh-104029
* gh-104177
<!-- /gh-linked-prs -->

Hints from issue discussion:
@jkchandalia are you interested in this?
@iritkatriel I can work on this.
Great. Have a look at /Tools/build/generate_opcode_h.py
(re generating the .h file).
> Great. Have a look at /Tools/build/generate_opcode_h.py
> (re generating the .h file).

Use ``make regen-opcode`` to run this script.
@jkchandalia thanks! It would be nice for me if this can make it in before the 3.12 feature freeze (May 8), since #103764 relies heavily on intrinsics, so this feature will make the bytecode generated by that PR much easier to understand. If that timeline is too tight for you, I'm happy to have a go at implementing this myself.
@JelleZijlstra that timeline is fine for me.
Great! Let me know if you need any help.

FYI I think these are some of the easiest ways to trigger intrinsics on current main for testing:

```
>>> dis.dis('(*a,)')
  0           0 RESUME                   0

  1           2 BUILD_LIST               0
              4 LOAD_NAME                0 (a)
              6 LIST_EXTEND              1
              8 CALL_INTRINSIC_1         6
             10 RETURN_VALUE
>>> dis.dis('+a')
  0           0 RESUME                   0

  1           2 LOAD_NAME                0 (a)
              4 CALL_INTRINSIC_1         5
              6 RETURN_VALUE
```
Absolutely, thank you. And I appreciate the examples.
@JelleZijlstra I've pushed my branch (https://github.com/jkchandalia/cpython/tree/display_intrinsics_names) and I wasn't quite sure if it's ready for a PR but can definitely open one if that's easier for feedback/discussion.

1. I added two lists to `opcode.py` for the intrinsics names (I added an extra option for not valid, not sure if this is needed)
2. I use these lists in `dis.py` to get the `argrepr` to pass to the `Instruction` class.
3. I added tests for args 2, 5, and 6 for `CALL_INTRINSIC_1` and updated an existing test for arg 3. I put the three new tests into one test function but can split out if better. I had some trouble getting something to trigger the other codes (0?, 1, 4) and also `CALL_INTRINSIC_2` for additional testing.
4. I updated `/Tools/build/generate_opcode_h.py` to include the new intrinsic names sections (not sure if this is desired?)
5. I regenerated `opcode.h` with the new intrinsics-related sections
6. I could definitely write the code to generate `pycore_intrinsics.h` as you suggested but I wasn’t quite sure where to put it? A separate script or another internal file output from `/Tools/build/generate_opcode_h.py` or something else?
Looks great! Feel free to open a draft PR and we can get it over the finish line.

You can get the CALL_INTRINSIC_2 with `except*` (position 38):
```
>>> dis.dis("try: pass\nexcept* Exception: x")
  0           0 RESUME                   0

  1           2 RETURN_CONST             0 (None)
              4 PUSH_EXC_INFO

  2           6 BUILD_LIST               0
              8 COPY                     2
             10 LOAD_NAME                0 (Exception)
             12 CHECK_EG_MATCH
             14 COPY                     1
             16 POP_JUMP_IF_NONE         8 (to 34)
             18 POP_TOP
             20 LOAD_NAME                1 (x)
             22 POP_TOP
             24 JUMP_FORWARD             3 (to 32)
        >>   26 LIST_APPEND              3
             28 POP_TOP
             30 JUMP_FORWARD             2 (to 36)
        >>   32 JUMP_FORWARD             1 (to 36)
        >>   34 POP_TOP
        >>   36 LIST_APPEND              1
             38 CALL_INTRINSIC_2         1
             40 COPY                     1
             42 POP_JUMP_IF_NOT_NONE     3 (to 50)
             44 POP_TOP
             46 POP_EXCEPT
             48 RETURN_CONST             0 (None)
        >>   50 SWAP                     2
             52 POP_EXCEPT
             54 RERAISE                  0
        >>   56 COPY                     3
             58 POP_EXCEPT
             60 RERAISE                  1
ExceptionTable:
  4 to 18 -> 56 [1] lasti
  20 to 22 -> 26 [4] lasti
  24 to 44 -> 56 [1] lasti
```

I don't think it's necessary to test every single one of the intrinsics in test_dis.

I would suggest simply expanding `generate_opcode_h.py` to also generate `pycore_intrinsics.h`. It already generates multiple files, so it doesn't seem too bad to make it generate another closely related file. We'll add a header to `pycore_intrinsics.h` to make it clear how it is generated. Perhaps @iritkatriel has a different opinion, though. We definitely shouldn't add the intrinsics to `opcode.h`, as that's a public header file and we don't want the intrinsics to be part of the public API.

Commit message: Regenerate intrinsics header file

Patch to review:
diff --git a/Include/internal/pycore_intrinsics.h b/Include/internal/pycore_intrinsics.h
index 46a52740eb8a0c..ec4731330b677e 100644
--- a/Include/internal/pycore_intrinsics.h
+++ b/Include/internal/pycore_intrinsics.h
@@ -1,26 +1,23 @@
+// Auto-generated by Tools/build/generate_opcode_h.py from Lib/opcode.py
 
 /* Unary Functions: */
+#define INTRINSIC_INVALID                        0
+#define INTRINSIC_PRINT                          1
+#define INTRINSIC_IMPORT_STAR                    2
+#define INTRINSIC_STOPITERATION_ERROR            3
+#define INTRINSIC_ASYNC_GEN_WRAP                 4
+#define INTRINSIC_UNARY_POSITIVE                 5
+#define INTRINSIC_LIST_TO_TUPLE                  6
 
-#define INTRINSIC_PRINT 1
-#define INTRINSIC_IMPORT_STAR 2
-#define INTRINSIC_STOPITERATION_ERROR 3
-#define INTRINSIC_ASYNC_GEN_WRAP 4
-#define INTRINSIC_UNARY_POSITIVE 5
-#define INTRINSIC_LIST_TO_TUPLE 6
-
-#define MAX_INTRINSIC_1 6
+#define MAX_INTRINSIC_1                          6
 
 
 /* Binary Functions: */
+#define INTRINSIC_INVALID                        0
+#define INTRINSIC_PREP_RERAISE_STAR              1
 
-#define INTRINSIC_PREP_RERAISE_STAR 1
-
-#define MAX_INTRINSIC_2 1
-
-
+#define MAX_INTRINSIC_2                          1
 typedef PyObject *(*instrinsic_func1)(PyThreadState* tstate, PyObject *value);
 typedef PyObject *(*instrinsic_func2)(PyThreadState* tstate, PyObject *value1, PyObject *value2);
-
 extern const instrinsic_func1 _PyIntrinsics_UnaryFunctions[];
-extern const instrinsic_func2 _PyIntrinsics_BinaryFunctions[];
-
+extern const instrinsic_func2 _PyIntrinsics_BinaryFunctions[];
\ No newline at end of file
diff --git a/Lib/dis.py b/Lib/dis.py
index 85c109584bf94f..3a8e6ac3bf5ace 100644
--- a/Lib/dis.py
+++ b/Lib/dis.py
@@ -11,6 +11,8 @@
     _cache_format,
     _inline_cache_entries,
     _nb_ops,
+    _intrinsic_1_descs,
+    _intrinsic_2_descs,
     _specializations,
     _specialized_instructions,
 )
@@ -42,6 +44,8 @@
 SEND = opmap['SEND']
 LOAD_ATTR = opmap['LOAD_ATTR']
 LOAD_SUPER_ATTR = opmap['LOAD_SUPER_ATTR']
+CALL_INTRINSIC_1 = opmap['CALL_INTRINSIC_1']
+CALL_INTRINSIC_2 = opmap['CALL_INTRINSIC_2']
 
 CACHE = opmap["CACHE"]
 
@@ -506,6 +510,10 @@ def _get_instructions_bytes(code, varname_from_oparg=None,
                                     if arg & (1<<i))
             elif deop == BINARY_OP:
                 _, argrepr = _nb_ops[arg]
+            elif deop == CALL_INTRINSIC_1:
+                argrepr = _intrinsic_1_descs[arg]
+            elif deop == CALL_INTRINSIC_2:
+                argrepr = _intrinsic_2_descs[arg]
         yield Instruction(_all_opname[op], op,
                           arg, argval, argrepr,
                           offset, starts_line, is_jump_target, positions)
diff --git a/Lib/opcode.py b/Lib/opcode.py
index aef8407948df15..8753e61b28c654 100644
--- a/Lib/opcode.py
+++ b/Lib/opcode.py
@@ -306,6 +306,21 @@ def pseudo_op(name, op, real_ops):
     ("NB_INPLACE_XOR", "^="),
 ]
 
+_intrinsic_1_descs = [
+    "INTRINSIC_INVALID",
+    "INTRINSIC_PRINT",
+    "INTRINSIC_IMPORT_STAR",
+    "INTRINSIC_STOPITERATION_ERROR",
+    "INTRINSIC_ASYNC_GEN_WRAP",
+    "INTRINSIC_UNARY_POSITIVE",
+    "INTRINSIC_LIST_TO_TUPLE",
+]
+
+_intrinsic_2_descs = [
+    'INTRINSIC_INVALID',
+    'INTRINSIC_PREP_RERAISE_STAR',
+    ]
+
 _specializations = {
     "BINARY_OP": [
         "BINARY_OP_ADD_FLOAT",
diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
index 5262c5c257cb89..bec3beecad5385 100644
--- a/Lib/test/test_dis.py
+++ b/Lib/test/test_dis.py
@@ -247,6 +247,72 @@ def wrap_func_w_kwargs():
 """ % (wrap_func_w_kwargs.__code__.co_firstlineno,
        wrap_func_w_kwargs.__code__.co_firstlineno + 1)
 
+dis_intrinsic_1_2 = """\
+  0        RESUME                   0
+
+  1        LOAD_CONST               0 (0)
+           LOAD_CONST               1 (('*',))
+           IMPORT_NAME              0 (math)
+           CALL_INTRINSIC_1         2 (INTRINSIC_IMPORT_STAR)
+           POP_TOP
+           RETURN_CONST             2 (None)
+"""
+
+dis_intrinsic_1_5 = """\
+  0        RESUME                   0
+
+  1        LOAD_NAME                0 (a)
+           CALL_INTRINSIC_1         5 (INTRINSIC_UNARY_POSITIVE)
+           RETURN_VALUE
+"""
+
+dis_intrinsic_1_6 = """\
+  0        RESUME                   0
+
+  1        BUILD_LIST               0
+           LOAD_NAME                0 (a)
+           LIST_EXTEND              1
+           CALL_INTRINSIC_1         6 (INTRINSIC_LIST_TO_TUPLE)
+           RETURN_VALUE
+"""
+dis_intrinsic_2_1 = """\
+  0        RESUME                   0
+
+  1        RETURN_CONST             0 (None)
+           PUSH_EXC_INFO
+
+  2        BUILD_LIST               0
+           COPY                     2
+           LOAD_NAME                0 (Exception)
+           CHECK_EG_MATCH
+           COPY                     1
+           POP_JUMP_IF_NONE         8 (to 34)
+           POP_TOP
+           LOAD_NAME                1 (x)
+           POP_TOP
+           JUMP_FORWARD             3 (to 32)
+        >> LIST_APPEND              3
+           POP_TOP
+           JUMP_FORWARD             2 (to 36)
+        >> JUMP_FORWARD             1 (to 36)
+        >> POP_TOP
+        >> LIST_APPEND              1
+           CALL_INTRINSIC_2         1 (INTRINSIC_PREP_RERAISE_STAR)
+           COPY                     1
+           POP_JUMP_IF_NOT_NONE     3 (to 50)
+           POP_TOP
+           POP_EXCEPT
+           RETURN_CONST             0 (None)
+        >> SWAP                     2
+           POP_EXCEPT
+           RERAISE                  0
+        >> COPY                     3
+           POP_EXCEPT
+           RERAISE                  1
+ExceptionTable:
+3 rows
+"""
+
 _BIG_LINENO_FORMAT = """\
   1        RESUME                   0
 
@@ -549,7 +615,7 @@ async def _asyncwith(c):
         >> COPY                     3
            POP_EXCEPT
            RERAISE                  1
-        >> CALL_INTRINSIC_1         3
+        >> CALL_INTRINSIC_1         3 (INTRINSIC_STOPITERATION_ERROR)
            RERAISE                  1
 ExceptionTable:
 12 rows
@@ -942,6 +1008,15 @@ def test_kw_names(self):
         # Test that value is displayed for KW_NAMES
         self.do_disassembly_test(wrap_func_w_kwargs, dis_kw_names)
 
+    def test_intrinsic_1(self):
+        # Test that argrepr is displayed for CALL_INTRINSIC_1
+        self.do_disassembly_test("from math import *", dis_intrinsic_1_2)
+        self.do_disassembly_test("+a", dis_intrinsic_1_5)
+        self.do_disassembly_test("(*a,)", dis_intrinsic_1_6)
+
+    def test_intrinsic_2(self):
+        self.do_disassembly_test("try: pass\nexcept* Exception: x", dis_intrinsic_2_1)
+
     def test_big_linenos(self):
         def func(count):
             namespace = {}
diff --git a/Tools/build/generate_opcode_h.py b/Tools/build/generate_opcode_h.py
index 645b9f1de1170b..284737c11e192f 100644
--- a/Tools/build/generate_opcode_h.py
+++ b/Tools/build/generate_opcode_h.py
@@ -52,6 +52,11 @@
 #endif  // !Py_INTERNAL_OPCODE_H
 """
 
+intrinsic_header = f"""
+// Auto-generated by {SCRIPT_NAME} from {PYTHON_OPCODE}
+
+""".lstrip()
+
 DEFINE = "#define {:<38} {:>3}\n"
 
 UINT32_MASK = (1<<32)-1
@@ -67,7 +72,9 @@ def write_int_array_from_ops(name, ops, out):
     assert bits == 0
     out.write(f"}};\n")
 
-def main(opcode_py, outfile='Include/opcode.h', internaloutfile='Include/internal/pycore_opcode.h'):
+def main(opcode_py, outfile='Include/opcode.h',
+         internaloutfile='Include/internal/pycore_opcode.h',
+         intrinsicoutfile='Include/internal/pycore_intrinsics.h'):
     opcode = {}
     if hasattr(tokenize, 'open'):
         fp = tokenize.open(opcode_py)   # Python 3.2+
@@ -107,9 +114,11 @@ def main(opcode_py, outfile='Include/opcode.h', internaloutfile='Include/interna
         opname_including_specialized[next_op] = name
         used[next_op] = True
 
-    with open(outfile, 'w') as fobj, open(internaloutfile, 'w') as iobj:
+    with open(outfile, 'w') as fobj, open(internaloutfile, 'w') as iobj, open(
+            intrinsicoutfile, "w") as nobj:
         fobj.write(header)
         iobj.write(internal_header)
+        nobj.write(intrinsic_header)
 
         for name in opname:
             if name in opmap:
@@ -172,6 +181,32 @@ def main(opcode_py, outfile='Include/opcode.h', internaloutfile='Include/interna
         for i, (op, _) in enumerate(opcode["_nb_ops"]):
             fobj.write(DEFINE.format(op, i))
 
+        nobj.write("/* Unary Functions: */")
+        nobj.write("\n")
+        for i, op in enumerate(opcode["_intrinsic_1_descs"]):
+            nobj.write(DEFINE.format(op, i))
+        nobj.write("\n")
+        nobj.write(DEFINE.format("MAX_INTRINSIC_1", i))
+
+        nobj.write("\n\n")
+        nobj.write("/* Binary Functions: */\n")
+        for i, op in enumerate(opcode["_intrinsic_2_descs"]):
+            nobj.write(DEFINE.format(op, i))
+        nobj.write("\n")
+        nobj.write(DEFINE.format("MAX_INTRINSIC_2", i))
+
+        nobj.write(
+            "typedef PyObject *(*instrinsic_func1)(PyThreadState* tstate, PyObject *value);"
+        )
+        nobj.write("\n")
+        nobj.write(
+            "typedef PyObject *(*instrinsic_func2)(PyThreadState* tstate, PyObject *value1, PyObject *value2);"
+        )
+        nobj.write("\n")
+        nobj.write("extern const instrinsic_func1 _PyIntrinsics_UnaryFunctions[];")
+        nobj.write("\n")
+        nobj.write("extern const instrinsic_func2 _PyIntrinsics_BinaryFunctions[];")
+
         fobj.write("\n")
         fobj.write("/* Defined in Lib/opcode.py */\n")
         fobj.write(f"#define ENABLE_SPECIALIZATION {int(ENABLE_SPECIALIZATION)}")


Code review comments:
diff_hunk: @@ -247,6 +247,72 @@ def wrap_func_w_kwargs():
 """ % (wrap_func_w_kwargs.__code__.co_firstlineno,
        wrap_func_w_kwargs.__code__.co_firstlineno + 1)
 
+dis_intrinsic_1_2 = """\
+  0        RESUME                   0
+
+  1        LOAD_CONST               0 (0)
+           LOAD_CONST               1 (('*',))
+           IMPORT_NAME              0 (math)
+           CALL_INTRINSIC_1         2 (INTRINSIC_IMPORT_STAR)
+           POP_TOP
+           RETURN_CONST             2 (None)
+"""
+
+dis_intrinsic_1_5 = """\
+  0        RESUME                   0
+
+  1        LOAD_NAME                0 (a)
+           CALL_INTRINSIC_1         5 (INTRINSIC_UNARY_POSITIVE)
+           RETURN_VALUE
+"""
+
+dis_intrinsic_1_6 = """\
+  0        RESUME                   0
+
+  1        BUILD_LIST               0
+           LOAD_NAME                0 (a)
+           LIST_EXTEND              1
+           CALL_INTRINSIC_1         6 (INTRINSIC_LIST_TO_TUPLE)
+           RETURN_VALUE
+"""
+dis_intrinsic_2_1 = """\
path: Lib/test/test_dis.py
text: @user1:
This is quite a big block of code that will have to be updated when any of the generated code changes. Would it be possible to instead test only for the `CALL_INTRINSIC_2         1 (INTRINSIC_PREP_RERAISE_STAR)` line, so we don't have to hardcode so much bytecode?

diff_hunk: @@ -172,6 +181,32 @@ def main(opcode_py, outfile='Include/opcode.h', internaloutfile='Include/interna
         for i, (op, _) in enumerate(opcode["_nb_ops"]):
             fobj.write(DEFINE.format(op, i))
 
+        nobj.write("/* Unary Functions: */")
+        nobj.write("\n")
+        for i, op in enumerate(opcode["_intrinsic_1_descs"]):
+            nobj.write(DEFINE.format(op, i))
+        nobj.write("\n")
+        nobj.write(DEFINE.format("MAX_INTRINSIC_1", i))
+
+        nobj.write("\n\n")
+        nobj.write("/* Binary Functions: */\n")
+        for i, op in enumerate(opcode["_intrinsic_2_descs"]):
+            nobj.write(DEFINE.format(op, i))
+        nobj.write("\n")
+        nobj.write(DEFINE.format("MAX_INTRINSIC_2", i))
+
+        nobj.write(
path: Tools/build/generate_opcode_h.py
text: @user1:
Let's move all this to a variable `intrinsic_footer` like we do for the header.

diff_hunk: @@ -172,6 +181,32 @@ def main(opcode_py, outfile='Include/opcode.h', internaloutfile='Include/interna
         for i, (op, _) in enumerate(opcode["_nb_ops"]):
             fobj.write(DEFINE.format(op, i))
 
+        nobj.write("/* Unary Functions: */")
+        nobj.write("\n")
+        for i, op in enumerate(opcode["_intrinsic_1_descs"]):
+            nobj.write(DEFINE.format(op, i))
+        nobj.write("\n")
+        nobj.write(DEFINE.format("MAX_INTRINSIC_1", i))
+
+        nobj.write("\n\n")
+        nobj.write("/* Binary Functions: */\n")
+        for i, op in enumerate(opcode["_intrinsic_2_descs"]):
+            nobj.write(DEFINE.format(op, i))
+        nobj.write("\n")
+        nobj.write(DEFINE.format("MAX_INTRINSIC_2", i))
+
+        nobj.write(
+            "typedef PyObject *(*instrinsic_func1)(PyThreadState* tstate, PyObject *value);"
+        )
+        nobj.write("\n")
+        nobj.write(
+            "typedef PyObject *(*instrinsic_func2)(PyThreadState* tstate, PyObject *value1, PyObject *value2);"
+        )
+        nobj.write("\n")
+        nobj.write("extern const instrinsic_func1 _PyIntrinsics_UnaryFunctions[];")
+        nobj.write("\n")
+        nobj.write("extern const instrinsic_func2 _PyIntrinsics_BinaryFunctions[];")
path: Tools/build/generate_opcode_h.py
text: @user1:
```suggestion
        nobj.write("extern const instrinsic_func2 _PyIntrinsics_BinaryFunctions[];")
        nobj.write("\n")
```

@user1:
(Added missing newline)