KEMBAR78
[mypyc] Add lowered primitive for unsafe list get item op by JukkaL · Pull Request #18136 · python/mypy · GitHub
Skip to content

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