Make CALL_LIST_APPEND a normal instruction, disable superinstructions in the JIT

Author: Fidget-Spinner

Include/internal/pycore_opcode_metadata.h

@@ -37,7 +37,6 @@ typedef struct _PyJitTracerInitialState {
 
 typedef struct _PyJitTracerPreviousState {
     bool dependencies_still_valid;
-    bool instr_is_super;
     int code_max_size;
     int code_curr_size;
     int instr_oparg;

Include/internal/pycore_tstate.h

@@ -4329,8 +4329,7 @@ dummy_func(
             DEOPT_IF(callable_o != interp->callable_cache.list_append);
         }
 
-        // This is secretly a super-instruction
-        op(_CALL_LIST_APPEND, (callable, self, arg -- c, s)) {
+        op(_CALL_LIST_APPEND, (callable, self, arg -- none, c, s)) {
             assert(oparg == 1);
             PyObject *self_o = PyStackRef_AsPyObjectBorrow(self);
 
@@ -4343,13 +4342,9 @@ dummy_func(
             }
             c = callable;
             s = self;
-            INPUTS_DEAD();
-        #if TIER_ONE
-            // Skip the following POP_TOP. This is done here in tier one, and
-            // during trace projection in tier two:
-            assert(next_instr->op.code == POP_TOP);
-            SKIP_OVER(1);
-        #endif
+            DEAD(callable);
+            DEAD(self);
+            none = PyStackRef_None;
         }
 
          op(_CALL_METHOD_DESCRIPTOR_O, (callable, self_or_null, args[oparg] -- res)) {
@@ -5597,15 +5592,10 @@ dummy_func(
             // Super instructions. Instruction deopted. There's a mismatch in what the stack expects
             // in the optimizer. So we have to reflect in the trace correctly.
             _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
-            if ((_tstate->jit_tracer_state.prev_state.instr->op.code == CALL_LIST_APPEND &&
-                opcode == POP_TOP) ||
-                (_tstate->jit_tracer_state.prev_state.instr->op.code == BINARY_OP_INPLACE_ADD_UNICODE &&
-                opcode == STORE_FAST)) {
-                _tstate->jit_tracer_state.prev_state.instr_is_super = true;
-            }
-            else {
-                _tstate->jit_tracer_state.prev_state.instr = next_instr;
-            }
+            // JIT should have disabled super instructions, as we can
+            // do these optimizations ourselves in the JIT.
+            assert(opcode != BINARY_OP_INPLACE_ADD_UNICODE);
+            _tstate->jit_tracer_state.prev_state.instr = next_instr;
             PyObject *prev_code = PyStackRef_AsPyObjectBorrow(frame->f_executable);
             if (_tstate->jit_tracer_state.prev_state.instr_code != (PyCodeObject *)prev_code) {
                 Py_SETREF(_tstate->jit_tracer_state.prev_state.instr_code, (PyCodeObject*)Py_NewRef((prev_code)));

Include/internal/pycore_uop_ids.h

@@ -703,12 +703,6 @@ _PyJit_translate_single_bytecode_to_trace(
     }
 #endif
 
-    // Skip over super instructions.
-    if (_tstate->jit_tracer_state.prev_state.instr_is_super) {
-        _tstate->jit_tracer_state.prev_state.instr_is_super = false;
-        return 1;
-    }
-
     if (opcode == ENTER_EXECUTOR) {
         goto full;
     }
@@ -957,12 +951,6 @@ _PyJit_translate_single_bytecode_to_trace(
                     trace[trace_length - 1].operand1 = PyStackRef_IsNone(frame->f_executable) ? 2 : ((int)(frame->stackpointer - _PyFrame_Stackbase(frame)));
                     break;
                 }
-                if (uop == _BINARY_OP_INPLACE_ADD_UNICODE) {
-                    assert(i + 1 == nuops);
-                    _Py_CODEUNIT *next = target_instr + 1 + _PyOpcode_Caches[_PyOpcode_Deopt[opcode]];
-                    assert(next->op.code == STORE_FAST);
-                    operand = next->op.arg;
-                }
                 // All other instructions
                 ADD_TO_TRACE(uop, oparg, operand, target);
             }
@@ -1077,7 +1065,6 @@ _PyJit_TryInitializeTracing(
     _tstate->jit_tracer_state.prev_state.instr_frame = frame;
     _tstate->jit_tracer_state.prev_state.instr_oparg = oparg;
     _tstate->jit_tracer_state.prev_state.instr_stacklevel = curr_stackdepth;
-    _tstate->jit_tracer_state.prev_state.instr_is_super = false;
     assert(curr_instr->op.code == JUMP_BACKWARD_JIT || (exit != NULL));
     _tstate->jit_tracer_state.initial_state.jump_backward_instr = curr_instr;
 

Include/internal/pycore_uop_metadata.h

@@ -299,8 +299,21 @@ dummy_func(void) {
     }
 
     op(_BINARY_OP_ADD_UNICODE, (left, right -- res, l, r)) {
-        REPLACE_OPCODE_IF_EVALUATES_PURE(left, right);
         res = sym_new_type(ctx, &PyUnicode_Type);
+        // Re-implement the BINARY_OP_INPLACE_ADD_UNICODE super-instruction.
+        if ((this_instr + 5)->opcode == _STORE_FAST) {
+            assert((this_instr + 1)->opcode == _POP_TOP_UNICODE);
+            assert((this_instr + 2)->opcode == _POP_TOP_UNICODE);
+            assert((this_instr + 3)->opcode == _CHECK_VALIDITY);
+            assert((this_instr + 4)->opcode == _SET_IP);
+            int local_slot = (this_instr + 5)->oparg;
+            if (PyJitRef_Unwrap(left) == PyJitRef_Unwrap(GETLOCAL(local_slot))) {
+                REPLACE_OP(this_instr, _BINARY_OP_INPLACE_ADD_UNICODE, oparg, local_slot);
+                REPLACE_OP(this_instr + 1, _NOP, 0, 0);
+                REPLACE_OP(this_instr + 2, _NOP, 0, 0);
+                REPLACE_OP(this_instr + 5, _NOP, 0, 0);
+            }
+        }
         l = left;
         r = right;
     }
@@ -1041,10 +1054,11 @@ dummy_func(void) {
         sym_set_const(flag, Py_True);
     }
 
-    op(_CALL_LIST_APPEND, (callable, self, arg -- c, s)) {
+    op(_CALL_LIST_APPEND, (callable, self, arg -- none, c, s)) {
         (void)(arg);
         c = callable;
         s = self;
+        none = sym_new_const(ctx, Py_None);
     }
 
     op(_GUARD_IS_FALSE_POP, (flag -- )) {