Skip to content

Commit 5ceeb6a

Browse files
jbower-fbfacebook-github-bot
jbower-fb
authored and
facebook-github-bot
committed
Support TO_BOOL and related optimizations
Summary: Implements what we need from [GH-106008](python/cpython#106003). Technically the included updates to `UNARY_NOT`, and `POP_JUMP_IF_FALSE|TRUE` are optimizations and not strictly needed. However, it really makes sense to to them. Reviewed By: DinoV Differential Revision: D80682012 fbshipit-source-id: 6150d52dddd28ef5c2e401ed17df7c7cad09074b
1 parent 7cd8c60 commit 5ceeb6a

4 files changed

Lines changed: 132 additions & 11 deletions

File tree

‎cinderx/Common/opcode_stubs.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
X(STORE_SLICE) \
8787
X(STORE_SUBSCR_DICT) \
8888
X(SWAP) \
89+
X(TO_BOOL) \
8990
X(UNPACK_SEQUENCE_LIST) \
9091
X(UNPACK_SEQUENCE_TUPLE) \
9192
X(UNPACK_SEQUENCE_TWO_TUPLE)
@@ -195,6 +196,7 @@ enum {
195196
X(STORE_ATTR_DICT) \
196197
X(STORE_ATTR_SPLIT_DICT) \
197198
X(STORE_ATTR_UNCACHABLE) \
199+
X(TO_BOOL) \
198200
X(UNARY_POSITIVE) \
199201
X(YIELD_FROM)
200202

‎cinderx/Jit/hir/builder.cpp‎

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ bool isSupportedOpcode(int opcode) {
215215
case STORE_SLICE:
216216
case STORE_SUBSCR:
217217
case SWAP:
218+
case TO_BOOL:
218219
case TP_ALLOC:
219220
case UNARY_INVERT:
220221
case UNARY_NEGATIVE:
@@ -876,6 +877,10 @@ void HIRBuilder::translate(
876877
break;
877878
}
878879
case UNARY_NOT:
880+
#if PY_VERSION_HEX >= 0x030E0000
881+
emitUnaryNot(tc);
882+
break;
883+
#endif
879884
case UNARY_NEGATIVE:
880885
case UNARY_POSITIVE:
881886
case UNARY_INVERT: {
@@ -958,6 +963,10 @@ void HIRBuilder::translate(
958963
emitCompareOp(tc, bc_instr);
959964
break;
960965
}
966+
case TO_BOOL: {
967+
emitToBool(tc);
968+
break;
969+
}
961970
case COPY_DICT_WITHOUT_KEYS: {
962971
emitCopyDictWithoutKeys(tc);
963972
break;
@@ -2106,6 +2115,34 @@ static inline UnaryOpKind get_unary_op_kind(
21062115
}
21072116
}
21082117

2118+
void HIRBuilder::emitUnaryNot(TranslationContext& tc) {
2119+
Register* operand = tc.frame.stack.pop();
2120+
Register* is_false = temps_.AllocateNonStack();
2121+
Register* const_false = temps_.AllocateNonStack();
2122+
tc.emit<LoadConst>(const_false, Type::fromObject(Py_False));
2123+
tc.emit<PrimitiveCompare>(
2124+
is_false, PrimitiveCompareOp::kEqual, const_false, operand);
2125+
2126+
BasicBlock* is_true_block = tc.block->cfg->AllocateBlock();
2127+
BasicBlock* is_false_block = tc.block->cfg->AllocateBlock();
2128+
BasicBlock* done_block = tc.block->cfg->AllocateBlock();
2129+
2130+
tc.emit<CondBranch>(is_false, is_false_block, is_true_block);
2131+
2132+
Register* result = temps_.AllocateStack();
2133+
2134+
tc.block = is_true_block;
2135+
tc.emit<LoadConst>(result, Type::fromObject(Py_False));
2136+
tc.emit<Branch>(done_block);
2137+
2138+
tc.block = is_false_block;
2139+
tc.emit<LoadConst>(result, Type::fromObject(Py_True));
2140+
tc.emit<Branch>(done_block);
2141+
2142+
tc.block = done_block;
2143+
tc.frame.stack.push(result);
2144+
}
2145+
21092146
void HIRBuilder::emitUnaryOp(
21102147
TranslationContext& tc,
21112148
const jit::BytecodeInstruction& bc_instr) {
@@ -2611,6 +2648,34 @@ void HIRBuilder::emitCompareOp(
26112648

26122649
tc.emit<Compare>(result, op, left, right, tc.frame);
26132650
stack.push(result);
2651+
if (PY_VERSION_HEX >= 0x030E0000 && bc_instr.oparg() & 0x10) {
2652+
emitToBool(tc);
2653+
}
2654+
}
2655+
2656+
void HIRBuilder::emitToBool(TranslationContext& tc) {
2657+
Register* operand = tc.frame.stack.pop();
2658+
Register* truthy_result = temps_.AllocateStack();
2659+
tc.emit<IsTruthy>(truthy_result, operand, tc.frame);
2660+
2661+
BasicBlock* true_block = tc.block->cfg->AllocateBlock();
2662+
BasicBlock* false_block = tc.block->cfg->AllocateBlock();
2663+
BasicBlock* done_block = tc.block->cfg->AllocateBlock();
2664+
2665+
tc.emit<CondBranch>(truthy_result, true_block, false_block);
2666+
2667+
Register* coerced_result = temps_.AllocateStack();
2668+
2669+
tc.block = true_block;
2670+
tc.emit<LoadConst>(coerced_result, Type::fromObject(Py_True));
2671+
tc.emit<Branch>(done_block);
2672+
2673+
tc.block = false_block;
2674+
tc.emit<LoadConst>(coerced_result, Type::fromObject(Py_False));
2675+
tc.emit<Branch>(done_block);
2676+
2677+
tc.block = done_block;
2678+
tc.frame.stack.push(coerced_result);
26142679
}
26152680

26162681
void HIRBuilder::emitCopyDictWithoutKeys(TranslationContext& tc) {
@@ -3688,9 +3753,18 @@ void HIRBuilder::emitPopJumpIf(
36883753

36893754
if (bc_instr.opcode() == POP_JUMP_IF_FALSE ||
36903755
bc_instr.opcode() == POP_JUMP_IF_TRUE) {
3691-
Register* tval = temps_.AllocateNonStack();
3692-
tc.emit<IsTruthy>(tval, var, tc.frame);
3693-
tc.emit<CondBranch>(tval, true_block, false_block);
3756+
Register* is_true = temps_.AllocateNonStack();
3757+
// In 3.14+ coercion to exactly Py_True or Py_False is performed by earlier
3758+
// instructions. See GH-106008.
3759+
if constexpr (PY_VERSION_HEX >= 0x030E0000) {
3760+
Register* const_true = temps_.AllocateNonStack();
3761+
tc.emit<LoadConst>(const_true, Type::fromObject(Py_True));
3762+
tc.emit<PrimitiveCompare>(
3763+
is_true, PrimitiveCompareOp::kEqual, var, const_true);
3764+
} else {
3765+
tc.emit<IsTruthy>(is_true, var, tc.frame);
3766+
}
3767+
tc.emit<CondBranch>(is_true, true_block, false_block);
36943768
} else {
36953769
tc.emit<CondBranch>(var, true_block, false_block);
36963770
}

‎cinderx/Jit/hir/builder.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ class HIRBuilder {
135135
void emitBinaryOp(
136136
TranslationContext& tc,
137137
const jit::BytecodeInstruction& bc_instr);
138+
void emitUnaryNot(TranslationContext& tc);
138139
void emitUnaryOp(
139140
TranslationContext& tc,
140141
const jit::BytecodeInstruction& bc_instr);
@@ -160,6 +161,7 @@ class HIRBuilder {
160161
void emitCompareOp(
161162
TranslationContext& tc,
162163
const jit::BytecodeInstruction& bc_instr);
164+
void emitToBool(TranslationContext& tc);
163165
void emitCopyDictWithoutKeys(TranslationContext& tc);
164166
void emitGetLen(TranslationContext& tc);
165167
void emitJumpIf(

‎cinderx/PythonLib/test_cinderx/test_python314_bytecodes.py‎

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,33 @@ def one():
1313

1414
@unittest.skipUnless(sys.version_info >= (3, 14), "Python 3.14+ only")
1515
class Python314Bytecodes(unittest.TestCase):
16-
def _assertBytecodeContains(self, func, expected_opcode):
16+
def _assertBytecodeContains(self, func, expected_opcode, expected_oparg=None):
1717
try:
1818
inner_function = func.inner_function
1919
except AttributeError:
2020
pass
2121
else:
2222
func = inner_function
23-
bytecode_instructions = list(dis.get_instructions(func))
24-
opcodes = [instr.opname for instr in bytecode_instructions]
2523

26-
self.assertIn(
27-
expected_opcode,
28-
opcodes,
29-
f"{expected_opcode} opcode should be present in {func.__name__} bytecode",
30-
)
24+
bytecode_instructions = dis.get_instructions(func)
25+
26+
if expected_oparg is None:
27+
opcodes = [instr.opname for instr in bytecode_instructions]
28+
self.assertIn(
29+
expected_opcode,
30+
opcodes,
31+
f"{expected_opcode} opcode should be present in {func.__name__} bytecode",
32+
)
33+
else:
34+
matching_instructions = [
35+
instr
36+
for instr in bytecode_instructions
37+
if instr.opname == expected_opcode and instr.arg == expected_oparg
38+
]
39+
self.assertTrue(
40+
len(matching_instructions) > 0,
41+
f"{expected_opcode} opcode with oparg {expected_oparg} should be present in {func.__name__} bytecode",
42+
)
3143

3244
def test_LOAD_SMALL_INT(self):
3345
@cinder_support.fail_if_deopt
@@ -221,6 +233,37 @@ def x():
221233
self.assertEqual(x(), True)
222234
self._assertBytecodeContains(x, "COMPARE_OP")
223235

236+
def test_COMPARE_OP_with_bool_coercion(self):
237+
@cinder_support.fail_if_deopt
238+
@cinder_support.failUnlessJITCompiled
239+
def x():
240+
if 1 == 2:
241+
return 2
242+
return 1
243+
244+
self.assertEqual(x(), 1)
245+
self._assertBytecodeContains(x, "COMPARE_OP", 88) # bit 5 set
246+
247+
def test_COMPARE_OP_without_bool_coercion(self):
248+
@cinder_support.fail_if_deopt
249+
@cinder_support.failUnlessJITCompiled
250+
def x():
251+
return 1 == 2
252+
253+
self.assertEqual(x(), False)
254+
self._assertBytecodeContains(x, "COMPARE_OP", 72) # bit 5 not set
255+
256+
def test_TO_BOOL_and_POP_JUMP_IF_FALSE(self):
257+
@cinder_support.fail_if_deopt
258+
@cinder_support.failUnlessJITCompiled
259+
def x(a):
260+
if a:
261+
return 1
262+
263+
self.assertEqual(x(2), 1)
264+
self._assertBytecodeContains(x, "TO_BOOL")
265+
self._assertBytecodeContains(x, "POP_JUMP_IF_FALSE")
266+
224267

225268
if __name__ == "__main__":
226269
unittest.main()

0 commit comments

Comments
 (0)