Rename to CALL_CFUNCTION and generalize to all c functions

python · markshannon · Oct 19, 2021 · Jun 26, 2021 · Jun 26, 2021 · Jun 26, 2021
commit 1d841b0744591d3d64de8dced8a4fdf0237af29c
diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h
@@ -338,15 +338,15 @@ typedef enum {
     PYCFUNCTION_WITH_KEYWORDS = 4,
     _PYCFUNCTION_FAST = 5,
     _PYCFUNCTION_FAST_WITH_KEYWORDS = 6,
-    PYCMETHOD = 7, /* Isn't normally used in builtins. */
+    PYCMETHOD = 7,
 } _BuiltinCallKinds;
 
 /* Specialization functions */
 
 int _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache);
 int _Py_Specialize_LoadGlobal(PyObject *globals, PyObject *builtins, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache);
 int _Py_Specialize_CallFunction(PyObject **stack_pointer, uint8_t original_oparg,
-    PyObject *builtins, _Py_CODEUNIT *instr, SpecializedCacheEntry *cache);
+    _Py_CODEUNIT *instr, SpecializedCacheEntry *cache);
 
 #define SPECIALIZATION_STATS 0
 #define SPECIALIZATION_STATS_DETAILED 0

diff --git a/Include/opcode.h b/Include/opcode.h
diff --git a/Lib/opcode.py b/Lib/opcode.py
@@ -230,5 +230,5 @@ def jabs_op(name, op):
     "LOAD_GLOBAL_MODULE",
     "LOAD_GLOBAL_BUILTIN",
     "CALL_FUNCTION_ADAPTIVE",
-    "CALL_FUNCTION_BUILTIN",
+    "CALL_CFUNCTION",
 ]
@@ -55,7 +55,7 @@ Py_LOCAL_INLINE(PyObject *) call_function(
 static PyObject * do_call_core(
     PyThreadState *tstate, PyObject *func,
     PyObject *callargs, PyObject *kwdict, int use_tracing);
-Py_LOCAL_INLINE(PyObject *) call_function_builtin(
+Py_LOCAL_INLINE(PyObject *) call_cfunction(
     PyThreadState *tstate,
     _PyAdaptiveEntry *cache0,
     _PyCallFunctionCache *cache1,
@@ -4076,7 +4076,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
             SpecializedCacheEntry *cache = GET_CACHE();
             if (cache->adaptive.counter == 0) {
                 next_instr--;
-                if (_Py_Specialize_CallFunction(stack_pointer, cache->adaptive.original_oparg, BUILTINS(), next_instr, cache) < 0) {
+                if (_Py_Specialize_CallFunction(stack_pointer, cache->adaptive.original_oparg, next_instr, cache) < 0) {
                     goto error;
                 }
                 DISPATCH();
@@ -4088,7 +4088,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
                 JUMP_TO_INSTRUCTION(CALL_FUNCTION);
             }
         }
-        case TARGET(CALL_FUNCTION_BUILTIN): {
+        case TARGET(CALL_CFUNCTION): {
             /* Builtin functions, WITHOUT keywords */
             SpecializedCacheEntry *caches = GET_CACHE();
             _PyAdaptiveEntry *cache0 = &caches[0].adaptive;
@@ -4099,7 +4099,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
 
             PyObject **sp, *res;
             sp = stack_pointer;
-            res = call_function_builtin(tstate, cache0, cache1, &sp,
+            res = call_cfunction(tstate, cache0, cache1, &sp,
                 cache0->original_oparg, cframe.use_tracing);
             stack_pointer = sp;
             PUSH(res);
@@ -5920,9 +5920,9 @@ do_call_core(PyThreadState *tstate,
     return PyObject_Call(func, callargs, kwdict);
 }
 
-/* Fast alternative for non-keyword calls to builtins. */
+/* Fast alternative for non-keyword calls to C functions. */
 Py_LOCAL_INLINE(PyObject *) _Py_HOT_FUNCTION
-call_function_builtin(PyThreadState *tstate,
+call_cfunction(PyThreadState *tstate,
               _PyAdaptiveEntry *cache0,
               _PyCallFunctionCache *cache1,
               PyObject ***pp_stack,
@@ -5941,18 +5941,24 @@ call_function_builtin(PyThreadState *tstate,
 
     switch ((_BuiltinCallKinds)cache0->index) {
         case PYCFUNCTION_NOARGS:
-        case PYCFUNCTION_O: {
+        case PYCFUNCTION_O:
             MAYBE_TRACE(cfunc(self, *stack));
             break;
-        }
-        case _PYCFUNCTION_FAST: {
+        case _PYCFUNCTION_FAST:
             MAYBE_TRACE(((_PyCFunctionFast)(void(*)(void))cfunc)(self, stack, oparg));
             break;
-        }
-        case _PYCFUNCTION_FAST_WITH_KEYWORDS: {
+        case _PYCFUNCTION_FAST_WITH_KEYWORDS:
             MAYBE_TRACE(((_PyCFunctionFastWithKeywords)(void(*)(void))cfunc)(
                 self, stack, oparg, 0));
             break;
+        case PYCFUNCTION: {
+            PyObject *args = _PyTuple_FromArray(stack, oparg);
+            if (args == NULL) {
+                break;
+            }
+            MAYBE_TRACE(cfunc(self, args));
+            Py_DECREF(args);
+            break;
         }
         case PYCFUNCTION_WITH_KEYWORDS: {
             PyObject *args = _PyTuple_FromArray(stack, oparg);
@@ -5963,8 +5969,9 @@ call_function_builtin(PyThreadState *tstate,
             Py_DECREF(args);
             break;
         }
-        /* Bulitins shouldn't have these flags */
-        case PYCFUNCTION:
+        /* This flag only applies to PyMethodObject.
+           We're only optimizing for PyCfunctionObject
+        */
         case PYCMETHOD:
         default:
             Py_UNREACHABLE();

diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h
diff --git a/Python/specialize.c b/Python/specialize.c
@@ -636,40 +636,33 @@ _Py_Specialize_LoadGlobal(
     return 0;
 }
 
+/* TODO:
+    - Specialize calling C types like int() with CALL_CTYPE
+    - Specialize python function calls.
+*/
 int
 _Py_Specialize_CallFunction(PyObject **stack_pointer, uint8_t original_oparg,
-    PyObject *builtins, _Py_CODEUNIT *instr, SpecializedCacheEntry *cache)
+    _Py_CODEUNIT *instr, SpecializedCacheEntry *cache)
 {
     PyObject *callable = stack_pointer[-(original_oparg + 1)];
     _PyAdaptiveEntry *cache0 = &cache->adaptive;
     _PyCallFunctionCache *cache1 = &cache[-1].call_function;
     if (!PyCallable_Check(callable)) {
         goto fail;
     }
-    if (!PyDict_CheckExact(builtins)) {
-        goto fail;
-    }
-    PyDictObject *builtins_dict = (PyDictObject *)builtins;
-    if (builtins_dict->ma_keys->dk_kind != DICT_KEYS_UNICODE) {
-        goto fail;
-    }
-    /* Specialize C methods */
+    /* Specialize C functions */
     if (PyCFunction_CheckExact(callable)) {
         PyCFunctionObject *meth = (PyCFunctionObject *)callable;
         if (meth->m_ml == NULL) {
             goto fail;
         }
         const char *name_ascii = meth->m_ml->ml_name;
-        /* Specialize builtins: check method actually exists in builtins */
-        PyObject *value = PyDict_GetItemString(builtins, name_ascii);
-        if (value == NULL ||
-            value != (PyObject *)meth) {
-            goto fail;
-        }
         _BuiltinCallKinds kind = -1;
         switch (PyCFunction_GET_FLAGS(meth) & (METH_VARARGS | METH_FASTCALL |
             METH_NOARGS | METH_O | METH_KEYWORDS | METH_METHOD)) {
             case METH_VARARGS:
+                kind = PYCFUNCTION;
+                break;
             case METH_VARARGS | METH_KEYWORDS:
                 kind = PYCFUNCTION_WITH_KEYWORDS;
                 break;
@@ -685,17 +678,19 @@ _Py_Specialize_CallFunction(PyObject **stack_pointer, uint8_t original_oparg,
             case METH_O:
                 kind = PYCFUNCTION_O;
                 break;
+            /* This case should never happen with PyCFunctionObject -- only
+               PyMethodObject. See zlib.compressobj()'s methods for an example.
+            */
             case METH_METHOD | METH_FASTCALL | METH_KEYWORDS:
-                kind = PYCMETHOD;
-                break;
+                // kind = PYCMETHOD;
             default:
                 SPECIALIZATION_FAIL(CALL_FUNCTION, type, callable, "bad call flags");
-                goto fail;
+                return -1;
         }
         assert(kind > 0);
         PyCFunction cfunc = PyCFunction_GET_FUNCTION(meth);
         assert(cfunc != NULL);
-        *instr = _Py_MAKECODEUNIT(CALL_FUNCTION_BUILTIN, _Py_OPARG(*instr));
+        *instr = _Py_MAKECODEUNIT(CALL_CFUNCTION, _Py_OPARG(*instr));
         cache0->index = (uint16_t)kind;
         cache1->cfunc = cfunc;
         goto success;