Skip to content

Commit 41a7934

Browse files
authored
[mypyc] Fix invalid unlikely() in certain rare branches (#11939)
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.
1 parent f6ebf10 commit 41a7934

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

mypyc/codegen/emitfunc.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,12 @@ def visit_goto(self, op: Goto) -> None:
122122
def visit_branch(self, op: Branch) -> None:
123123
true, false = op.true, op.false
124124
negated = op.negated
125+
negated_rare = False
125126
if true is self.next_block and op.traceback_entry is None:
126127
# Switch true/false since it avoids an else block.
127128
true, false = false, true
128129
negated = not negated
130+
negated_rare = True
129131

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

151153
# For error checks, tell the compiler the branch is unlikely
152154
if op.traceback_entry is not None or op.rare:
153-
cond = 'unlikely({})'.format(cond)
155+
if not negated_rare:
156+
cond = 'unlikely({})'.format(cond)
157+
else:
158+
cond = 'likely({})'.format(cond)
154159

155160
if false is self.next_block:
156161
if op.traceback_entry is None:

mypyc/test/test_emitfunc.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,29 @@ def test_branch_is_error_next_block(self) -> None:
187187
"""if (cpy_r_b == 2) goto CPyL9;""",
188188
next_block=next_block)
189189

190+
def test_branch_rare(self) -> None:
191+
self.assert_emit(Branch(self.b, BasicBlock(8), BasicBlock(9), Branch.BOOL, rare=True),
192+
"""if (unlikely(cpy_r_b)) {
193+
goto CPyL8;
194+
} else
195+
goto CPyL9;
196+
""")
197+
next_block = BasicBlock(9)
198+
self.assert_emit(Branch(self.b, BasicBlock(8), next_block, Branch.BOOL, rare=True),
199+
"""if (unlikely(cpy_r_b)) goto CPyL8;""",
200+
next_block=next_block)
201+
next_block = BasicBlock(8)
202+
b = Branch(self.b, next_block, BasicBlock(9), Branch.BOOL, rare=True)
203+
self.assert_emit(b,
204+
"""if (likely(!cpy_r_b)) goto CPyL9;""",
205+
next_block=next_block)
206+
next_block = BasicBlock(8)
207+
b = Branch(self.b, next_block, BasicBlock(9), Branch.BOOL, rare=True)
208+
b.negated = True
209+
self.assert_emit(b,
210+
"""if (likely(cpy_r_b)) goto CPyL9;""",
211+
next_block=next_block)
212+
190213
def test_call(self) -> None:
191214
decl = FuncDecl('myfn', None, 'mod',
192215
FuncSignature([RuntimeArg('m', int_rprimitive)], int_rprimitive))

0 commit comments

Comments
 (0)