fix a bunch of bugs in the abstract interp

python · gvanrossum · Feb 20, 2024 · Feb 20, 2024 · Feb 20, 2024 · Feb 21, 2024
commit e1ee2ad076541e1e983a4c056fc93d3a7c5cf568
diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h
@@ -270,16 +270,19 @@ void _PyThreadState_PopFrame(PyThreadState *tstate, _PyInterpreterFrame *frame);
  * The frame that is being expanded MUST be the current executing frame, and
  * it must be at the top of the datastack.
  * */
-static inline void
+static inline int
 _PyFrame_GrowLocalsPlus(PyThreadState *tstate, _PyInterpreterFrame *frame, int size)
 {
     assert(_PyThreadState_HasStackSpace(tstate, size));
     assert(tstate->current_frame == frame);
     // Make sure we are the top frame.
-    assert((PyObject **)frame + _PyFrame_GetCode(frame)->co_framesize ==
-           tstate->datastack_top);
+    if ((PyObject **)frame + _PyFrame_GetCode(frame)->co_framesize !=
+           tstate->datastack_top) {
+        return 0;
+    }
     tstate->datastack_top += size;
     assert(tstate->datastack_top < tstate->datastack_limit);
+    return 1;
 }
 
 
@@ -300,7 +303,9 @@ _PyFrame_ConvertToTier2(PyThreadState *tstate, _PyInterpreterFrame *frame,
     if (!_PyThreadState_HasStackSpace(tstate, localsplus_grow)) {
         return 1;
     }
-    _PyFrame_GrowLocalsPlus(tstate, frame, localsplus_grow);
+    if (!_PyFrame_GrowLocalsPlus(tstate, frame, localsplus_grow)) {
+        return 1;
+    }
     frame->tier2_extra_size += localsplus_grow;
     return 0;
 }

diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py
@@ -891,7 +891,7 @@ def test_function_inlining(self):
         def testfunc(n):
             a = 1
             for _ in range(n):
-                x = foo(a, a)
+                x = foo(a, 2)
             return x
 
         res, ex = self._run_with_optimizer(testfunc, 32)
@@ -901,10 +901,29 @@ def testfunc(n):
         self.assertLessEqual(len(guard_both_float_count), 1)
         self.assertIn("_COMPARE_OP_STR", uops)
 
+    def test_method_inlining(self):
+        thing = Bar()
+        def testfunc(n):
+            a = 1
+            for _ in range(n):
+                x = thing.foo(a, a)
+            return x
+
+        res, ex = self._run_with_optimizer(testfunc, 32)
+        self.assertTrue(res)
+        self.assertIsNotNone(ex)
+        uops = get_opnames(ex)
+        self.assertLessEqual(len(guard_both_float_count), 1)
+        self.assertIn("_COMPARE_OP_STR", uops)
 
 def foo(x, y):
+    print(x)
     return x + y
 
+class Bar:
+    def foo(self, x, y):
+        self
+        return x + y
 
 if __name__ == "__main__":
     unittest.main()
@@ -1107,6 +1107,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
 
 // Jump here from EXIT_IF()
 side_exit:
+    frame = _PyEvalFrame_ReconstructTier2Frame(tstate, frame, &stack_pointer);
+    // Unrecoverable memory error.
+    if (frame == NULL) {
+        goto error_tier_two;
+    }
     OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
     UOP_STAT_INC(uopcode, miss);
     uint32_t exit_index = next_uop[-1].exit_index;

diff --git a/Python/optimizer.c b/Python/optimizer.c
@@ -711,7 +711,9 @@ translate_bytecode_to_trace(
                                 Py_FatalError("garbled expansion");
                         }
                         // Temp buffer for _POP_FRAME optimizations (if needed)
-                        ADD_TO_TRACE(_NOP, 0, 0, 0);
+                        if (uop == _POP_FRAME) {
+                            ADD_TO_TRACE(_NOP, 0, 0, 0);
+                        }
                         ADD_TO_TRACE(uop, oparg, operand, target);
                         if (uop == _POP_FRAME) {
                             TRACE_STACK_POP();

@@ -327,6 +327,7 @@ sym_new_known_notnull(_Py_UOpsAbstractInterpContext *ctx)
     if (res == NULL) {
         return NULL;
     }
+    sym_set_flag(res, KNOWN);
     sym_set_flag(res, NOT_NULL);
     return res;
 }
@@ -621,6 +622,12 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
     OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); \
     } while (0);
 
