Skip to content
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

[mypyc] Add lowered primitive for unsafe list get item op #18136

Merged
merged 6 commits into from
Dec 10, 2024

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Nov 10, 2024

This inlines the list get item op in loops like for x in <list>.

I estimated the impact using two microbenchmarks that iterate over list[int] objects. One of them was 1.3x faster, while the other was 1.09x faster.

Since we now generate detailed IR for the op, instead of using a C primitive function, this also opens up further IR optimization opportunities in the future.

This inlines the list get item op in loops like `for x in <list>`.

I estimated the impact using two microbenchmarks that iterate over
`list[int]` objects. One of them was 1.3x faster, while the other
was 1.09x faster.

Since we now generate detailed IR for the op, instead of using a
C primitive function, this also opens up further IR optimization
opportunities in the future.
@JukkaL
Copy link
Collaborator Author

JukkaL commented Nov 11, 2024

This seems broken on 32-bit platforms. I may need to tweak some integer sizes.

def list_item_ptr(builder: LowLevelIRBuilder, obj: Value, index: Value, line: int) -> Value:
"""Get a pointer to a list item (index must be valid and non-negative).

Type of index must be c_pyssize_t_rprimitive.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be useful to add a doc/comment about the internal representation of lists.
I assume it's a just a list of pointers, and that's why we assume it's pyssize_t per element. And we translate &list[index] to list + size_t*index. (or something similar to this)

@JukkaL JukkaL merged commit 568648d into master Dec 10, 2024
13 checks passed
@JukkaL JukkaL deleted the mypyc-lower-get-item branch December 10, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants