`LOAD_FAST` is not always converted to `LOAD_FAST_BORROW` in a `basicblock`

[brijkapadia]

Bug report

Bug description:

Originally addressed in Issue #144388, the bytecode compiler does not always convert a LOAD_FAST instruction into a LOAD_FAST_BORROW. For example, the following python code, which gets compiled into only one basicblock, has a LOAD_FAST when it could have a LOAD_FAST_BORROW:

from dis import dis

def f(a,b):
    return a if a < b else b

dis(f)

Byte code

  3           RESUME                   0

  4           LOAD_FAST_BORROW_LOAD_FAST_BORROW 1 (a, b)
              COMPARE_OP              18 (bool(<))
              POP_JUMP_IF_FALSE        3 (to L1)
              NOT_TAKEN
              LOAD_FAST_BORROW         0 (a)
              RETURN_VALUE
      L1:     LOAD_FAST                1 (b)
              RETURN_VALUE

In contrast, the following expanded version of f does convert all LOAD_FAST instructions into LOAD_FAST_BORROW and has three different basic blocks:

from dis import dis

def g(a,b):
    if a < b:
        return a
    else:
        return b

dis(g)

Byte code

   3           RESUME                   0

  4           LOAD_FAST_BORROW_LOAD_FAST_BORROW 1 (a, b)
              COMPARE_OP              18 (bool(<))
              POP_JUMP_IF_FALSE        3 (to L1)
              NOT_TAKEN

  5           LOAD_FAST_BORROW         0 (a)
              RETURN_VALUE

  7   L1:     LOAD_FAST_BORROW         1 (b)
              RETURN_VALUE

The function of interest is optimize_load_fast in Python/flowgraph.c. I have tried to fix this bug but have not been able to yet. All I have found so far is that the LOAD_FAST instruction is misclassified as REF_UNCONSUMED here for the case of function f:

cpython/Python/flowgraph.c

Lines 3006 to 3011 in 149c465

	for (Py_ssize_t i = 0; i < refs.size; i++) {
	ref r = ref_stack_at(&refs, i);
	if (r.instr != -1) {
	instr_flags[r.instr] |= REF_UNCONSUMED;
	}
	}

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-145629: Expand block inlining to enable LOAD_FAST_BORROW optimization for return ternary expressions #146503

[VanshAgarwal24036]

I investigated in in optimize_load_fast() and found that LOAD_FAST branch is marked REF_UNCONSUMED during reference stack cleanup, which prevents the LOAD_FAST -> LOAD_FAST_BORROW optimization.

From debugging i found that it appears that the refrence remains on the simulated stack when the block ends with RETURN_VALUE, causing it to be incorrectly classified.

Debug line added:

printf("[DEBUG] Marking instr %d as REF_UNCONSUMED (block size %d)\n",
       r.instr, block->b_iused);

Terminal output:

[DEBUG] Marking instr 0 as REF_UNCONSUMED (block size 1)

I want to confirm that whether the refrence stack should treat RETURN_VALUE as consuming the top stack value during this analysis step.

[anand-official]

Root Cause Analysis & Proposed Fix

I've investigated this issue further and can confirm @VanshAgarwal24036's analysis. The root cause is in the reference stack cleanup logic (lines 3006-3011 in Python/flowgraph.c).

The Problem

When a basic block ends with RETURN_VALUE, the bytecode simulation leaves the reference on the stack, which is then marked as REF_UNCONSUMED during cleanup. This prevents the LOAD_FAST → LOAD_FAST_BORROW optimization from occurring.

However, RETURN_VALUE does consume its operand - it pops the value from the stack to return it. The issue is that the simulation doesn't account for this consumption before the cleanup loop runs.

Proposed Solution

The fix requires modifying the reference stack simulation to properly handle terminal instructions like RETURN_VALUE that consume stack values. Here's the approach:

Before the cleanup loop (around line 3005), add:

// Handle instructions that consume but don't explicitly pop during simulation
if (block->b_iused > 0) {
    cfg_instr *last_instr = &block->b_instr[block->b_iused - 1];
    if (last_instr->i_opcode == RETURN_VALUE && refs.size > 0) {
        // RETURN_VALUE consumes the top stack value
        ref_stack_pop(&refs);
    }
}

This ensures that when a block ends with RETURN_VALUE, we properly consume the reference from the simulated stack before marking remaining references as unconsumed.

Performance Impact

This fix will enable the LOAD_FAST_BORROW optimization in single-block conditional expressions (like the ternary operator example), reducing reference counting overhead in hot paths.

Testing

I can provide a comprehensive test case and benchmark demonstrating the improvement if needed. Would you like me to prepare a PR with this fix?

[brijkapadia]

@anand-official When I tested your solution, it did not convert the LOAD_FAST into a LOAD_FAST_BORROW from the first example in the original issue.

Would you like me to prepare a PR with this fix?

No.

---

I want to confirm that whether the refrence stack should treat RETURN_VALUE as consuming the top stack value during this analysis step.

@VanshAgarwal24036 RETURN_VALUE should consume the stack variable. As such, I am not exactly sure about the root cause of this issue. Unless I am missing something, the issue seems very subtle.

I would appreciate any guidance from people who understand the byte code optimizer better than I do.

[anand-official]

Thank you for the detailed bug report. I’ve been looking into this as well and wanted to share my findings and a question.

What I found
The first LOAD_FAST (instruction 0) gets marked REF_UNCONSUMED in the cleanup loop (lines 3006–3011).

This happens because the reference stack still contains an entry when the block ends with RETURN_VALUE. It looks like the simulation never pops for RETURN_VALUE in the main loop.

Why the simple fix didn’t work
I tried popping the reference stack for RETURN_VALUE before cleanup, but that alone doesn’t enable the optimization because the REF_LAST_USE flag for that instruction was already missed in the main loop. We need to handle the pop inside the main loop, just like for BINARY_OP or COMPARE_OP.

The challenge
RETURN_VALUE consumes the top stack value, but that value may or may not be a reference. The current reference stack only tracks references, not all values. To pop correctly, we’d need to know whether the top value is a reference – essentially, we’d need a parallel “value type” stack or a way to deduce it from the instruction that produced it.

Question for core devs
@markshannon, @brandtbucher, @iritkatriel—how is the reference simulation supposed to handle RETURN_VALUE? Is there a known limitation or a simpler way to determine if the top value is a reference without a full value stack? I’d be happy to help implement a fix once we agree on the right approach.

[VanshAgarwal24036]

I also added debug prints while investigating this and saw that the instruction corresponding to the LOAD_FAST in the else branch gets marked REF_UNCONSUMED during the cleanup loop.

For example:

[DEBUG] Marking instr 0 as REF_UNCONSUMED (block size 1)

This seems to correspond to the LOAD_FAST b before RETURN_VALUE.

So the reference stack still contains the entry when the block ends with RETURN_VALUE, which then gets marked as REF_UNCONSUMED.