cython / cython

The most widely used Python to C compiler
https://cython.org
Apache License 2.0
9.48k stars 1.49k forks source link

[ENH] Missed optimization in untyped int comparison #4819

Open da-woods opened 2 years ago

da-woods commented 2 years ago

Is your feature request related to a problem? Please describe.

Suppose you have a comparison of an untyped variable with an int

def f(x):
    if x == 1:
        return "xxx"

Cython assumes that x is likely an int and has an optimized function to compare it to the C value 1 (__Pyx_PyInt_EqObjC). However this function returns a Python True/False object. In this case (which I imagine is fairly common) the value is fed to an if statement and so is immediately converted to a C True/False value:

__pyx_t_1 = __Pyx_PyInt_EqObjC(__pyx_v_x, __pyx_int_1, 1, 0); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 2, __pyx_L1_error)
__Pyx_GOTREF(__pyx_t_1);
__pyx_t_2 = __Pyx_PyObject_IsTrue(__pyx_t_1); if (unlikely((__pyx_t_2 < 0))) __PYX_ERR(0, 2, __pyx_L1_error)
__Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;

Describe the solution you'd like

It'd be nice to be able to skip this PyObject intermediate. It'd be needed only in the fallback case right at the end of __Pyx_PyInt_EqObjC.

I imagine there's other similar optimized comparison functions that would also benefit from skipping the PyObject intermediate in the event that they're fed immediately to a C if statement (but I haven't looked in detail).


da-woods commented 2 years ago

I can see why this doesn't work - OptimizeBuiltinCallsTransform can't easily change the return type. There's a similar failure to optimize in cdef int y = x + 1

scoder commented 2 years ago

The transform also handles CoerceFromPyTypeNode and calls coerce_to() in some cases. As long as there is a known C integer result type (so that we can safely assume a valid integer range for the result), coercion could change the return type after the fact by replacing the implementation again.