Skip to content

gh-148380: remove all uses of _PyType_LookupByVersion in optimizer_bytecodes.c#148394

Open
NekoAsakura wants to merge 2 commits intopython:mainfrom
NekoAsakura:gh-148380/remove-LookupByVersion
Open

gh-148380: remove all uses of _PyType_LookupByVersion in optimizer_bytecodes.c#148394
NekoAsakura wants to merge 2 commits intopython:mainfrom
NekoAsakura:gh-148380/remove-LookupByVersion

Conversation

@NekoAsakura
Copy link
Copy Markdown
Contributor

@NekoAsakura NekoAsakura commented Apr 11, 2026

I couldn't add _RECORD_TOS to LOAD_ATTR_CLASS_WITH_METACLASS_CHECK since the recording slot's already taken by _RECORD_TOS_TYPE.

first = True
for part in macro.uops:
match part:
case parser.OpName():
if part.name == "flush":
parts.append(Flush())
else:
if part.name not in uops:
raise analysis_error(
f"No Uop named {part.name}", macro.tokens[0]
)
uop = uops[part.name]
if uop.properties.records_value and not first:
raise analysis_error(
f"Recording uop {part.name} must be first in macro",
macro.tokens[0])

So there's a minor optimisation regression on the metaclass path, but not much we can do about it.
As for naming, it's a right mix in the file, so I just went with what felt consistent.

@Fidget-Spinner
Copy link
Copy Markdown
Member

So there's a minor optimisation regression on the metaclass path,

Oh dear, thanks for pointing that out. That's the one for enum.attr optimization, so we can't really make that worse.

I suggest we wait for #148378 to be done and then do that?

@NekoAsakura
Copy link
Copy Markdown
Contributor Author

Right, hold off then. Give my a kick once #148378 lands :)

PyObject *type = sym_get_probable_value(owner);
if (type != NULL && ((PyTypeObject *)type)->tp_version_tag == type_version) {
if (type == sym_get_const(ctx, owner)) {
ADD_OP(_NOP, 0, 0);
Copy link
Copy Markdown
Member

@Fidget-Spinner Fidget-Spinner Apr 11, 2026

Choose a reason for hiding this comment

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

Btw, It think this is bugged on main, there needs to be a watcher installed on the type for this path similar to _GUARD_TYPE_VERSION. would you like to open a PR to fix it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure! Which issue should I link to?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you. I think it's fine to open a new issue describing the bug.

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.

2 participants