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

RFC: Array.usize #4654

Closed
fgdorais opened this issue Jul 5, 2024 · 2 comments · Fixed by #4801 or #4802
Closed

RFC: Array.usize #4654

fgdorais opened this issue Jul 5, 2024 · 2 comments · Fixed by #4801 or #4802
Labels
P-low We are not planning to work on this issue RFC accepted RFC is waiting for a corresponding PR (external or internal) RFC Request for comments

Comments

@fgdorais
Copy link
Contributor

fgdorais commented Jul 5, 2024

Proposal

I propose to add Array.usize to get the USize size of an array, without going through Nat.

Currently, to get the USize of an array, one needs to use Array.size and then Nat.toUSize. The latter takes the return value of Array.size and calculates the remainder modulo USize.size. This is not optimal since arrays are stored in memory and therefore cannot exceed USize.size. This proposal would bypass the remainder step and return the correct array size as an USize scalar (which is trivial to access in the current array implementation).

Other array operations have optimizations that bypass this trivial size check for the sake of nearly matching C-language performance. Some of my code at UnicodeBasic and elsewhere could benefit a lot from this nearly trivial improvement to the array API.

The drawback is that this adds one more unsafe implementation. So this request needs some careful consideration.

Impact

Add 👍 to issues you consider important. If others benefit from the changes in this proposal being added, please ask them to add 👍 to it.

@fgdorais fgdorais added the RFC Request for comments label Jul 5, 2024
@Kha
Copy link
Member

Kha commented Jul 5, 2024

This proposal would bypass the remainder step

Surely this step is optimized out by LLVM anyway?

@fgdorais
Copy link
Contributor Author

fgdorais commented Jul 5, 2024

Of course, no actual division is done but it's not quite optimized out. The issue is that the m_size component of a lean_array_object cannot be larger than LEAN_MAX_SMALL_NAT but LLVM doesn't know that which leads to overly complex code. On the small branch, the only one actually used, LLVM does optimize the box/unbox to an (unnecessary!) and to clear the top bit. Anyway, the end result much more complex than what should simply return the m_size component of the array.

@Kha Kha added RFC accepted RFC is waiting for a corresponding PR (external or internal) P-low We are not planning to work on this issue labels Jul 19, 2024
fgdorais added a commit to fgdorais/lean4 that referenced this issue Jul 21, 2024
Add efficient usize functions for Array, ByteArray, FloatArray.

Closes leanprover#4654
github-merge-queue bot pushed a commit that referenced this issue Jul 21, 2024
Add efficient `usize` functions for `Array`, `ByteArray`, `FloatArray`.

This is part 1 of 2 since there is a need to update stage0 between the
two parts. (See discussion below.)

Closes #4654
github-merge-queue bot pushed a commit that referenced this issue Jul 21, 2024
This is part 2 of 2 of #4801 (which closes #4654). That PR was split in
two to allow a stage0 update between declaring the `usize` functions and
using them where they are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low We are not planning to work on this issue RFC accepted RFC is waiting for a corresponding PR (external or internal) RFC Request for comments
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants