GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||
GeezerVison is showing github.com — gh-131798: Split `FOR_ITER_GEN` into smaller uops by Sacul0457 · Pull Request #148372 · python/cpython Conversation
|
Uh oh. you just caught a bug in the JIT optimizer. Would you like to fix it? The problem is that this line is wrong https://github.com/python/cpython/blob/main/Python/optimizer_symbols.c#L807 it should not be returning PyGen_Type, but rather NULL. Instead, _Py_uop_sym_get_probable_type should be the one returning PyGen_Type. If you want to fix it, just move the lines around amend this small test to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sure! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Please open an issue and fix it in that PR. Thank you. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@Sacul0457 this might not be worth it, but splitting up _CHECK_AND_ALLOCATE_OBJECT into the CHECK and _ALLOCATE should be worth it (we can remove the redundant checks through same thing as _GUARD_TYPE_VERSION). I just merged a PR into main that should make this easier. Would you like to work on that instead? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sure :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hi @Fidget-Spinner! op(_CHECK_OBJECT, (type_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) { PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); EXIT_IF(!PyStackRef_IsNull(self_or_null)); // ... // ... EXIT_IF(FT_ATOMIC_LOAD_UINT32_RELAXED(tp->tp_version_tag) != type_version); } Then Then in optimizer_bytecodes.c: op(_CHECK_OBJECT, (type_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) { PyObject *probable_callable = sym_get_probable_value(callable); PyTypeObject *tp = (PyTypeObject *)probable_callable; if (tp->tp_version_tag == type_version && sym_is_not_null(self_or_null)) { // If the type version has not changed since we last saw it, // then we know this __init__ is definitely the same one as in the cache. ADD_OP(_NOP, 0, 0); } else { // do what _GUARD_TYPE_VERSION did in its else case sym_set_non_null(ctx, callable); sym_set_non_null(ctx, self_or_null); } } |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@Sacul0457 you need to do similar to what we're currently doing in _GUARD_TYPE_VERSION, which is to set the type watcher if it doesn't match, and leave the first type check around. The refactored uop looks correct assuming you copied everything in correctly. Please open a PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.