Non-optimal code generation for equality test

[robertwb]

Reported by gfurnish on 2 Aug 2008 12:17 UTC Consider:

 151:                 if top._expression_type==enum_SymbolicConstant and PyObject_TypeCheck((<SymbolicConstant>top)._obj,Integer) and (<SymbolicConstant>top)._obj<0:

        __pyx_1 = __Pyx_PyBool_FromLong((__pyx_v_top->_expression_type == __pyx_e_4sage_9symbolics_10expression_enum_SymbolicConstant)); if (unlikely(!__pyx_1)) {__pyx_filename = __pyx_f[__pyx_lineno = 151; __pyx_clineno = __LINE__; goto __pyx_L1;}
        __pyx_2 = __Pyx_PyObject_IsTrue(__pyx_1); if (unlikely(__pyx_2 < 0)) {__pyx_filename = __pyx_f[0](0];); __pyx_lineno = 151; __pyx_clineno = __LINE__; goto __pyx_L1;}
        if (__pyx_2) {

Note it is converting the result of a C boolean compare to a Python boolean object, then converting it back to a C boolean.

Migrated-From: http://trac.cython.org/ticket/38

[robertwb]

Comment by robertwb on 3 Aug 2008 05:42 UTC Note that Pyx_PyBool_FromLong and Pyx_PyObject_IsTrue are macros, and the compiler should be able to unwind the definitions to do the right thing here.

However, the reason this happens is that your "and" has a Python expression on the right, so the left operand is converted to a Python expression as well. You can cast the right to a bint to get what you want.

[robertwb]

Comment by anonymous on 10 Aug 2008 20:59 UTC I'm not at all convinced that it is a safe assumption to assume that gcc will optimize this away under all circumstances. (In particular note that PyBool_FromLong increments true or false, but is true does not undo this, so not all of the effects will get optimized away). There is a workaround so this is not a high priority either way (if the compiler does not optimize it away, we can use bints, and if it does, additions are quick).

[robertwb]

Modified by robertwb on 19 Aug 2008 04:36 UTC

[robertwb]

Comment by Stefan Behnel on 5 Sep 2008 09:30 UTC I agree that a boolean statement should stay in C space as long as possible. So, instead of emitting a type conversion depending on any of the subexpressions being Python expressions, we should look at each side of a boolean operator separately, as that's the way it's evaluated in the C code anyway.

[robertwb]

Comment by robertwb on 29 Oct 2008 00:23 UTC This has been done: http://hg.cython.org/cython-devel/rev/c2374c45d350