GH-118910: Less boilerplate in the tier 2 optimizer (#118913)

Author: markshannon

Include/internal/pycore_optimizer.h

@@ -93,7 +93,9 @@ typedef struct ty_arena {
 } ty_arena;
 
 struct _Py_UOpsContext {
-    PyObject_HEAD
+    char done;
+    char out_of_space;
+    bool contradiction;
     // The current "executing" frame.
     _Py_UOpsAbstractFrame *frame;
     _Py_UOpsAbstractFrame frames[MAX_ABSTRACT_FRAME_DEPTH];
@@ -121,16 +123,16 @@ extern _Py_UopsSymbol *_Py_uop_sym_new_const(_Py_UOpsContext *ctx, PyObject *con
 extern _Py_UopsSymbol *_Py_uop_sym_new_null(_Py_UOpsContext *ctx);
 extern bool _Py_uop_sym_has_type(_Py_UopsSymbol *sym);
 extern bool _Py_uop_sym_matches_type(_Py_UopsSymbol *sym, PyTypeObject *typ);
-extern bool _Py_uop_sym_set_null(_Py_UopsSymbol *sym);
-extern bool _Py_uop_sym_set_non_null(_Py_UopsSymbol *sym);
-extern bool _Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ);
-extern bool _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val);
+extern void _Py_uop_sym_set_null(_Py_UOpsContext *ctx, _Py_UopsSymbol *sym);
+extern void _Py_uop_sym_set_non_null(_Py_UOpsContext *ctx, _Py_UopsSymbol *sym);
+extern void _Py_uop_sym_set_type(_Py_UOpsContext *ctx, _Py_UopsSymbol *sym, PyTypeObject *typ);
+extern void _Py_uop_sym_set_const(_Py_UOpsContext *ctx, _Py_UopsSymbol *sym, PyObject *const_val);
 extern bool _Py_uop_sym_is_bottom(_Py_UopsSymbol *sym);
 extern int _Py_uop_sym_truthiness(_Py_UopsSymbol *sym);
 extern PyTypeObject *_Py_uop_sym_get_type(_Py_UopsSymbol *sym);
 
 
-extern int _Py_uop_abstractcontext_init(_Py_UOpsContext *ctx);
+extern void _Py_uop_abstractcontext_init(_Py_UOpsContext *ctx);
 extern void _Py_uop_abstractcontext_fini(_Py_UOpsContext *ctx);
 
 extern _Py_UOpsAbstractFrame *_Py_uop_frame_new(

Lib/test/test_generated_cases.py

@@ -909,7 +909,6 @@ def test_overridden_abstract_args(self):
         case OP2: {
             _Py_UopsSymbol *out;
             out = sym_new_not_null(ctx);
-            if (out == NULL) goto out_of_space;
             stack_pointer[-1] = out;
             break;
         }
@@ -934,7 +933,6 @@ def test_no_overridden_case(self):
         case OP: {
             _Py_UopsSymbol *out;
             out = sym_new_not_null(ctx);
-            if (out == NULL) goto out_of_space;
             stack_pointer[-1] = out;
             break;
         }

Python/optimizer_analysis.c

@@ -297,20 +297,6 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
     INST->oparg = ARG;            \
     INST->operand = OPERAND;
 
-#define OUT_OF_SPACE_IF_NULL(EXPR)     \
-    do {                               \
-        if ((EXPR) == NULL) {          \
-            goto out_of_space;         \
-        }                              \
-    } while (0);
-
-#define _LOAD_ATTR_NOT_NULL \
-    do {                    \
-    OUT_OF_SPACE_IF_NULL(attr = _Py_uop_sym_new_not_null(ctx)); \
-    OUT_OF_SPACE_IF_NULL(null = _Py_uop_sym_new_null(ctx)); \
-    } while (0);
-
-
 /* Shortened forms for convenience, used in optimizer_bytecodes.c */
 #define sym_is_not_null _Py_uop_sym_is_not_null
 #define sym_is_const _Py_uop_sym_is_const
@@ -324,10 +310,10 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
 #define sym_has_type _Py_uop_sym_has_type
 #define sym_get_type _Py_uop_sym_get_type
 #define sym_matches_type _Py_uop_sym_matches_type
-#define sym_set_null _Py_uop_sym_set_null
-#define sym_set_non_null _Py_uop_sym_set_non_null
-#define sym_set_type _Py_uop_sym_set_type
-#define sym_set_const _Py_uop_sym_set_const
+#define sym_set_null(SYM) _Py_uop_sym_set_null(ctx, SYM)
+#define sym_set_non_null(SYM) _Py_uop_sym_set_non_null(ctx, SYM)
+#define sym_set_type(SYM, TYPE) _Py_uop_sym_set_type(ctx, SYM, TYPE)
+#define sym_set_const(SYM, CNST) _Py_uop_sym_set_const(ctx, SYM, CNST)
 #define sym_is_bottom _Py_uop_sym_is_bottom
 #define sym_truthiness _Py_uop_sym_truthiness
 #define frame_new _Py_uop_frame_new
@@ -408,18 +394,20 @@ optimize_uops(
     _PyUOpInstruction *first_valid_check_stack = NULL;
     _PyUOpInstruction *corresponding_check_stack = NULL;
 
-    if (_Py_uop_abstractcontext_init(ctx) < 0) {
-        goto out_of_space;
-    }
+    _Py_uop_abstractcontext_init(ctx);
     _Py_UOpsAbstractFrame *frame = _Py_uop_frame_new(ctx, co, ctx->n_consumed, 0, curr_stacklen);
     if (frame == NULL) {
         return -1;
     }
     ctx->curr_frame_depth++;
     ctx->frame = frame;
+    ctx->done = false;
+    ctx->out_of_space = false;
+    ctx->contradiction = false;
 
     _PyUOpInstruction *this_instr = NULL;
-    for (int i = 0; i < trace_len; i++) {
+    for (int i = 0; !ctx->done; i++) {
+        assert(i < trace_len);
         this_instr = &trace[i];
 
         int oparg = this_instr->oparg;
@@ -447,32 +435,22 @@ optimize_uops(
         ctx->frame->stack_pointer = stack_pointer;
         assert(STACK_LEVEL() >= 0);
     }
-    Py_UNREACHABLE();
-
-out_of_space:
-    DPRINTF(3, "\n");
-    DPRINTF(1, "Out of space in abstract interpreter\n");
-    goto done;
-error:
-    DPRINTF(3, "\n");
-    DPRINTF(1, "Encountered error in abstract interpreter\n");
-    if (opcode <= MAX_UOP_ID) {
-        OPT_ERROR_IN_OPCODE(opcode);
+    if (ctx->out_of_space) {
+        DPRINTF(3, "\n");
+        DPRINTF(1, "Out of space in abstract interpreter\n");
+    }
+    if (ctx->contradiction) {
+        // Attempted to push a "bottom" (contradiction) symbol onto the stack.
+        // This means that the abstract interpreter has hit unreachable code.
+        // We *could* generate an _EXIT_TRACE or _FATAL_ERROR here, but hitting
+        // bottom indicates type instability, so we are probably better off
+        // retrying later.
+        DPRINTF(3, "\n");
+        DPRINTF(1, "Hit bottom in abstract interpreter\n");
+        _Py_uop_abstractcontext_fini(ctx);
+        return 0;
     }
-    _Py_uop_abstractcontext_fini(ctx);
-    return -1;
 
-hit_bottom:
-    // Attempted to push a "bottom" (contradition) symbol onto the stack.
-    // This means that the abstract interpreter has hit unreachable code.
-    // We *could* generate an _EXIT_TRACE or _FATAL_ERROR here, but hitting
-    // bottom indicates type instability, so we are probably better off
-    // retrying later.
-    DPRINTF(3, "\n");
-    DPRINTF(1, "Hit bottom in abstract interpreter\n");
-    _Py_uop_abstractcontext_fini(ctx);
-    return 0;
-done:
     /* Either reached the end or cannot optimize further, but there
      * would be no benefit in retrying later */
     _Py_uop_abstractcontext_fini(ctx);
@@ -485,6 +463,16 @@ optimize_uops(
         first_valid_check_stack->operand = max_space;
     }
     return trace_len;
+
+error:
+    DPRINTF(3, "\n");
+    DPRINTF(1, "Encountered error in abstract interpreter\n");
+    if (opcode <= MAX_UOP_ID) {
+        OPT_ERROR_IN_OPCODE(opcode);
+    }
+    _Py_uop_abstractcontext_fini(ctx);
+    return -1;
+
 }