Merge pull request #80 from vitek/_defnode_refactor

DefNode refactoring and initial closure function call inlining

Cython/Compiler/ExprNodes.py

@@ -3864,6 +3864,103 @@ class SimpleCallNode(CallNode):
                 code.funcstate.release_temp(self.opt_arg_struct)
 
 
+class InlinedDefNodeCallNode(CallNode):
+    #  Inline call to defnode
+    #
+    #  function       PyCFunctionNode
+    #  function_name  NameNode
+    #  args           [ExprNode]
+
+    subexprs = ['args', 'function_name']
+    is_temp = 1
+    type = py_object_type
+    function = None
+    function_name = None
+
+    def can_be_inlined(self):
+        func_type= self.function.def_node
+        if func_type.star_arg or func_type.starstar_arg:
+            return False
+        if len(func_type.args) != len(self.args):
+            return False
+        return True
+
+    def analyse_types(self, env):
+        self.function_name.analyse_types(env)
+
+        for arg in self.args:
+            arg.analyse_types(env)
+
+        func_type = self.function.def_node
+        actual_nargs = len(self.args)
+
+        # Coerce arguments
+        some_args_in_temps = False
+        for i in xrange(actual_nargs):
+            formal_type = func_type.args[i].type
+            arg = self.args[i].coerce_to(formal_type, env)
+            if arg.is_temp:
+                if i > 0:
+                    # first argument in temp doesn't impact subsequent arguments
+                    some_args_in_temps = True
+            elif arg.type.is_pyobject and not env.nogil:
+                if arg.nonlocally_immutable():
+                    # plain local variables are ok
+                    pass
+                else:
+                    # we do not safely own the argument's reference,
+                    # but we must make sure it cannot be collected
+                    # before we return from the function, so we create
+                    # an owned temp reference to it
+                    if i > 0: # first argument doesn't matter
+                        some_args_in_temps = True
+                    arg = arg.coerce_to_temp(env)
+            self.args[i] = arg
+
+        if some_args_in_temps:
+            # if some args are temps and others are not, they may get
+            # constructed in the wrong order (temps first) => make
+            # sure they are either all temps or all not temps (except
+            # for the last argument, which is evaluated last in any
+            # case)
+            for i in xrange(actual_nargs-1):
+                arg = self.args[i]
+                if arg.nonlocally_immutable():
+                    # locals, C functions, unassignable types are safe.
+                    pass
+                elif arg.type.is_cpp_class:
+                    # Assignment has side effects, avoid.
+                    pass
+                elif env.nogil and arg.type.is_pyobject:
+                    # can't copy a Python reference into a temp in nogil
+                    # env (this is safe: a construction would fail in
+                    # nogil anyway)
+                    pass
+                else:
+                    #self.args[i] = arg.coerce_to_temp(env)
+                    # instead: issue a warning
+                    if i > 0:
+                        warning(arg.pos, "Argument evaluation order in C function call is undefined and may not be as expected", 0)
+                        break
+
+    def generate_result_code(self, code):
+        arg_code = [self.function_name.py_result()]
+        func_type = self.function.def_node
+        for arg, proto_arg in zip(self.args, func_type.args):
+            if arg.type.is_pyobject:
+                arg_code.append(arg.result_as(proto_arg.type))
+            else:
+                arg_code.append(arg.result())
+        arg_code = ', '.join(arg_code)
+        code.putln(
+            "%s = %s(%s); %s" % (
+                self.result(),
+                self.function.def_node.entry.pyfunc_cname,
+                arg_code,
+                code.error_goto_if_null(self.result(), self.pos)))
+        code.put_gotref(self.py_result())
+
+
 class PythonCapiFunctionNode(ExprNode):
     subexprs = []
     def __init__(self, pos, py_name, cname, func_type, utility_code = None):
@@ -6252,15 +6349,16 @@ class GeneratorExpressionNode(LambdaNode):
         super(GeneratorExpressionNode, self).analyse_declarations(env)
         # No pymethdef required
         self.def_node.pymethdef_required = False
