Miscellaneous cleanup

python · markshannon · Mar 21, 2022 · Mar 10, 2022 · Mar 10, 2022 · Mar 10, 2022
commit ecfb193e11b6fb8408cc7ed132c2984aac8810b2
@@ -105,9 +105,9 @@ frame_getback(PyFrameObject *f, void *closure)
     return res;
 }
 
-/* Given the index of the effective opcode,
-   scan back to construct the oparg with EXTENDED_ARG */
-// XXX This is broken!
+// Given the index of the effective opcode, scan back to construct the oparg
+// with EXTENDED_ARG. This only works correctly with *unquickened* code,
+// obtained via a call to _PyCode_GetCode!
 static unsigned int
 get_arg(const _Py_CODEUNIT *codestr, Py_ssize_t i)
 {
@@ -171,18 +171,17 @@ top_of_stack(int64_t stack)
 static int64_t *
 mark_stacks(PyCodeObject *code_obj, int len)
 {
-    // XXX: this is one big TODO!!!
-    PyObject *xxx = _PyCode_GetCode(code_obj);
-    if (xxx == NULL) {
+    PyObject *co_code = _PyCode_GetCode(code_obj);
+    if (co_code == NULL) {
         return NULL;
     }
-    _Py_CODEUNIT *code = (_Py_CODEUNIT *)PyBytes_AS_STRING(xxx);
+    _Py_CODEUNIT *code = (_Py_CODEUNIT *)PyBytes_AS_STRING(co_code);
     int64_t *stacks = PyMem_New(int64_t, len+1);
     int i, j, opcode;
 
     if (stacks == NULL) {
         PyErr_NoMemory();
-        Py_DECREF(xxx);
+        Py_DECREF(co_code);
         return NULL;
     }
     for (int i = 1; i <= len; i++) {
@@ -310,7 +309,7 @@ mark_stacks(PyCodeObject *code_obj, int len)
             }
         }
     }
-    Py_DECREF(xxx);
+    Py_DECREF(co_code);
     return stacks;
 }
 
@@ -845,15 +844,23 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
 static int
 _PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame, int opcode, int oparg)
 {
-    // XXX: Does this handle EXTENDED_ARGs/CACHEs?
+    // This only works when opcode is a non-quickened form:
+    assert(_PyOpcode_Deopt[opcode] == opcode);
+    int check_oparg = 0;
     for (int i = 0; i < frame->f_lasti; i++) {
         _Py_CODEUNIT instruction = _PyCode_CODE(frame->f_code)[i];
-        int deopt = _PyOpcode_Deopt[_Py_OPCODE(instruction)];
-        instruction = _Py_MAKECODEUNIT(deopt, oparg);
-        if (instruction == _Py_MAKECODEUNIT(opcode, oparg)) {
+        int check_opcode = _PyOpcode_Deopt[_Py_OPCODE(instruction)];
+        check_oparg |= _Py_OPARG(instruction);
+        if (check_opcode == opcode && check_oparg == oparg) {
             return 1;
         }
-        i += _PyOpcode_Caches[deopt];
+        if (check_opcode == EXTENDED_ARG) {
+            check_oparg <<= 8;
+        }
+        else {
+            check_oparg = 0;
+        }
+        i += _PyOpcode_Caches[check_opcode];
     }
     return 0;
 }
@@ -872,7 +879,10 @@ _PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) {
     }
     co = frame->f_code;
     fast = _PyFrame_GetLocalsArray(frame);
-    if (frame->f_lasti < 0 && _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS) {
+    // COPY_FREE_VARS has no quickened forms, so no need to use _PyOpcode_Deopt
+    // here:
+    if (frame->f_lasti < 0 && _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS)
+    {
         /* Free vars have not been initialized -- Do that */
         PyCodeObject *co = frame->f_code;
         PyObject *closure = frame->f_func->func_closure;

@@ -8949,6 +8949,8 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyCodeObject *co,
         // "firstarg" is a cell here unless (very unlikely) super()
         // was called from the C-API before the first MAKE_CELL op.
         if (cframe->f_lasti >= 0) {
+            // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need
+            // to use _PyOpcode_Deopt here:
             assert(_Py_OPCODE(_PyCode_CODE(co)[0]) == MAKE_CELL || 
                    _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS);
             assert(PyCell_Check(firstarg));

@@ -6706,7 +6706,6 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj,
        then call the trace function if we're tracing source lines.
     */
     initialize_trace_info(&tstate->trace_info, frame);
-    // XXX: Is this broken?
     int prev = _PyCode_CODE(frame->f_code)[instr_prev];
     int lastline;
     if (_PyOpcode_Deopt[_Py_OPCODE(prev)] == RESUME && _Py_OPARG(prev) == 0) {
@@ -6724,6 +6723,8 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj,
         /* Trace backward edges (except in 'yield from') or if line number has changed */
         int trace = line != lastline ||
             (frame->f_lasti < instr_prev &&
+            // SEND has no quickened forms, so no need to use _PyOpcode_Deopt
+            // here:
             _Py_OPCODE(_PyCode_CODE(frame->f_code)[frame->f_lasti]) != SEND);
         if (trace) {
             result = call_trace(func, obj, tstate, frame, PyTrace_LINE, Py_None);

@@ -304,8 +304,9 @@ def generate_code(self, name: str, code: types.CodeType) -> str:
             self.write(f".co_endlinetable = {co_endlinetable},")
             self.write(f".co_columntable = {co_columntable},")
             self.write(f"._co_code = {make_string_literal(code.co_code)},")
-        self.deallocs.append(f"_PyStaticCode_Dealloc((PyCodeObject *)&{name});")
-        self.interns.append(f"_PyStaticCode_InternStrings((PyCodeObject *)&{name})")
+        cast = f"(PyCodeObject *)&{name}"
+        self.deallocs.append(f"_PyStaticCode_Dealloc({cast});")
+        self.interns.append(f"_PyStaticCode_InternStrings({cast})")
         return f"& {name}.ob_base.ob_base"
 
     def generate_tuple(self, name: str, t: Tuple[object, ...]) -> str: