Skip to content

gh-131798: Split CHECK_FUNCTION_VERSION into two guards in the JIT#145080

Open
Sacul0457 wants to merge 11 commits intopython:mainfrom
Sacul0457:Split-up-CHECK_FUNCTION_VERSION
Open

gh-131798: Split CHECK_FUNCTION_VERSION into two guards in the JIT#145080
Sacul0457 wants to merge 11 commits intopython:mainfrom
Sacul0457:Split-up-CHECK_FUNCTION_VERSION

Conversation

@Sacul0457
Copy link
Contributor

@Sacul0457 Sacul0457 commented Feb 21, 2026

This PR splits up _CHECK_FUNCTION_VERSION into _GUARD_CALLABLE_FUNCTION and itself. This allows the optimizer to remove the PyFunction_Check check if we know the type. I think the naming can be improved but this is just what I went for.

This can be done for _CHECK_FUNCTION_VERSION_KW too but I left that out to keep the change smaller.

@Sacul0457 Sacul0457 force-pushed the Split-up-CHECK_FUNCTION_VERSION branch from 37e58ef to e775a6b Compare February 25, 2026 09:08
@Sacul0457 Sacul0457 marked this pull request as ready for review March 13, 2026 13:24
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@Sacul0457
Copy link
Contributor Author

Is this optimization still correct/sound after #146101?

op(_CHECK_FUNCTION_VERSION, (func_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) {
assert(sym_matches_type(callable, &PyFunction_Type));
if (sym_get_func_version(callable) == func_version) {
REPLACE_OP(this_instr, _NOP, 0, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised this existing code I wrote might be wrong

The idea is that it's possible to modify a function version midway in a trace, so we need to recheck it or at least add a watcher at the first check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sacul0457 would you be willing to help me fix this (please open a separate issue if so)? Basically you'd need to add a PyFunction_AddWatcher in the else case of this branch. (See what we do in _GUARD_TYPE_VERSION). It's just a callback that invalidates the executors (see type_watcher_callback).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, Ill open an issue and the PR in a bit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I was just taking a look at this issue and I've got a few questions:

  • We add a PyFunction_AddWatcher only if the func_version was set properly, like what _GUARD_TYPE_VERSION version does?
  • If so, what PyFunction_WatchCallback do I add to PyFunction_AddWatcher or am I suppose to create one similar to type_watcher_callback

I'll continue to look into this in the meantime, thx! :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We add a PyFunction_AddWatcher only if the func_version was set properly, like what _GUARD_TYPE_VERSION version does?

Yup!

If so, what PyFunction_WatchCallback do I add to PyFunction_AddWatcher or am I suppose to create one similar to type_watcher_callback

Yes you need to create your own one with its own ID similar to type_watcher_callback.

Copy link
Member

@Fidget-Spinner Fidget-Spinner Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function watcher needs to be more specific though: we want to invalidate for everything except for PyFunction_EVENT_DESTROY. As that doesn't actually invalidate any of our assumptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok ty!

Yes you need to create your own one with its own ID similar to type_watcher_callback.

Just one more thing on the callback (sorry!),
type_watcher_callback calls PyType_Unwatch, and so I'm guessing I also need to create some sort of PyFunc_Unwatch?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be PyFunction_ClearWatcher (check Objects/funcobject.c)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found it thx!

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