magic numbers

Treat back edges as an exit, not a penalty, this way they are more likely to end at a backedge instead of ending at random spots

Author: Fidget-Spinner

Include/internal/pycore_optimizer.h

@@ -45,20 +45,22 @@ extern "C" {
  * Runtime charging uses trace-buffer capacity consumed for each bytecode. */
 #define AVG_SLOTS_PER_INSTRUCTION  6
 
-/* Heuristic backward-edge penalty: leave room for about
+/* Heuristic backward-edge exit quality: leave room for about 1 unroll and
  * N_BACKWARD_SLACK more bytecodes before reaching EXIT_QUALITY_CLOSE_LOOP,
  * based on AVG_SLOTS_PER_INSTRUCTION. */
 #define N_BACKWARD_SLACK           50
-#define FITNESS_BACKWARD_EDGE      (FITNESS_INITIAL/2 \
-                                      - N_BACKWARD_SLACK * AVG_SLOTS_PER_INSTRUCTION)
+#define EXIT_QUALITY_BACKWARD_EDGE (EXIT_QUALITY_CLOSE_LOOP / 2 - N_BACKWARD_SLACK * AVG_SLOTS_PER_INSTRUCTION)
 
 /* Backward edge penalty for JUMP_BACKWARD_NO_INTERRUPT (coroutines/yield-from).
- * Smaller than FITNESS_BACKWARD_EDGE since these loops are very short. */
-#define FITNESS_BACKWARD_EDGE_COROUTINE  (FITNESS_BACKWARD_EDGE / 4)
+ * Smaller than FITNESS_BACKWARD_EDGE since we want to trace through them. */
+#define EXIT_QUALITY_BACKWARD_EDGE_COROUTINE  (EXIT_QUALITY_BACKWARD_EDGE / 8)
 
 /* Penalty for a perfectly balanced (50/50) branch.
- * 7 such branches (after per-slot cost) exhaust fitness to EXIT_QUALITY_DEFAULT. */
-#define FITNESS_BRANCH_BALANCED    ((FITNESS_INITIAL - EXIT_QUALITY_DEFAULT) / (7 * AVG_SLOTS_PER_INSTRUCTION))
+ * 7 such branches (after per-slot cost) exhaust fitness to EXIT_QUALITY_DEFAULT. 
+ * The calculation assumes the branches are spread out roughly equally throughout the trace.
+ */
+#define FITNESS_BRANCH_BALANCED    ((FITNESS_INITIAL - EXIT_QUALITY_DEFAULT - \
+                                        (MAX_TARGET_LENGTH / 7 * AVG_SLOTS_PER_INSTRUCTION)) / (7))
 
 
 typedef struct _PyJitUopBuffer {

Python/optimizer.c

@@ -614,15 +614,25 @@ static inline int32_t
 compute_exit_quality(_Py_CODEUNIT *target_instr, int opcode,
                      const _PyJitTracerState *tracer)
 {
-    if (target_instr == tracer->initial_state.start_instr ||
-        target_instr == tracer->initial_state.close_loop_instr) {
-        return EXIT_QUALITY_CLOSE_LOOP;
-    }
-    if (target_instr->op.code == ENTER_EXECUTOR && !_PyJit_EnterExecutorShouldStopTracing(opcode)) {
-        return EXIT_QUALITY_ENTER_EXECUTOR;
-    }
-    else if (_PyOpcode_Caches[_PyOpcode_Deopt[opcode]] > 0) {
-        return EXIT_QUALITY_SPECIALIZABLE;
+    // We need to check for this, otherwise the first instruction (JUMP_BACKWARD usually)
+    // is mistakenly thought of as an exit.
+    if (uop_buffer_length((_PyJitUopBuffer *)&tracer->code_buffer) > CODE_SIZE_NO_PROGRESS) {
+        if (target_instr == tracer->initial_state.start_instr ||
+            target_instr == tracer->initial_state.close_loop_instr) {
+            return EXIT_QUALITY_CLOSE_LOOP;
+        }
+        else if (target_instr->op.code == ENTER_EXECUTOR && !_PyJit_EnterExecutorShouldStopTracing(opcode)) {
+            return EXIT_QUALITY_ENTER_EXECUTOR;
+        }
+        else if (opcode == JUMP_BACKWARD_JIT || opcode == JUMP_BACKWARD) {
+            return EXIT_QUALITY_BACKWARD_EDGE;
+        }
+        else if (opcode == JUMP_BACKWARD_NO_INTERRUPT) {
+            return EXIT_QUALITY_BACKWARD_EDGE_COROUTINE;
+        }
+        else if (_PyOpcode_Caches[_PyOpcode_Deopt[opcode]] > 0) {
+            return EXIT_QUALITY_SPECIALIZABLE;
+        }
     }
     return EXIT_QUALITY_DEFAULT;
 }
@@ -820,8 +830,6 @@ _PyJit_translate_single_bytecode_to_trace(
     // One for possible _DEOPT, one because _CHECK_VALIDITY itself might _DEOPT
     trace->end -= 2;
 
-    const struct opcode_macro_expansion *expansion = &_PyOpcode_macro_expansion[opcode];
-
     assert(opcode != ENTER_EXECUTOR && opcode != EXTENDED_ARG);
     assert(!_PyErr_Occurred(tstate));
 
@@ -843,6 +851,7 @@ _PyJit_translate_single_bytecode_to_trace(
     trace->end -= needs_guard_ip;
 
 #if Py_DEBUG
+    const struct opcode_macro_expansion *expansion = &_PyOpcode_macro_expansion[opcode];
     int space_needed = expansion->nuops + needs_guard_ip + 2 + (!OPCODE_HAS_NO_SAVE_IP(opcode));
     assert(uop_buffer_remaining_space(trace) > space_needed);
 #endif
@@ -880,15 +889,8 @@ _PyJit_translate_single_bytecode_to_trace(
         case JUMP_BACKWARD_NO_JIT:
         case JUMP_BACKWARD:
             ADD_TO_TRACE(_CHECK_PERIODIC, 0, 0, target);
-            tracer->translator_state.fitness -= FITNESS_BACKWARD_EDGE;
-            DPRINTF(3, "  backward edge penalty: -%d -> fitness=%d\n",
-                    FITNESS_BACKWARD_EDGE, tracer->translator_state.fitness);
             break;
         case JUMP_BACKWARD_NO_INTERRUPT:
-            tracer->translator_state.fitness -= FITNESS_BACKWARD_EDGE_COROUTINE;
-            DPRINTF(3, "  coroutine backward edge penalty: -%d -> fitness=%d\n",
-                    FITNESS_BACKWARD_EDGE_COROUTINE,
-                    tracer->translator_state.fitness);
             break;
 
         case RESUME: