Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
gh-131798: JIT: Further optimize_CALL_ISINSTANCE
for class tuples#134543
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
_CALL_ISINSTANCE
for class tuples_CALL_ISINSTANCE
for class tuplesself.assertIn("_BUILD_TUPLE",uops) | ||
self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW",uops) |
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.
_BUILD_TUPLE
is preventing us from optimizing out_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW
.
The bytecode is basically:
LOAD_CONSTLOAD_CONST_BUILD_TUPLE_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW
To optimize this, we'd need some special handling for_BUILD_TUPLE
inremove_unneeded_uops
.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
When you're done making the requested changes, leave the comment: |
So that was a lie 😆 Anyway, I think I addressed all your comments! :) I have made the requested changes; please review again |
Thanks for making the requested changes! @brandtbucher: please review the changes made to this pull request. |
Uh oh!
There was an error while loading.Please reload this page.
We can already const eval
isinstance(inst, cls)
calls when both arguments are known, but only ifcls
is a single class (e.g.int
).This PR adds support for
isinstance(inst, (cls1, cls2, ..., clsN))
. This allows us to optimize for example:isinstance(42, (int, str))
(toTrue
)isinstance(42, (bool, str))
(toFalse
)We can narrow to
True
even when only some of the classes are known, as long asinst
is an instance of one of the known classes.