-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
GH-118095: Make BINARY_SUBSCR_GETITEM suitable for tier 2 #120793
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-118095: Make BINARY_SUBSCR_GETITEM suitable for tier 2 #120793
Conversation
I think I see the issue. When projecting, we have special handling for So I think for this to work, we need to add a cache entry to every |
@brandtbucher Thanks for the analysis |
It looks like we bail out of projection in |
Well, that was a bug. But not the bug that's causing this to fail. |
This failure seems interesting:
|
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.
It's a shame that we need to fish the method out of the type twice, but otherwise this is a good, straightforward change. Just one question about the other change being smuggled in alongside it:
I just kicked off JIT benchmarks and stats for you. |
This reverts commit 2af2a91.
2% fewer tier one instructions. Overall perf impact is in the noise, but two SymPy benchmarks got 15% faster. |
Makes
BINARY_SUBSCR_GETITEM
suitable for tier 2, including fix for #118540