Re: [Cython] Bug in Cython producing incorrect C code
On 01/25/2012 07:59 AM, Vitja Makarov wrote: 2012/1/25 mark florisson: On 24 January 2012 19:18, Dag Sverre Seljebotn wrote: On 01/24/2012 08:05 PM, Vitja Makarov wrote: 2012/1/24 mark florisson: On 24 January 2012 18:30, Vitja Makarovwrote: 2012/1/24 Robert Bradshaw: On Tue, Jan 24, 2012 at 6:09 AM, Vitja Makarov wrote: 2012/1/24 mark florisson: On 24 January 2012 11:37, Konrad Hinsen wrote: Compiling the attached Cython file produced the attached C file which has errors in lines 532-534: __pyx_v_self->xx = None; __pyx_v_self->yy = None; __pyx_v_self->zz = None; There is no C symbol "None", so this doesn't compile. I first noticed the bug in Cython 0.15, but it's still in the latest revision from Github. Konrad. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel Hm, it seems the problem is that the call to the builtin float results in SimpleCallNode being replaced with PythonCApiNode, which then generates the result code, but the list of coerced nodes are CloneNodes of the original rhs, and CloneNode does not generate the result code of the original rhs (i.e. allocate and assign to a temp), which results in a None result. Maybe CascadedAssignmentNode should replace CloneNode.arg with the latest self.rhs in generate_assignment_code? I'm not entirely sure. Seems like a hack to me. May be it's better to run OptimizeBuiltinCalls before AnalyseExpressionsTransform? Doesn't OptimizeBuiltinCalls take advantage of type information? Yes, it does :( So as Mark said the problem is CascadedAssignmentNode.coerced_rhs_list is created before rhs is updated. I think deferring the CloneNode creation to code generation time works (are there any known problem with doing type coercions at code generation time?). Coercion errors at code generation time? Apologies up front for raising my voice, as my knowledge of the internals are getting so rusty...take this with a grain of salt. I'm +1 on working towards having the code generation phase be pure code generation. I did some refactorings to take mini-steps towards that once upon a time, moving some error conditions to before code generation. My preferred approach would be to do away with CascadedAssignmentNode at the parse tree stage: a = b = c = expr goes to tmp = expr c = tmp b = tmp a = tmp and so on. Of course it gets messier; (expr1)[expr2] = (expr3).attr = expr4 But apart from getting the time of evaluating each expression right the transform should be straightforward. One of the tempnodes/"let"-nodes (I forgot which one, or if they've been consolidated) should be able to fix this. Takes some more work though than a quick hack though... Dag In principle it was doing the same thing, apart from the actual rewrite. I suppose the replacement problem can also be circumvented by manually wrapping self.rhs in a CoerceToTempNode. The problem with coerce_to_temp is that it does not create this node if the result is already in a temp. Creating it manually does mean an extra useless assignment, but it is an easy fix which happens at analyse_types time. Instead we could also use another node that just proxies a few things like generate_result_code and the result method. I like the idea though, it would be nice to only handle things in SingleAssignmentNode. I recently added broadcasting (inserting leading dimensions) and scalar assignment to memoryviews, and you can only catch that at the assignment point. Currently it only supports single assignments as the functionality is only in SingleAssignmentNode. I must say though, the following would look a bit weird: a = b[:] = c[:, :] = d as you always expect a kind of "cascade", e.g. you expect c[:, :] to be assignable to b[:], or 'a', but none of that may be true at all. So I'm fine with disallowing that, I think people should only use cascaded assignment for variables. I don't think that is a problem myself; but that's perhaps just because I'm so used to it (and to "a = b.x = y" not invoking b.__getattr__, and so on). After all, that is what you get with Python and NumPy! This is, in a sense Python being a bit strange and us just following Python. So I'm +1 for supporting this if we can do it "by accident". E.g. save 'env' during analyse_types and in generate_assignment_code do rhs = CloneNode(self.rhs).coerce_to(lhs.type, self.env) rhs.generate_evaluation_code(code) lhs.generate_assignment_code(rhs, code) Seems to work. Yeah, that's better. I don't like idea of transforming cascade assignment into N single assignment since we might break some optimizations and loose CF info. But what if the user decides to write tmp = expr a = tmp b = tmp manually? Shouldn't the same optimizations apply then? Consider that if we can get down to a single assignment node, we can then split it into SingleAssignmentNode into "Assi
Re: [Cython] Bug in Cython producing incorrect C code
mark florisson, 24.01.2012 21:28: > I must say though, the following would look a bit weird: > > a = b[:] = c[:, :] = d > > as you always expect a kind of "cascade", e.g. you expect c[:, :] to > be assignable to b[:], or 'a', but none of that may be true at all. That's normal for a typed language that has type auto-coercion. I consider this a major feature. It certainly makes the internals tricky, but when working on the assignment code, I always tried to keep the coercions and the eventual assignment code independent, even in the face of efficient tuple unpacking, because I considered it the expected behaviour. Stefan ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] 0.16 release
On 25 January 2012 01:27, Robert Bradshaw wrote: > On Mon, Jan 23, 2012 at 2:27 AM, mark florisson > wrote: >> Hey, >> >> It's been almost three months since we talked about a 0.16 release, I >> think it's quite ready. It would already be a big release, it would be >> good to see how people like it, and to catch any issues etc before we >> pile on more features. > > I would love to do a release soon. Last time this came up, I think the > big issue was (compilation) performance regression. Has this been > adequately addressed? Sort of. Basically if you don't use memoryviews it will be as fast as it used to be, otherwise there is about a 3 second constant time overhead (on my machine). > The other issue is that there are a couple of > doctest failures with Sage. One source of problems is decorators due > to the (ugly) disallowing of function re-declarations, I'll try look > into this one. There are also a huge number of segfaults (see the > bottom of > https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-tests/lastSuccessfulBuild/artifact/log.txt > ) which we need to get to the bottom of. Oh I see. I suppose to try it out under a debugger one would have to compile the whole of sage from source? > - Robert > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] inline defnode calls
On 25 January 2012 06:49, Vitja Makarov wrote: > 2012/1/25 mark florisson : >> I just noticed the inline defnode call code. When I try to compile >> with 'cython -Xoptimize.inline_defnode_calls=True test.pyx' with the >> following code: >> >> def foo(x): print foo >> foo(10) >> >> I get >> >> Error compiling Cython file: >> >> ... >> def foo(x): >> print x >> >> foo(10) >> ^ >> >> >> test.pyx:4:3: Compiler crash in InlineDefNodeCalls >> >> ModuleNode.body = StatListNode(test.pyx:1:0) >> StatListNode.stats[2] = ExprStatNode(test.pyx:4:3) >> ExprStatNode.expr = SimpleCallNode(test.pyx:4:3, >> result_is_used = True, >> use_managed_ref = True) >> >> Compiler crash traceback from this point on: >> File "/Users/mark/cy/Cython/Compiler/Visitor.py", line 176, in _visitchild >> result = handler_method(child) >> File "/Users/mark/cy/Cython/Compiler/Optimize.py", line 1656, in >> visit_SimpleCallNode >> if not function_name.cf_state.is_single: >> AttributeError: 'NoneType' object has no attribute 'is_single' > > > Thanks for the report! The feature is still experimental and by > default is disabled. > Anyway it wouldn't work for your example. It works when we know what > exactly function is referred by the name so it's closure case: > > def foo(): > def bar(): > pass > bar() > > -- > vitja. > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel Ah, neat. I thought it was perhaps also defying monkeypatching. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] inline defnode calls
2012/1/25 mark florisson : > On 25 January 2012 06:49, Vitja Makarov wrote: >> 2012/1/25 mark florisson : >>> I just noticed the inline defnode call code. When I try to compile >>> with 'cython -Xoptimize.inline_defnode_calls=True test.pyx' with the >>> following code: >>> >>> def foo(x): print foo >>> foo(10) >>> >>> I get >>> >>> Error compiling Cython file: >>> >>> ... >>> def foo(x): >>> print x >>> >>> foo(10) >>> ^ >>> >>> >>> test.pyx:4:3: Compiler crash in InlineDefNodeCalls >>> >>> ModuleNode.body = StatListNode(test.pyx:1:0) >>> StatListNode.stats[2] = ExprStatNode(test.pyx:4:3) >>> ExprStatNode.expr = SimpleCallNode(test.pyx:4:3, >>> result_is_used = True, >>> use_managed_ref = True) >>> >>> Compiler crash traceback from this point on: >>> File "/Users/mark/cy/Cython/Compiler/Visitor.py", line 176, in _visitchild >>> result = handler_method(child) >>> File "/Users/mark/cy/Cython/Compiler/Optimize.py", line 1656, in >>> visit_SimpleCallNode >>> if not function_name.cf_state.is_single: >>> AttributeError: 'NoneType' object has no attribute 'is_single' >> >> >> Thanks for the report! The feature is still experimental and by >> default is disabled. >> Anyway it wouldn't work for your example. It works when we know what >> exactly function is referred by the name so it's closure case: >> >> def foo(): >> def bar(): >> pass >> bar() >> >> -- >> vitja. >> ___ >> cython-devel mailing list >> cython-devel@python.org >> http://mail.python.org/mailman/listinfo/cython-devel > > Ah, neat. I thought it was perhaps also defying monkeypatching. I'm thinking about implementing "conditional inlining": depending on what function actually is it'll make direct call to C function or PyObject_Call(). -- vitja. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] inline defnode calls
On 25 January 2012 11:24, Vitja Makarov wrote: > 2012/1/25 mark florisson : >> On 25 January 2012 06:49, Vitja Makarov wrote: >>> 2012/1/25 mark florisson : I just noticed the inline defnode call code. When I try to compile with 'cython -Xoptimize.inline_defnode_calls=True test.pyx' with the following code: def foo(x): print foo foo(10) I get Error compiling Cython file: ... def foo(x): print x foo(10) ^ test.pyx:4:3: Compiler crash in InlineDefNodeCalls ModuleNode.body = StatListNode(test.pyx:1:0) StatListNode.stats[2] = ExprStatNode(test.pyx:4:3) ExprStatNode.expr = SimpleCallNode(test.pyx:4:3, result_is_used = True, use_managed_ref = True) Compiler crash traceback from this point on: File "/Users/mark/cy/Cython/Compiler/Visitor.py", line 176, in _visitchild result = handler_method(child) File "/Users/mark/cy/Cython/Compiler/Optimize.py", line 1656, in visit_SimpleCallNode if not function_name.cf_state.is_single: AttributeError: 'NoneType' object has no attribute 'is_single' >>> >>> >>> Thanks for the report! The feature is still experimental and by >>> default is disabled. >>> Anyway it wouldn't work for your example. It works when we know what >>> exactly function is referred by the name so it's closure case: >>> >>> def foo(): >>> def bar(): >>> pass >>> bar() >>> >>> -- >>> vitja. >>> ___ >>> cython-devel mailing list >>> cython-devel@python.org >>> http://mail.python.org/mailman/listinfo/cython-devel >> >> Ah, neat. I thought it was perhaps also defying monkeypatching. > > > I'm thinking about implementing "conditional inlining": depending on > what function actually is it'll make direct call to C function or > PyObject_Call(). > Sounds like a good idea. Any idea how much faster that can be? > -- > vitja. > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] inline defnode calls
On 25 January 2012 11:32, mark florisson wrote: > On 25 January 2012 11:24, Vitja Makarov wrote: >> 2012/1/25 mark florisson : >>> On 25 January 2012 06:49, Vitja Makarov wrote: 2012/1/25 mark florisson : > I just noticed the inline defnode call code. When I try to compile > with 'cython -Xoptimize.inline_defnode_calls=True test.pyx' with the > following code: > > def foo(x): print foo > foo(10) > > I get > > Error compiling Cython file: > > ... > def foo(x): > print x > > foo(10) > ^ > > > test.pyx:4:3: Compiler crash in InlineDefNodeCalls > > ModuleNode.body = StatListNode(test.pyx:1:0) > StatListNode.stats[2] = ExprStatNode(test.pyx:4:3) > ExprStatNode.expr = SimpleCallNode(test.pyx:4:3, > result_is_used = True, > use_managed_ref = True) > > Compiler crash traceback from this point on: > File "/Users/mark/cy/Cython/Compiler/Visitor.py", line 176, in > _visitchild > result = handler_method(child) > File "/Users/mark/cy/Cython/Compiler/Optimize.py", line 1656, in > visit_SimpleCallNode > if not function_name.cf_state.is_single: > AttributeError: 'NoneType' object has no attribute 'is_single' Thanks for the report! The feature is still experimental and by default is disabled. Anyway it wouldn't work for your example. It works when we know what exactly function is referred by the name so it's closure case: def foo(): def bar(): pass bar() -- vitja. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel >>> >>> Ah, neat. I thought it was perhaps also defying monkeypatching. >> >> >> I'm thinking about implementing "conditional inlining": depending on >> what function actually is it'll make direct call to C function or >> PyObject_Call(). >> > > Sounds like a good idea. Any idea how much faster that can be? Hm, probably about an order of magnitude for a noop function (simple test of cdef vs def call). >> -- >> vitja. >> ___ >> cython-devel mailing list >> cython-devel@python.org >> http://mail.python.org/mailman/listinfo/cython-devel ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] 0.16 release
mark florisson, 25.01.2012 11:43: > On 25 January 2012 01:27, Robert Bradshaw wrote: >> On Mon, Jan 23, 2012 at 2:27 AM, mark florisson wrote: >>> It's been almost three months since we talked about a 0.16 release, I >>> think it's quite ready. It would already be a big release, it would be >>> good to see how people like it, and to catch any issues etc before we >>> pile on more features. >> >> I would love to do a release soon. Last time this came up, I think the >> big issue was (compilation) performance regression. Has this been >> adequately addressed? > > Sort of. Basically if you don't use memoryviews it will be as fast as > it used to be, otherwise there is about a 3 second constant time > overhead (on my machine). > >> The other issue is that there are a couple of >> doctest failures with Sage. One source of problems is decorators due >> to the (ugly) disallowing of function re-declarations, I'll try look >> into this one. There are also a huge number of segfaults (see the >> bottom of >> https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-tests/lastSuccessfulBuild/artifact/log.txt >> ) which we need to get to the bottom of. > > Oh I see. I suppose to try it out under a debugger one would have to > compile the whole of sage from source? It might be easier to log into sage.math, go to the sage build directory that Jenkins uses and do some experiments there. It's in /levi/scratch/robertwb/hudson/sage-4.8/ Stefan ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] inline defnode calls
mark florisson, 25.01.2012 12:36: > On 25 January 2012 11:32, mark florisson wrote: >> On 25 January 2012 11:24, Vitja Makarov wrote: >>> I'm thinking about implementing "conditional inlining": depending on >>> what function actually is it'll make direct call to C function or >>> PyObject_Call(). >> >> Sounds like a good idea. Any idea how much faster that can be? > > Hm, probably about an order of magnitude for a noop function (simple > test of cdef vs def call). Easily, yes. It avoids all the argument type conversion and packing/unpacking. That's a huge overhead compared to a straight conditional C function call which the C compiler could even inline, or the CPU could at least keep in its branch prediction cache and optimise its pipeline for. Stefan ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] 0.16 release
On 25 January 2012 12:00, Stefan Behnel wrote: > mark florisson, 25.01.2012 11:43: >> On 25 January 2012 01:27, Robert Bradshaw wrote: >>> On Mon, Jan 23, 2012 at 2:27 AM, mark florisson wrote: It's been almost three months since we talked about a 0.16 release, I think it's quite ready. It would already be a big release, it would be good to see how people like it, and to catch any issues etc before we pile on more features. >>> >>> I would love to do a release soon. Last time this came up, I think the >>> big issue was (compilation) performance regression. Has this been >>> adequately addressed? >> >> Sort of. Basically if you don't use memoryviews it will be as fast as >> it used to be, otherwise there is about a 3 second constant time >> overhead (on my machine). >> >>> The other issue is that there are a couple of >>> doctest failures with Sage. One source of problems is decorators due >>> to the (ugly) disallowing of function re-declarations, I'll try look >>> into this one. There are also a huge number of segfaults (see the >>> bottom of >>> https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-tests/lastSuccessfulBuild/artifact/log.txt >>> ) which we need to get to the bottom of. >> >> Oh I see. I suppose to try it out under a debugger one would have to >> compile the whole of sage from source? > > It might be easier to log into sage.math, go to the sage build directory > that Jenkins uses and do some experiments there. It's in > > /levi/scratch/robertwb/hudson/sage-4.8/ > > Stefan > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel Ah, neat, thanks. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] 0.16 release
On Wed, Jan 25, 2012 at 4:17 AM, mark florisson wrote: > On 25 January 2012 12:00, Stefan Behnel wrote: >> mark florisson, 25.01.2012 11:43: >>> On 25 January 2012 01:27, Robert Bradshaw wrote: On Mon, Jan 23, 2012 at 2:27 AM, mark florisson wrote: > It's been almost three months since we talked about a 0.16 release, I > think it's quite ready. It would already be a big release, it would be > good to see how people like it, and to catch any issues etc before we > pile on more features. I would love to do a release soon. Last time this came up, I think the big issue was (compilation) performance regression. Has this been adequately addressed? >>> >>> Sort of. Basically if you don't use memoryviews it will be as fast as >>> it used to be, otherwise there is about a 3 second constant time >>> overhead (on my machine). >>> The other issue is that there are a couple of doctest failures with Sage. One source of problems is decorators due to the (ugly) disallowing of function re-declarations, I'll try look into this one. There are also a huge number of segfaults (see the bottom of https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-tests/lastSuccessfulBuild/artifact/log.txt ) which we need to get to the bottom of. >>> >>> Oh I see. I suppose to try it out under a debugger one would have to >>> compile the whole of sage from source? >> >> It might be easier to log into sage.math, go to the sage build directory >> that Jenkins uses and do some experiments there. It's in >> >> /levi/scratch/robertwb/hudson/sage-4.8/ And compiling Sage from scratch isn't actually that hard: type "make" and wait a couple of hours. I've updated the description at https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-build/ to explain how to build a cython-devel sage locally as others have asked for this as well, in summary you apply the patch at http://sage.math.washington.edu/home/robertwb/hudson-sage/sage-4.8/devel/sage-main/.hg/patches/0.16 to the repo in $SAGE_ROOT/devel/sage-main/ , install https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-build/lastSuccessfulBuild/artifact/cython-devel.spkg by downloading it and running "sage -i cython-devel.spkg" and then do "sage -ba" to re-build all Cython files. sage -gdb and sage -t -gdb /path/to/file are useful to know as well. - Robert ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] 0.16 release
On 1/25/12 11:39 AM, Robert Bradshaw wrote: install https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-build/lastSuccessfulBuild/artifact/cython-devel.spkg by downloading it and running "sage -i cython-devel.spkg" In fact, you could just do sage -i https://sage.math.washington.edu:8091/hudson/view/ext-libs/job/sage-build/lastSuccessfulBuild/artifact/cython-devel.spkg and Sage will (at least, should) download it for you, so that's even one less step! Jason ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Bug in Cython producing incorrect C code
2012/1/25 Stefan Behnel : > mark florisson, 24.01.2012 14:53: >> On 24 January 2012 11:37, Konrad Hinsen wrote: >>> Compiling the attached Cython file produced the attached C file which >>> has errors in lines 532-534: >>> >>> __pyx_v_self->xx = None; >>> __pyx_v_self->yy = None; >>> __pyx_v_self->zz = None; >>> >>> There is no C symbol "None", so this doesn't compile. >>> >>> I first noticed the bug in Cython 0.15, but it's still in the latest >>> revision from Github. >> >> Hm, it seems the problem is that the call to the builtin float results >> in SimpleCallNode being replaced with PythonCApiNode, which then >> generates the result code, but the list of coerced nodes are >> CloneNodes of the original rhs, and CloneNode does not generate the >> result code of the original rhs (i.e. allocate and assign to a temp), >> which results in a None result. > > Back to the old idea of separating the type analysis into 1) a basic > typing, inference and entry creation step and 2) a proper type analysis, > coercion, etc. step. > Yeah! I think the issue must be fixed before release. We can start moving slowly in this direction and split CascadedAssignmentNode.analyse_types into parts: - normal analyse_types()/expressions() - create clone nodes at some late stage > The type driven optimisations would then run in between the two. That would > simplify the optimisations (which would no longer have to unpack wrapped > nodes) and improve the type analysis because it could work with the > optimised types, e.g. return types of optimised builtin functions. > > I'm not entirely sure where the type inference should run. It may make more > sense to move it after the tree optimisations to make use of optimised > function calls. > > While we're at it, we should also replace the current type inference > mechanism with a control flow based one. > > Sounds like a good topic for a Cython hacking workshop. > Nice. Any news on that? -- vitja. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
