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

Expose LLVM's or disjoint instruction on ints #373

Closed
alion02 opened this issue Apr 24, 2024 · 5 comments
Closed

Expose LLVM's or disjoint instruction on ints #373

alion02 opened this issue Apr 24, 2024 · 5 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@alion02
Copy link

alion02 commented Apr 24, 2024

Proposal

Problem statement

When writing heavy bit-manipulating code you sometimes need to combine integers bitwise according to the following truth table:

a b combine(a, b)
0 0 0
0 1 1
1 0 1
1 1 never happens

The above truth table can be implemented using any of the following:

  • bitor
  • bitxor
  • add and friends

Which one you choose can have an impact on the performance of downstream code. Occasionally, you might want to exercise properties of more than one of these operations at the same time.

Motivating examples or use cases

Consider the construction of a mask like so: let c = combine(a, b << 2);. This comes up fairly often when dealing with packed data, for instance. Using + for combine can result in a single instruction on x86: lea c, [a + b * 4]. On the other hand, using | allows LLVM to apply bitwise optimizations, such as remembering more known bits or tracking tighter bounds on the range of c.

In projects which commonly utilize this operation, it would be beneficial to have a canonical way to write it. Due to the above concerns, however, this isn't always feasible, and a user attempting to maximize performance must take into account the tradeoffs every time.

Solution sketch

Expose LLVM's or disjoint instruction, which is the canonical way to write the above truth table. This takes the optimization burden off the programmer and shoves it onto LLVM, which is much better equipped to handle it.

  1. Add the function core::intrinsics::disjoint_or, following the example of core::intrinsics::unchecked_add and friends.
  2. Expose a function on all signed and unsigned (and NonZero?) integers, like so:
/// Unchecked bitwise merging of disjoint sets of set bits. This operation is equivalent to
/// `self | rhs`, `self ^ rhs`, and `self + rhs`, and assumes that all of those operations
/// produce the same result; that is, that there are no pairs of merged bits where both
/// bits are set.
/// 
/// This function frequently allows the optimizer to better reason about the operation than
/// any of the other operations which can be used to emulate it do.
/// 
/// The safe alternatives are the aforementioned operators.
/// 
/// # Safety
/// 
/// This results in undefined behavior if any pair of merged bits has both bits set.
#[must_use]
#[inline(always)]
pub const unsafe fn unchecked_bitmerge(self, rhs: Self) -> Self {
    // SAFETY: the caller must uphold the safety contract for `unchecked_bitmerge`.
    unsafe { intrinsics::disjoint_or(self, rhs) }
}

Alternatives

  • Do nothing. The lack of a canonical operation continues. Some of the performance benefits can be gained manually using already available tools through repetitive and tedious labor. Performance characteristics are different between architectures and possibly even microarchitectures, which makes this an exceedingly poor cost-benefit effort.
  • Improve LLVM to the point that this function can be perfectly expressed in user code with the help of unreachable_unchecked. LLVM does not recognize this right now. I assumed this is a very complicated problem - if it's not, then perhaps this is a better choice. I'm surprised this wasn't brought up in the unchecked math stabilization PR.

Links and related work

Internals thread. Blog post about the introduction of the feature to LLVM.

@alion02 alion02 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Apr 24, 2024
@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Apr 30, 2024
@joshtriplett
Copy link
Member

Discussed in today's libs-api meeting. Approved, but please use disjoint_bitor for the intrinsic and unchecked_disjoint_bitor for the non-intrinsic function.

@joboet
Copy link

joboet commented May 2, 2024

unchecked_disjoint_bitor conflicts with the API guidelines. Could I suggest naming it disjoint_bitor_unchecked instead?

@programmerjake
Copy link
Member

tbh that naming seems kinda backwards to me, I prefer bitor_disjoint_unckecked

@RustyYato
Copy link

But unchecked first makes it consistent with the recently stabilized unchecked_{add,sub,mul} methods on integers. Which seems more valuable

@Amanieu
Copy link
Member

Amanieu commented May 14, 2024

Closing since this ACP has been approved, discussion should move to the PR/tracking issue.

@Amanieu Amanieu closed this as completed May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

6 participants