+        self.def_node.py_wrapper_required = False
         self.def_node.is_cyfunction = False
         # Force genexpr signature
         self.def_node.entry.signature = TypeSlots.pyfunction_noargs
 
     def generate_result_code(self, code):
         code.putln(
-            '%s = %s(%s, NULL); %s' % (
+            '%s = %s(%s); %s' % (
                 self.result(),
-                self.def_node.entry.func_cname,
+                self.def_node.entry.pyfunc_cname,
                 self.self_result_code(),
                 code.error_goto_if_null(self.result(), self.pos)))
         code.put_gotref(self.py_result())

Cython/Compiler/FlowControl.py

@@ -327,6 +327,32 @@ class NameReference(object):
         return '%s(entry=%r)' % (self.__class__.__name__, self.entry)
 
 
+class ControlFlowState(list):
+    # Keeps track of Node's entry assignments
+    #
+    # cf_is_null        [boolean] It is uninitialized
+    # cf_maybe_null     [boolean] May be uninitialized
+    # is_single         [boolean] Has only one assignment at this point
+
+    cf_maybe_null = False
+    cf_is_null = False
+    is_single = False
+
+    def __init__(self, state):
+        if Uninitialized in state:
+            state.discard(Uninitialized)
+            self.cf_maybe_null = True
+            if not state:
+                self.cf_is_null = True
+        else:
+            if len(state) == 1:
+                self.is_single = True
+        super(ControlFlowState, self).__init__(state)
+
+    def one(self):
+        return self[0]
+
+
 class GVContext(object):
     """Graphviz subgraph object."""
 
@@ -530,11 +556,10 @@ def check_definitions(flow, compiler_directives):
 
     messages.report()
 
-    # Remove Uninitialized from cf_state
     for node in assmt_nodes:
-        node.cf_state.discard(Uninitialized)
+        node.cf_state = ControlFlowState(node.cf_state)
     for node in references:
-        node.cf_state.discard(Uninitialized)
+        node.cf_state = ControlFlowState(node.cf_state)
 
 
 class AssignmentCollector(TreeVisitor):
@@ -632,13 +657,7 @@ class ControlFlowAnalysis(CythonTransform):
         return node
 
     def visit_DefNode(self, node):
-        ## XXX: no target name node here
         node.used = True
-        entry = node.entry
-        if entry.is_anonymous:
-            entry = self.env.lookup(node.name)
-        if entry:
-            self.flow.mark_assignment(node, object_expr_not_none, entry)
         return self.visit_FuncDefNode(node)
 
     def visit_GeneratorBodyDefNode(self, node):

Cython/Compiler/Naming.py

@@ -19,6 +19,7 @@ funcdoc_prefix    = pyrex_prefix + "doc_"
 enum_prefix       = pyrex_prefix + "e_"
 func_prefix       = pyrex_prefix + "f_"
 pyfunc_prefix     = pyrex_prefix + "pf_"
+pywrap_prefix     = pyrex_prefix + "pw_"
 genbody_prefix    = pyrex_prefix + "gb_"
 gstab_prefix      = pyrex_prefix + "getsets_"
 prop_get_prefix   = pyrex_prefix + "getprop_"

Cython/Compiler/Optimize.py

@@ -1643,6 +1643,28 @@ class EarlyReplaceBuiltinCalls(Visitor.EnvTransform):
             return node
         return kwargs
 
+class InlineDefNodeCalls(Visitor.CythonTransform):
+    visit_Node = Visitor.VisitorTransform.recurse_to_children
+
+    def visit_SimpleCallNode(self, node):
+        self.visitchildren(node)
+        if not self.current_directives.get('optimize.inline_defnode_calls'):
+            return node
+        function_name = node.function
+        if not function_name.is_name:
+            return node
+        if not function_name.cf_state.is_single:
+            return node
+        function = function_name.cf_state.one().rhs
+        if not isinstance(function, ExprNodes.PyCFunctionNode):
+            return node
+        inlined = ExprNodes.InlinedDefNodeCallNode(
+            node.pos, function_name=function_name,
+            function=function, args=node.args)
+        if inlined.can_be_inlined():
+            return inlined
+        return node
+
 
 class OptimizeBuiltinCalls(Visitor.EnvTransform):
     """Optimize some common methods calls and instantiation patterns

Cython/Compiler/Options.py

@@ -106,6 +106,9 @@ directive_defaults = {
     'warn.unused_arg': False,
     'warn.unused_result': False,
 
+# optimizations
+    'optimize.inline_defnode_calls': False,
+
 # remove unreachable code
     'remove_unreachable': True,
 

Cython/Compiler/ParseTreeTransforms.py

@@ -1493,6 +1493,9 @@ if VALUE is not None:
 
             if node.py_func:
                 node.stats.insert(0, node.py_func)
+                self.visit(node.py_func)
+                if node.py_func.needs_assignment_synthesis(env):
+                    node = [node, self._synthesize_assignment(node.py_func, env)]
         else:
             node.body.analyse_declarations(lenv)
 
@@ -1514,6 +1517,54 @@ if VALUE is not None:
         self.seen_vars_stack.pop()
         return node
 
+    def visit_DefNode(self, node):
+        node = self.visit_FuncDefNode(node)
+        env = self.env_stack[-1]
+        if (not isinstance(node, Nodes.DefNode) or
+            node.fused_py_func or node.is_generator_body or
+            not node.needs_assignment_synthesis(env)):
+            return node
+        return [node, self._synthesize_assignment(node, env)]
+
+    def _synthesize_assignment(self, node, env):
+        # Synthesize assignment node and put it right after defnode
+        genv = env
+        while genv.is_py_class_scope or genv.is_c_class_scope:
+            genv = genv.outer_scope
+
+        if genv.is_closure_scope:
+            rhs = node.py_cfunc_node = ExprNodes.InnerFunctionNode(
+                node.pos, def_node=node,
+                pymethdef_cname=node.entry.pymethdef_cname,
+                code_object=ExprNodes.CodeObjectNode(node))
+        else:
+            rhs = ExprNodes.PyCFunctionNode(
+                node.pos,
+                def_node=node,
+                pymethdef_cname=node.entry.pymethdef_cname,
+                binding=self.current_directives.get('binding'),
+                specialized_cpdefs=node.specialized_cpdefs,
+                code_object=ExprNodes.CodeObjectNode(node))
+
+        if env.is_py_class_scope:
+            rhs.binding = True
+
+        node.is_cyfunction = rhs.binding
+
+        if node.decorators:
+            for decorator in node.decorators[::-1]:
+                rhs = ExprNodes.SimpleCallNode(
+                    decorator.pos,
+                    function = decorator.decorator,
+                    args = [rhs])
+
+        assmt = Nodes.SingleAssignmentNode(
+            node.pos,
+            lhs=ExprNodes.NameNode(node.pos,name=node.name),
+            rhs=rhs)
+        assmt.analyse_declarations(env)
+        return assmt
+
     def visit_ScopedExprNode(self, node):
         env = self.env_stack[-1]
         node.analyse_declarations(env)
@@ -1645,11 +1696,13 @@ if VALUE is not None:
         return None
 
     def visit_CnameDecoratorNode(self, node):
-        self.visitchildren(node)
-
-        if not node.node:
+        child_node = self.visit(node.node)
+        if not child_node:
             return None
-
+        if type(child_node) is list: # Assignment synthesized
+            node.child_node = child_node[0]
+            return [node] + child_node[1:]
+        node.node = child_node
         return node
 
     def create_Property(self, entry):
@@ -2065,9 +2118,6 @@ class CreateClosureClasses(CythonTransform):
         return from_closure, in_closure
 
     def create_class_from_scope(self, node, target_module_scope, inner_node=None):
-        # skip generator body
-        if node.is_generator_body:
-            return
         # move local variables into closure
         if node.is_generator:
             for entry in node.local_scope.entries.values():
@@ -2160,6 +2210,10 @@ class CreateClosureClasses(CythonTransform):
             self.path.pop()
         return node
 
+    def visit_GeneratorBodyDefNode(self, node):
+        self.visitchildren(node)
+        return node
+
     def visit_CFuncDefNode(self, node):
         self.visitchildren(node)
         return node

Cython/Compiler/Pipeline.py

@@ -137,6 +137,7 @@ def create_pipeline(context, mode, exclude_classes=()):
     from AutoDocTransforms import EmbedSignature
     from Optimize import FlattenInListTransform, SwitchTransform, IterationTransform
     from Optimize import EarlyReplaceBuiltinCalls, OptimizeBuiltinCalls
+    from Optimize import InlineDefNodeCalls
     from Optimize import ConstantFolding, FinalOptimizePhase
     from Optimize import DropRefcountingTransform
     from Buffer import IntroduceBufferAuxiliaryVars
@@ -185,6 +186,7 @@ def create_pipeline(context, mode, exclude_classes=()):
         MarkOverflowingArithmetic(context),
         IntroduceBufferAuxiliaryVars(context),
         _check_c_declarations,
+        InlineDefNodeCalls(context),
         AnalyseExpressionsTransform(context),
         FindInvalidUseOfFusedTypes(context),
         CreateClosureClasses(context),  ## After all lookups and type inference

Cython/Compiler/Symtab.py

@@ -156,6 +156,7 @@ class Entry(object):
     from_closure = 0
     is_declared_generic = 0
     is_readonly = 0
+    pyfunc_cname = None
     func_cname = None
     func_modifiers = []
     final_func_cname = None

tests/run/closure_inlining.pyx

+# cython: optimize.inline_defnode_calls=True
+# mode: run
+cimport cython
+
+@cython.test_fail_if_path_exists('//SimpleCallNode')
+@cython.test_assert_path_exists('//InlinedDefNodeCallNode')
+def simple_noargs():
+    """
+    >>> simple_noargs()
+    123
+    """
+    def inner():
+        return 123
+    return inner()
+
+
+@cython.test_fail_if_path_exists('//SimpleCallNode')
+@cython.test_assert_path_exists('//InlinedDefNodeCallNode')
+def test_coerce(a, int b):
+    """
+    >>> test_coerce(2, 2)
+    4
+    """
+    def inner(int a, b):
+        return a * b
+    return inner(a, b)
+
+
+cdef class Foo(object):
+    def __repr__(self):
+        return '<Foo>'
+
+
+@cython.test_fail_if_path_exists('//SimpleCallNode')
+@cython.test_assert_path_exists('//InlinedDefNodeCallNode')
+def test_func_signature(a):
+    """
+    >>> test_func_signature(Foo())
+    <Foo>
+    """
+
+    def inner(Foo a):
+        return a
+    return inner(a)
+
+@cython.test_fail_if_path_exists('//SimpleCallNode')
+@cython.test_assert_path_exists('//InlinedDefNodeCallNode')
+def test_func_signature2(a, b):
+    """
+    >>> test_func_signature2(Foo(), 123)
+    (<Foo>, 123)
+    """
+
+    def inner(Foo a, b):
+        return a, b
+    return inner(a, b)
+
+# Starred args and default values are not yet supported for inlining
+@cython.test_assert_path_exists('//SimpleCallNode')
+def test_defaults(a, b):
+    """
+    >>> test_defaults(1, 2)
+    (1, 2, 123)
+    """
+    def inner(a, b=b, c=123):
+        return a, b, c
+    return inner(a)
+
+@cython.test_assert_path_exists('//SimpleCallNode')
+def test_starred(a):
+    """
+    >>> test_starred(123)
+    (123, (), {})
+    """
+    def inner(a, *args, **kwargs):
+        return a, args, kwargs
+    return inner(a)

tests/run/type_inference.pyx

@@ -406,10 +406,10 @@ cdef object some_float_value():
     return 2.0
 
 
-@cython.test_fail_if_path_exists('//NameNode[@type.is_pyobject = True]')
-@cython.test_assert_path_exists('//NameNode[@type.is_pyobject]',
-                                '//NameNode[@type.is_pyobject = False]')
 @infer_types(None)
+@cython.test_fail_if_path_exists('//DefNode//NameNode[@type.is_pyobject = True]')
+@cython.test_assert_path_exists('//DefNode//NameNode[@type.is_pyobject]',
+                                '//DefNode//NameNode[@type.is_pyobject = False]')
 def double_loop():
     """
     >>> double_loop() == 1.0 * 10