Skip to content

gh-144540: Eliminate _MAKE_HEAP_FUNCTION for owned references#146320

Open
Sacul0457 wants to merge 1 commit intopython:mainfrom
Sacul0457:Remove-Redundant-_MAKE_HEAP_SAFE
Open

gh-144540: Eliminate _MAKE_HEAP_FUNCTION for owned references#146320
Sacul0457 wants to merge 1 commit intopython:mainfrom
Sacul0457:Remove-Redundant-_MAKE_HEAP_SAFE

Conversation

@Sacul0457
Copy link
Contributor

@Sacul0457 Sacul0457 commented Mar 23, 2026

If the reference of the returned object is not borrowed, we remove the _MAKE_HEAP_FUNCTION. Hence I ended up changing the test_make_heap_safe_optimized_owned test to test that _MAKE_HEAP_FUNCTION is no longer in the trace as the value returned is not a borrowed reference.

Also, for a "return of a return", given the inner function returns a borrowed reference, the double _MAKE_HEAP_FUNCTION is reduced to just 1 :)

  44 OPTIMIZED: _MAKE_HEAP_SAFE_r11 (0, target=3, operand0=0, operand1=0)
  45 OPTIMIZED: _RETURN_VALUE_r11 (0, target=3, operand0=0, operand1=0)
  46 OPTIMIZED: _CHECK_VALIDITY_r11 (0, jump_target=71, operand0=0, operand1=0)
  47 OPTIMIZED: _SET_IP_r11 (0, target=10, operand0=0x1b5f9947d24, operand1=0)
  48 OPTIMIZED: _RETURN_VALUE_r11 (0, target=10, operand0=0, operand1=0)

I hope I didn't misunderstand the issue...

@cocolato
Copy link
Contributor

I think this might be a problem because in creation functions such as sym_new_not_null and sym_new_unknown, :

JitOptRef
_Py_uop_sym_new_unknown(JitOptContext *ctx)
{
JitOptSymbol *res = sym_new(ctx);
if (res == NULL) {
return out_of_space_ref(ctx);
}
return PyJitRef_Wrap(res);
}
JitOptRef
_Py_uop_sym_new_not_null(JitOptContext *ctx)
{
JitOptSymbol *res = sym_new(ctx);
if (res == NULL) {
return out_of_space_ref(ctx);
}
res->tag = JIT_SYM_NON_NULL_TAG;
return PyJitRef_Wrap(res);
}

PyJitRef_Wrap produces a ref with tag = 0.
This means that tag = 0 is the default state, indicating “we have no specific information about borrowing/ownership,” rather than “definitely owned.” Only those explicitly marked with tag = 1 via PyJitRef_Borrow() are “known to be borrowed.”

@cocolato
Copy link
Contributor

Mark also reminded me in the last PR: #144414 (review).

@Fidget-Spinner
Copy link
Member

Hmm Mark is indeed right. However, currently the only place that can generate borrowed references are LOAD_FAST_BORROW. I suspect we need to create a new tag bit for this to work.

We only support good perf on 64 bit systems right now. I propose we drop the 32 bit Windows support so that we have 3 tag bits to use and can support "unknown" references.

@cocolato
Copy link
Contributor

I propose we drop the 32 bit Windows support so that we have 3 tag bits to use and can support "unknown" references.

I also don’t think it’s necessary to maintain the performance of legacy systems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants