diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py index 964a13e8dad5a18f565c8e814718ce880be0fbcd..e8d96ce7266046ffdfbed477a6dfbac9d4ab4e47 100644 --- a/Cython/Compiler/Nodes.py +++ b/Cython/Compiler/Nodes.py @@ -6559,13 +6559,9 @@ class IfStatNode(StatNode): code.mark_pos(self.pos) end_label = code.new_label() last = len(self.if_clauses) - if self.else_clause: - # If the 'else' clause is 'unlikely', then set the preceding 'if' clause to 'likely' to reflect that. - self._set_branch_hint(self.if_clauses[-1], self.else_clause, inverse=True) - else: + if not self.else_clause: last -= 1 # avoid redundant goto at end of last if-clause for i, if_clause in enumerate(self.if_clauses): - self._set_branch_hint(if_clause, if_clause.body) if_clause.generate_execution_code(code, end_label, is_last=i == last) if self.else_clause: code.mark_pos(self.else_clause.pos) @@ -6574,25 +6570,6 @@ class IfStatNode(StatNode): code.putln("}") code.put_label(end_label) - def _set_branch_hint(self, clause, statements_node, inverse=False): - if not statements_node.is_terminator: - return - if isinstance(statements_node, StatListNode): - if not statements_node.stats: - return - statements = statements_node.stats - else: - statements = [statements_node] - # Anything that unconditionally raises exceptions should be considered unlikely. - if isinstance(statements[-1], (RaiseStatNode, ReraiseStatNode)): - if len(statements) > 1: - # Allow simple statements before the 'raise', but no conditions, loops, etc. - non_branch_nodes = (ExprStatNode, AssignmentNode, DelStatNode, GlobalNode, NonlocalNode) - for node in statements[:-1]: - if not isinstance(node, non_branch_nodes): - return - clause.branch_hint = 'likely' if inverse else 'unlikely' - def generate_function_definitions(self, env, code): for clause in self.if_clauses: clause.generate_function_definitions(env, code) diff --git a/Cython/Compiler/Optimize.py b/Cython/Compiler/Optimize.py index d836409f692b2c29793b01836249ef6b2b12c576..da28e25dbd55966b4ba280a427edfa4d29f99d50 100644 --- a/Cython/Compiler/Optimize.py +++ b/Cython/Compiler/Optimize.py @@ -4779,6 +4779,7 @@ class FinalOptimizePhase(Visitor.EnvTransform, Visitor.NodeRefCleanupMixin): - isinstance -> typecheck for cdef types - eliminate checks for None and/or types that became redundant after tree changes - eliminate useless string formatting steps + - inject branch hints for unlikely if-cases that only raise exceptions - replace Python function calls that look like method calls by a faster PyMethodCallNode """ in_loop = False @@ -4877,6 +4878,45 @@ class FinalOptimizePhase(Visitor.EnvTransform, Visitor.NodeRefCleanupMixin): self.in_loop = old_val return node + def visit_IfStatNode(self, node): + """Assign 'unlikely' branch hints to if-clauses that only raise exceptions. + """ + self.visitchildren(node) + last_non_unlikely_clause = None + for i, if_clause in enumerate(node.if_clauses): + self._set_ifclause_branch_hint(if_clause, if_clause.body) + if not if_clause.branch_hint: + last_non_unlikely_clause = if_clause + if node.else_clause and last_non_unlikely_clause: + # If the 'else' clause is 'unlikely', then set the preceding 'if' clause to 'likely' to reflect that. + self._set_ifclause_branch_hint(last_non_unlikely_clause, node.else_clause, inverse=True) + return node + + def _set_ifclause_branch_hint(self, clause, statements_node, inverse=False): + if not statements_node.is_terminator: + return + if isinstance(statements_node, Nodes.StatListNode): + if not statements_node.stats: + return + statements = statements_node.stats + else: + statements = [statements_node] + # Anything that unconditionally raises exceptions should be considered unlikely. + if isinstance(statements[-1], (Nodes.RaiseStatNode, Nodes.ReraiseStatNode)): + if len(statements) > 1: + # Allow simple statements before the 'raise', but no conditions, loops, etc. + non_branch_nodes = ( + Nodes.ExprStatNode, + Nodes.AssignmentNode, + Nodes.DelStatNode, + Nodes.GlobalNode, + Nodes.NonlocalNode, + ) + for node in statements[:-1]: + if not isinstance(node, non_branch_nodes): + return + clause.branch_hint = 'likely' if inverse else 'unlikely' + class ConsolidateOverflowCheck(Visitor.CythonTransform): """ diff --git a/tests/compile/branch_hints.pyx b/tests/compile/branch_hints.pyx new file mode 100644 index 0000000000000000000000000000000000000000..d7d86d3cf30085995f258f97963490897cf42400 --- /dev/null +++ b/tests/compile/branch_hints.pyx @@ -0,0 +1,45 @@ +# mode: compile +# tag: if, unlikely + +cimport cython + + +@cython.test_assert_path_exists( + "//IfClauseNode", + "//IfClauseNode[not(@branch_hint)]", +) +def if_simple(x): + if x: + x = 2 + + +@cython.test_assert_path_exists( + "//IfClauseNode", + "//IfClauseNode[not(@branch_hint)]", +) +def if_return(x): + if x: + return 1 + raise TypeError() + + +@cython.test_assert_path_exists( + "//IfClauseNode", + "//IfClauseNode[@branch_hint = 'unlikely']", +) +def if_raise_else(x): + if x: + raise TypeError() + else: + return 1 + + +@cython.test_assert_path_exists( + "//IfClauseNode", + "//IfClauseNode[@branch_hint = 'likely']", +) +def if_else_raise(x): + if x: + return 1 + else: + raise TypeError()