Uh oh!
There was an error while loading. Please reload this page.
gh-131798: Split CHECK_FUNCTION_VERSION into two guards in the JIT - #145080
gh-131798: Split CHECK_FUNCTION_VERSION into two guards in the JIT#145080Sacul0457 wants to merge 11 commits into
CHECK_FUNCTION_VERSION into two guards in the JIT#145080Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
37e58ef to
e775a6bCompareSacul0457
commented
Mar 21, 2026
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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).
There was a problem hiding this comment.
Sure, Ill open an issue and the PR in a bit
There was a problem hiding this comment.
Hi, I was just taking a look at this issue and I've got a few questions:
- We add a
PyFunction_AddWatcheronly if thefunc_versionwas set properly, like what_GUARD_TYPE_VERSIONversion does? - If so, what
PyFunction_WatchCallbackdo I add toPyFunction_AddWatcheror am I suppose to create one similar totype_watcher_callback
I'll continue to look into this in the meantime, thx! :)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
It should be PyFunction_ClearWatcher (check Objects/funcobject.c)
Sacul0457
commented
Apr 9, 2026
Is this optimization still possible/correct with #146300? |
This PR is stale because it has been open for 30 days with no activity. |
This PR splits up
_CHECK_FUNCTION_VERSIONinto_GUARD_CALLABLE_FUNCTIONand itself. This allows the optimizer to remove thePyFunction_Checkcheck 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_KWtoo but I left that out to keep the change smaller.