-
-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-119258: Handle STORE_ATTR_WITH_HINT
in tier two
#119481
Conversation
STORE_ATTR_WITH_HINT
in tier twoSTORE_ATTR_WITH_HINT
in tier two
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hoping to remove STORE_ATTR_WITH_HINT
once almost all instance have inline values.
Until then, this seems like a worthwhile improvement.
One question and a couple of suggestions.
@@ -246,7 +246,7 @@ dict_next_version(PyInterpreterState *interp) | |||
((INTERP)->dict_state.global_version += DICT_VERSION_INCREMENT) | |||
#endif | |||
|
|||
void | |||
PyAPI_FUNC(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this symbol need to be exposed?
It isn't used by the JIT.
It is used, via the _PyDict_NotifyEvent
inline function.
Python/bytecodes.c
Outdated
@@ -2135,10 +2135,8 @@ dummy_func( | |||
_GUARD_DORV_NO_DICT + | |||
_STORE_ATTR_INSTANCE_VALUE; | |||
|
|||
inst(STORE_ATTR_WITH_HINT, (unused/1, type_version/2, hint/1, value, owner --)) { | |||
op(_STORE_ATTR_WITH_HINT, (hint/1, value, owner --)) { | |||
PyTypeObject *tp = Py_TYPE(owner); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyTypeObject *tp = Py_TYPE(owner); |
This is only used in the assert, AFAICT, so will generate a compiler warning for non-debug builds.
Python/bytecodes.c
Outdated
PyTypeObject *tp = Py_TYPE(owner); | ||
assert(type_version != 0); | ||
DEOPT_IF(tp->tp_version_tag != type_version); | ||
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); | |
assert(Py_TYPE(owner)->tp_flags & Py_TPFLAGS_MANAGED_DICT); |
When you're done making the requested changes, leave the comment: |
This way we can remove the version check in the optimizer, too!