Address PR comments

Author: savannahostrowski

Lib/test/test_capi/test_opt.py

@@ -1665,38 +1665,6 @@ def testfunc(n):
         self.assertNotIn("_UNARY_INVERT", uops)
         self.assertNotIn("_POP_TOP_LOAD_CONST_INLINE_BORROW", uops)
 
-    def test_replace_with_true_pop_top_load_const_inline_borrow(self):
-        def testfunc(n):
-            x = 0
-            for _ in range(n):
-                a = 42
-                result = bool(a)
-                if result:
-                    x += 1
-            return x
-        res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
-        self.assertEqual(res, TIER2_THRESHOLD)
-        self.assertIsNotNone(ex)
-        uops = get_opnames(ex)
-        self.assertNotIn("_REPLACE_WITH_TRUE", uops)
-        self.assertNotIn("_POP_TOP_LOAD_CONST_INLINE_BORROW", uops)
-
-    def test_to_bool_none_pop_top_load_const_inline_borrow(self):
-        def testfunc(n):
-            x = 0
-            for _ in range(n):
-                a = None
-                result = bool(a)
-                if result:
-                    x += 1
-            return x
-        res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
-        self.assertEqual(res, 0)
-        self.assertIsNotNone(ex)
-        uops = get_opnames(ex)
-        self.assertNotIn("_TO_BOOL_NONE", uops)
-        self.assertNotIn("_POP_TOP_LOAD_CONST_INLINE_BORROW", uops)
-
     def test_compare_op_pop_two_load_const_inline_borrow(self):
         def testfunc(n):
             x = 0

Misc/NEWS.d/next/Core_and_Builtins/2025-08-09-04-07-05.gh-issue-132732.8BiIVJ.rst

@@ -1 +1 @@
-Optimize ``_COMPARE_OP``, ``_CONTAINS_OP``, ``_GET_LEN``, ``_REPLACE_WITH_TRUE``, ``_TO_BOOL_NONE``, `_UNARY_NEGATIVE``, ``_UNARY_NOT``, and ``_UNARY_INVERT`` in JIT builds with constant-loading uops (``_POP_TWO_LOAD_CONST_INLINE_BORROW`` and ``_POP_TOP_LOAD_CONST_INLINE_BORROW``), and then remove both to reduce instruction count.
+Optimize ``_COMPARE_OP``, ``_CONTAINS_OP``, ``_UNARY_NEGATIVE``, ``_UNARY_NOT``, and ``_UNARY_INVERT`` in JIT builds with constant-loading uops (``_POP_TWO_LOAD_CONST_INLINE_BORROW`` and ``_POP_TOP_LOAD_CONST_INLINE_BORROW``), and then remove both to reduce instruction count.

Python/optimizer_bytecodes.c

@@ -362,7 +362,6 @@ dummy_func(void) {
     }
 
     op(_TO_BOOL_NONE, (value -- res)) {
-        REPLACE_OPCODE_IF_EVALUATES_PURE(value);
         int already_bool = optimize_to_bool(this_instr, ctx, value, &res);
         if (!already_bool) {
             sym_set_const(value, Py_None);
@@ -424,8 +423,8 @@ dummy_func(void) {
     }
 
     op(_COMPARE_OP, (left, right -- res)) {
+        REPLACE_OPCODE_IF_EVALUATES_PURE(left, right);
         if (oparg & 16) {
-            REPLACE_OPCODE_IF_EVALUATES_PURE(left, right);
             res = sym_new_type(ctx, &PyBool_Type);
         }
         else {
@@ -1028,7 +1027,6 @@ dummy_func(void) {
     }
 
     op(_REPLACE_WITH_TRUE, (value -- res)) {
-        REPLACE_OPCODE_IF_EVALUATES_PURE(value);
         res = sym_new_const(ctx, Py_True);
     }
 

Python/optimizer_cases.c.h

@@ -232,29 +232,28 @@ def replace_opcode_if_evaluates_pure(
                 emitter.emit(f"{outp.name} = sym_new_const_steal(ctx, PyStackRef_AsPyObjectSteal({outp.name}_stackref));\n")
             else:
                 emitter.emit(f"{outp.name} = sym_new_const(ctx, PyStackRef_AsPyObjectBorrow({outp.name}_stackref));\n")
-
         if len(self.original_uop.stack.outputs) == 1:
             outp = self.original_uop.stack.outputs[0]
             if not outp.peek:
                 if self.original_uop.name.startswith('_'):
-                    if len(used_stack_inputs) == 1:
-                        emitter.emit(f"""
-                        if (sym_is_const(ctx, {outp.name})) {{
-                            PyObject *result = sym_get_const(ctx, {outp.name});
-                            if (_Py_IsImmortal(result)) {{
-                                // Replace with _POP_TOP_LOAD_CONST_INLINE_BORROW since we have one input and an immortal result
-                                REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)result);
-                            }}
-                        }}""")
-                    elif len(used_stack_inputs) == 2:
+                    # Map input count to the appropriate constant-loading uop
+                    input_count_to_uop = {
+                        1: "_POP_TOP_LOAD_CONST_INLINE_BORROW",
+                        2: "_POP_TWO_LOAD_CONST_INLINE_BORROW"
+                    }
+
+                    input_count = len(used_stack_inputs)
+                    if input_count in input_count_to_uop:
+                        replacement_uop = input_count_to_uop[input_count]
+                        input_desc = "one input" if input_count == 1 else "two inputs"
                         emitter.emit(f"""
-                        if (sym_is_const(ctx, {outp.name})) {{
-                            PyObject *result = sym_get_const(ctx, {outp.name});
-                            if (_Py_IsImmortal(result)) {{
-                                // Replace with _POP_TWO_LOAD_CONST_INLINE_BORROW since we have two inputs and an immortal result
-                                REPLACE_OP(this_instr, _POP_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)result);
-                            }}
-                        }}""")
+                                     if (sym_is_const(ctx, {outp.name})) {{
+                                        PyObject *result = sym_get_const(ctx, {outp.name});
+                                        if (_Py_IsImmortal(result)) {{
+                                            // Replace with {replacement_uop} since we have {input_desc} and an immortal result
+                                            REPLACE_OP(this_instr, {replacement_uop}, 0, (uintptr_t)result);
+                                        }}
+                                    }}""")
 
         storage.flush(self.out)
         emitter.emit("break;\n")