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

Implement @depositBits and @extractBits #18680

Closed
wants to merge 28 commits into from

Conversation

ominitay
Copy link
Contributor

Implements @depositBits and @extractBits, corresponding to pdep and pext from BMI2. The builtins are supported in the LLVM backend for any integer width, are supported in the x86 backend for integer widths below and including 64 bits, and are evaluated at compile-time when possible (also at any integer width). The functionality remains unimplemented in all other backends, and is out of the scope of this PR.

Apologies for how long it has taken me to find the time to get this merge-ready, but I'm happy for it to be merged now, and any further enhancements to be split out into further issues.

Continuation of #15285
Closes #14995

@ominitay
Copy link
Contributor Author

All checks passed, happy for this to be merged if approved :)

@ominitay
Copy link
Contributor Author

Rebased onto master.

@ominitay
Copy link
Contributor Author

Converted to draft as there are a couple of changes I'm part-way through making to this PR. Should be ready for review in a week or two.

@ominitay ominitay marked this pull request as draft March 22, 2024 03:17
@ominitay
Copy link
Contributor Author

Emulation on unsupported targets and large int types has been moved to compiler-rt. Yet to implement calls to this from other backends, but x86 backend should now pass behaviour tests. Before ready for review, I'll look at implementing testing of the various implementations of this against each other to each other with random integers, as we have 4 of them: the two compiler-rt methods (generic and bigint), the comptime bigint, and the native instruction call, so any latent bugs I could have missed should be detected. This is kind of building off of @matu3ba's suggestions: #15285 (comment)

ominitay added 21 commits April 17, 2024 00:20
This change implements depositBits and extractBits (equivalents of PDEP
and PEXT) for Zig's bit ints. This change lays the groundwork for
implementation of `@depositBits` and `@extractBits`.

Tests have been added to check the behaviour of these two functions.

The functions currently don't handle negative values (though negative
values may be converted to twos complement externally), and
aren't optimal in either memory or performance.
Implements std.math.big.int.Mutable.convertFromTwosComplement, to match
convertToTwosComplement.
Incomplete: currently only implemented for 64-bit-or-smaller integers
for x86(-64) in the LLVM backend.
Removes the requirement to copy and modify `mask`, removing the need to
clone `mask` into a `Mutable` bigint.
Implements compiler-rt functions to emulate the PEXT and PDEP instructions from
BMI2. These also implement the same functionality for arbitrarily-big integers.
The existing emulation of these instructions has been removed from the LLVM
backend, and replaced with calls to these compiler-rt functions. Some rework has
been done in the backend to reduce code duplication.
Adds calls into compiler-rt in the x86 backend for depositBits and extractBits.
This brings the x86 backend on-par with the LLVM backend, now fully supporting
these builtins for all targets and integer sizes. Some refactoring has been
applied to reduce code duplication.
Adds a test for u256 to provide some coverage for codegen of __pdep_bigint and
__pext_bigint. Also stops skipping tests on the x86 backend.
Disables the two behaviour tests which are caused to fail on the x86_64 backend
by ziglang#19498. Fixing the underlying issue is not within the scope of this pull request.
@ominitay
Copy link
Contributor Author

Disabled the failing behaviour tests caused by the bug in the x86 backend. My rationale here is that it's not within the scope of this PR to work around, as a user needing to use this functionality could simply just not use the x86 backend.

@andrewrk
Copy link
Member

andrewrk commented Jun 8, 2024

draft status for > 30 days

@andrewrk andrewrk closed this Jun 8, 2024
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.

Add @pext and @pdep builtins
2 participants