+#define _LOAD_ATTR_NOT_NULL_SELF \
+    do {                    \
+    OUT_OF_SPACE_IF_NULL(attr = sym_new_known_notnull(ctx)); \
+    OUT_OF_SPACE_IF_NULL(self = sym_new_known_notnull(ctx)); \
+    } while (0);
+
 int
 real_localsplus_idx(_Py_UOpsAbstractInterpContext *ctx, int oparg)
 {

diff --git a/Python/tier2_redundancy_eliminator_bytecodes.c b/Python/tier2_redundancy_eliminator_bytecodes.c
@@ -265,6 +265,29 @@ dummy_func(void) {
         (void)owner;
     }
 
+    op(_LOAD_ATTR_METHOD_WITH_VALUES, (descr/4, owner -- attr, self if (oparg & 1))) {
+        _LOAD_ATTR_NOT_NULL_SELF
+        (void)descr;
+        (void)owner;
+    }
+
+    op(_LOAD_ATTR_METHOD_NO_DICT, (descr/4, owner -- attr, self if (oparg & 1))) {
+        _LOAD_ATTR_NOT_NULL_SELF
+        (void)descr;
+        (void)owner;
+    }
+
+    op(_LOAD_ATTR_METHOD_LAZY_DICT, (descr/4, owner -- attr, self if (oparg & 1))) {
+        _LOAD_ATTR_NOT_NULL_SELF
+        (void)descr;
+        (void)owner;
+    }
+
+    op(_INIT_CALL_BOUND_METHOD_EXACT_ARGS, (callable, unused, unused[oparg] -- attr, self, unused[oparg])) {
+        _LOAD_ATTR_NOT_NULL_SELF
+        (void)callable;
+    }
+
     op(_CHECK_FUNCTION_EXACT_ARGS, (func_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) {
         sym_set_type(callable, &PyFunction_Type);
         (void)self_or_null;
@@ -290,6 +313,7 @@ dummy_func(void) {
         assert(self_or_null != NULL);
         assert(args != NULL);
         if (sym_is_not_null(self_or_null)) {
+            DPRINTF(2, "BOUND METHOD FIDDLING\n");
             // Bound method fiddling, same as _INIT_CALL_PY_EXACT_ARGS in VM
             args--;
             argcount++;
@@ -301,6 +325,7 @@ dummy_func(void) {
         // and make the current stack the new locals.
         // This also sets up for true call inlining.
         if (sym_is_known(self_or_null)) {
+            DPRINTF(2, "I KNOW YOU %d, %d\n", args - ctx->frame->locals, ctx->frame->locals_len);
             localsplus_start = args;
             n_locals_already_filled = argcount;
         }
@@ -313,11 +338,9 @@ dummy_func(void) {
         if (ctx->frame->is_inlined) {
             PyFunctionObject *func = ctx_prev_frame(ctx)->func;
             PyCodeObject *co = (PyCodeObject *)ctx_prev_frame(ctx)->func->func_code;
-            assert((this_instr - 1)->opcode == _SET_IP ||
-                (this_instr - 1)->opcode == _CHECK_VALIDITY_AND_SET_IP ||
-               (this_instr - 1)->opcode == _CHECK_VALIDITY);
+            assert((this_instr - 1)->opcode == _NOP);
             REPLACE_OP(this_instr, _POST_INLINE,
-                       stack_pointer - ctx_prev_frame(ctx)->stack_pointer,
+                       (stack_pointer - ctx_prev_frame(ctx)->stack_pointer),
                        ctx_prev_frame(ctx)->reconstruction_offset);
             REPLACE_OP((this_instr - 1), _SET_FRAME_NAMES, 0,
                        (uintptr_t)Py_NewRef(co->co_names));
@@ -352,6 +375,7 @@ dummy_func(void) {
         assert((this_instr - 3)->opcode == _CHECK_STACK_SPACE);
         _PyUOpInstruction *reconstruction_start = end_writebuffer;
         if (compile_frame_reconstruction(ctx, &end_writebuffer, true_end)) {
+            DPRINTF(1, "OUT OF WRITE SPACE FOR RECONSTRUCTION\n");
             goto error;
         }
         uint64_t reconstruction_offset = (uint64_t)(reconstruction_start - trace);

diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h
@@ -493,11 +493,9 @@
             if (ctx->frame->is_inlined) {
                 PyFunctionObject *func = ctx_prev_frame(ctx)->func;
                 PyCodeObject *co = (PyCodeObject *)ctx_prev_frame(ctx)->func->func_code;
-                assert((this_instr - 1)->opcode == _SET_IP ||
-                   (this_instr - 1)->opcode == _CHECK_VALIDITY_AND_SET_IP ||
-                   (this_instr - 1)->opcode == _CHECK_VALIDITY);
+                assert((this_instr - 1)->opcode == _NOP);
                 REPLACE_OP(this_instr, _POST_INLINE,
-                       stack_pointer - ctx_prev_frame(ctx)->stack_pointer,
+                       (stack_pointer - ctx_prev_frame(ctx)->stack_pointer),
                        ctx_prev_frame(ctx)->reconstruction_offset);
                 REPLACE_OP((this_instr - 1), _SET_FRAME_NAMES, 0,
                        (uintptr_t)Py_NewRef(co->co_names));
@@ -1283,28 +1281,32 @@
         }
 
         case _LOAD_ATTR_METHOD_WITH_VALUES: {
+            _Py_UOpsSymType *owner;
             _Py_UOpsSymType *attr;
             _Py_UOpsSymType *self = NULL;
-            attr = sym_new_unknown(ctx);
-            if (attr == NULL) goto out_of_space;
-            self = sym_new_unknown(ctx);
-            if (self == NULL) goto out_of_space;
+            owner = stack_pointer[-1];
+            PyObject *descr = (PyObject *)this_instr->operand;
+            _LOAD_ATTR_NOT_NULL_SELF
+            (void)descr;
+            (void)owner;
             stack_pointer[-1] = attr;
-            stack_pointer[0] = self;
-            stack_pointer += 1;
+            if (oparg & 1) stack_pointer[0] = self;
+            stack_pointer += (oparg & 1);
             break;
         }
 
         case _LOAD_ATTR_METHOD_NO_DICT: {
+            _Py_UOpsSymType *owner;
             _Py_UOpsSymType *attr;
             _Py_UOpsSymType *self = NULL;
-            attr = sym_new_unknown(ctx);
-            if (attr == NULL) goto out_of_space;
-            self = sym_new_unknown(ctx);
-            if (self == NULL) goto out_of_space;
+            owner = stack_pointer[-1];
+            PyObject *descr = (PyObject *)this_instr->operand;
+            _LOAD_ATTR_NOT_NULL_SELF
+            (void)descr;
+            (void)owner;
             stack_pointer[-1] = attr;
-            stack_pointer[0] = self;
-            stack_pointer += 1;
+            if (oparg & 1) stack_pointer[0] = self;
+            stack_pointer += (oparg & 1);
             break;
         }
 
@@ -1329,15 +1331,17 @@
         }
 
         case _LOAD_ATTR_METHOD_LAZY_DICT: {
+            _Py_UOpsSymType *owner;
             _Py_UOpsSymType *attr;
             _Py_UOpsSymType *self = NULL;
-            attr = sym_new_unknown(ctx);
-            if (attr == NULL) goto out_of_space;
-            self = sym_new_unknown(ctx);
-            if (self == NULL) goto out_of_space;
+            owner = stack_pointer[-1];
+            PyObject *descr = (PyObject *)this_instr->operand;
+            _LOAD_ATTR_NOT_NULL_SELF
+            (void)descr;
+            (void)owner;
             stack_pointer[-1] = attr;
-            stack_pointer[0] = self;
-            stack_pointer += 1;
+            if (oparg & 1) stack_pointer[0] = self;
+            stack_pointer += (oparg & 1);
             break;
         }
 
@@ -1356,13 +1360,13 @@
         }
 
         case _INIT_CALL_BOUND_METHOD_EXACT_ARGS: {
-            _Py_UOpsSymType *func;
+            _Py_UOpsSymType *callable;
+            _Py_UOpsSymType *attr;
             _Py_UOpsSymType *self;
-            func = sym_new_unknown(ctx);
-            if (func == NULL) goto out_of_space;
-            self = sym_new_unknown(ctx);
-            if (self == NULL) goto out_of_space;
-            stack_pointer[-2 - oparg] = func;
+            callable = stack_pointer[-2 - oparg];
+            _LOAD_ATTR_NOT_NULL_SELF
+            (void)callable;
+            stack_pointer[-2 - oparg] = attr;
             stack_pointer[-1 - oparg] = self;
             break;
         }
@@ -1405,6 +1409,7 @@
             assert(self_or_null != NULL);
             assert(args != NULL);
             if (sym_is_not_null(self_or_null)) {
+                DPRINTF(2, "BOUND METHOD FIDDLING\n");
                 // Bound method fiddling, same as _INIT_CALL_PY_EXACT_ARGS in VM
                 args--;
                 argcount++;
@@ -1415,6 +1420,7 @@
             // and make the current stack the new locals.
             // This also sets up for true call inlining.
             if (sym_is_known(self_or_null)) {
+                DPRINTF(2, "I KNOW YOU %d, %d\n", args - ctx->frame->locals, ctx->frame->locals_len);
                 localsplus_start = args;
                 n_locals_already_filled = argcount;
             }
@@ -1454,6 +1460,7 @@
             assert((this_instr - 3)->opcode == _CHECK_STACK_SPACE);
             _PyUOpInstruction *reconstruction_start = end_writebuffer;
             if (compile_frame_reconstruction(ctx, &end_writebuffer, true_end)) {
+                DPRINTF(1, "OUT OF WRITE SPACE FOR RECONSTRUCTION\n");
                 goto error;
             }
             uint64_t reconstruction_offset = (uint64_t)(reconstruction_start - trace);