Skip to content

Commit

Permalink
[mypyc] Fix invalid unlikely() in certain rare branches (#11939)
Browse files Browse the repository at this point in the history
If we switch true/false branches, we also need to switch between
likely/unlikely.

This has an impact at least on accessing module-level final attributes
with non-constant initializers. I couldn't see a significant
difference in benchmark results compared to master, but this seems to 
help together with some other work-in-progress improvements I have 
been working on.
  • Loading branch information
JukkaL authored Jan 10, 2022
1 parent f6ebf10 commit 41a7934
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
7 changes: 6 additions & 1 deletion mypyc/codegen/emitfunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,12 @@ def visit_goto(self, op: Goto) -> None:
def visit_branch(self, op: Branch) -> None:
true, false = op.true, op.false
negated = op.negated
negated_rare = False
if true is self.next_block and op.traceback_entry is None:
# Switch true/false since it avoids an else block.
true, false = false, true
negated = not negated
negated_rare = True

neg = '!' if negated else ''
cond = ''
Expand All @@ -150,7 +152,10 @@ def visit_branch(self, op: Branch) -> None:

# For error checks, tell the compiler the branch is unlikely
if op.traceback_entry is not None or op.rare:
cond = 'unlikely({})'.format(cond)
if not negated_rare:
cond = 'unlikely({})'.format(cond)
else:
cond = 'likely({})'.format(cond)

if false is self.next_block:
if op.traceback_entry is None:
Expand Down
23 changes: 23 additions & 0 deletions mypyc/test/test_emitfunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,29 @@ def test_branch_is_error_next_block(self) -> None:
"""if (cpy_r_b == 2) goto CPyL9;""",
next_block=next_block)

def test_branch_rare(self) -> None:
self.assert_emit(Branch(self.b, BasicBlock(8), BasicBlock(9), Branch.BOOL, rare=True),
"""if (unlikely(cpy_r_b)) {
goto CPyL8;
} else
goto CPyL9;
""")
next_block = BasicBlock(9)
self.assert_emit(Branch(self.b, BasicBlock(8), next_block, Branch.BOOL, rare=True),
"""if (unlikely(cpy_r_b)) goto CPyL8;""",
next_block=next_block)
next_block = BasicBlock(8)
b = Branch(self.b, next_block, BasicBlock(9), Branch.BOOL, rare=True)
self.assert_emit(b,
"""if (likely(!cpy_r_b)) goto CPyL9;""",
next_block=next_block)
next_block = BasicBlock(8)
b = Branch(self.b, next_block, BasicBlock(9), Branch.BOOL, rare=True)
b.negated = True
self.assert_emit(b,
"""if (likely(cpy_r_b)) goto CPyL9;""",
next_block=next_block)

def test_call(self) -> None:
decl = FuncDecl('myfn', None, 'mod',
FuncSignature([RuntimeArg('m', int_rprimitive)], int_rprimitive))
Expand Down

0 comments on commit 41a7934

Please sign in to comment.