Remove _BINARY_OP_INPLACE_ADD_UNICODE super instruction

Author: Fidget-Spinner

Include/internal/pycore_opcode_metadata.h

@@ -588,7 +588,7 @@ dummy_func(
             BINARY_OP_SUBSCR_STR_INT,
             BINARY_OP_SUBSCR_DICT,
             BINARY_OP_SUBSCR_GETITEM,
-            // BINARY_OP_INPLACE_ADD_UNICODE,  // See comments at that opcode.
+            BINARY_OP_INPLACE_ADD_UNICODE,
             BINARY_OP_EXTEND,
         };
 
@@ -762,13 +762,10 @@ dummy_func(
         macro(BINARY_OP_ADD_UNICODE) =
             _GUARD_TOS_UNICODE + _GUARD_NOS_UNICODE + unused/5 + _BINARY_OP_ADD_UNICODE + _POP_TOP_UNICODE + _POP_TOP_UNICODE;
 
-        // This is a subtle one. It's a super-instruction for
-        // BINARY_OP_ADD_UNICODE followed by STORE_FAST
-        // where the store goes into the left argument.
-        // So the inputs are the same as for all BINARY_OP
-        // specializations, but there is no output.
-        // At the end we just skip over the STORE_FAST.
-        op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right --)) {
+        // This is a subtle one. We write NULL to the local
+        // of the following STORE_FAST and leave the result for STORE_FAST
+        // later to store.
+        op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right -- res)) {
             PyObject *left_o = PyStackRef_AsPyObjectBorrow(left);
             assert(PyUnicode_CheckExact(left_o));
             assert(PyUnicode_CheckExact(PyStackRef_AsPyObjectBorrow(right)));
@@ -796,20 +793,16 @@ dummy_func(
              * that the string is safe to mutate.
              */
             assert(Py_REFCNT(left_o) >= 2 || !PyStackRef_IsHeapSafe(left));
-            PyStackRef_CLOSE_SPECIALIZED(left, _PyUnicode_ExactDealloc);
-            DEAD(left);
             PyObject *temp = PyStackRef_AsPyObjectSteal(*target_local);
-            PyObject *right_o = PyStackRef_AsPyObjectSteal(right);
+            PyObject *right_o = PyStackRef_AsPyObjectBorrow(right);
             PyUnicode_Append(&temp, right_o);
-            *target_local = PyStackRef_FromPyObjectSteal(temp);
-            Py_DECREF(right_o);
-            ERROR_IF(PyStackRef_IsNull(*target_local));
-        #if TIER_ONE
-            // The STORE_FAST is already done. This is done here in tier one,
-            // and during trace projection in tier two:
-            assert(next_instr->op.code == STORE_FAST);
-            SKIP_OVER(1);
-        #endif
+            PyStackRef_CLOSE_SPECIALIZED(right, _PyUnicode_ExactDealloc);
+            DEAD(right);
+            PyStackRef_CLOSE_SPECIALIZED(left, _PyUnicode_ExactDealloc);
+            DEAD(left);
+            ERROR_IF(temp == NULL);
+            res = PyStackRef_FromPyObjectSteal(temp);
+            *target_local = PyStackRef_NULL;
         }
 
        op(_GUARD_BINARY_OP_EXTEND, (descr/4, left, right -- left, right)) {
@@ -5594,7 +5587,6 @@ dummy_func(
             _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
             // 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) {

Include/internal/pycore_uop_ids.h

@@ -951,6 +951,12 @@ _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);
             }

Include/internal/pycore_uop_metadata.h

@@ -300,26 +300,11 @@ dummy_func(void) {
 
     op(_BINARY_OP_ADD_UNICODE, (left, right -- res, l, r)) {
         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;
     }
 
-    op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right --)) {
-        JitOptRef res;
+    op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right -- res)) {
         if (sym_is_const(ctx, left) && sym_is_const(ctx, right)) {
             assert(PyUnicode_CheckExact(sym_get_const(ctx, left)));
             assert(PyUnicode_CheckExact(sym_get_const(ctx, right)));
@@ -333,8 +318,7 @@ dummy_func(void) {
         else {
             res = sym_new_type(ctx, &PyUnicode_Type);
         }
-        // _STORE_FAST:
-        GETLOCAL(this_instr->operand0) = res;
+        GETLOCAL(this_instr->operand0) = sym_new_null(ctx);
     }
 
     op(_BINARY_OP_SUBSCR_INIT_CALL, (container, sub, getitem  -- new_frame)) {