Skip to content

fix(nvvm): cast bit-counting intrinsic results back to the return width#403

Open
nh13 wants to merge 1 commit into
Rust-GPU:mainfrom
nh13:nh_fix-bit-count-intrinsic-result-cast
Open

fix(nvvm): cast bit-counting intrinsic results back to the return width#403
nh13 wants to merge 1 commit into
Rust-GPU:mainfrom
nh13:nh_fix-bit-count-intrinsic-result-cast

Conversation

@nh13

@nh13 nh13 commented Jul 22, 2026

Copy link
Copy Markdown

Fixes #401.

core::intrinsics::{ctpop, ctlz, cttz, ctlz_nonzero, cttz_nonzero} are declared as returning u32 for every operand width, while the LLVM intrinsics they lower to return a value as wide as their operand. rustc_codegen_llvm casts the result back (the intcast(ret, result.layout.llvm_type(self), false) in each arm of compiler/rustc_codegen_llvm/src/intrinsic.rs); this backend never has.

The oversized value was then stored into the 4-byte result slot. For u64/u128 that store is out of bounds for the slot, so it is discarded, the intrinsic call becomes dead, and the device silently computes 0. For u8/u16 the store is in bounds and the high bytes of the result are left uninitialized. Width 32 was the only correct width, because there the cast is a no-op.

This is easy to see in the emitted PTX. Before this change, the entire body of

#[kernel]
pub unsafe fn count_ones_u64(buffer: *const u64, out: *mut u32) {
    unsafe { *out = (*buffer).count_ones(); }
}

is gone -- no popc, and not even the store:

.visible .entry count_ones_u64(
    .param .u64 count_ones_u64_param_0,
    .param .u64 count_ones_u64_param_1
)
{
    ret;
}

After, the popc.b64 is there and the result is stored, and the resulting SASS is a correct 64-bit population count (POPC / POPC / IADD3 / STG.E).

The fix

Rather than wrapping each of the five arms individually (and handle_128_bit_intrinsic, which is a free function with no result in scope), the whole arm is wrapped in a single intcast to the intrinsic's result layout. Casting to an identical type is a no-op, so the intrinsics in that arm which do return the operand width -- bswap, bitreverse, rotate_*, saturating_* -- are unaffected, and any intrinsic added there later is covered automatically.

Tests

Three dis compiletests pin the emitted PTX:

  • count_ones_u64 -- must contain popc.b64
  • leading_zeros_u64 -- must contain clz.b64
  • count_ones_u128 -- exercises the separate emulated handle_128_bit_intrinsic path; must contain two popc.b64 plus the combining add

All three compiled down to a bare ret before this change. They pass -Cdebuginfo=0 so the goldens do not embed library/core line numbers, which would otherwise churn on every rust-toolchain.toml bump.

Verified in ghcr.io/rust-gpu/rust-cuda-ubuntu24-cuda12: full compiletest suite green across compute_61,compute_75,compute_90 (66 passed), and cargo fmt --all -- --check plus cargo clippy -p rustc_codegen_nvvm clean with RUSTFLAGS=-Dwarnings.

Also corrects the 128-bit row in guide/src/features.md, which claimed the bit-manipulation intrinsics were unsupported -- they are emulated over the two 64-bit halves, and with this fix they return correct results.

🤖 Generated with Claude Code

`core::intrinsics::{ctpop, ctlz, cttz, ctlz_nonzero, cttz_nonzero}` are
declared as returning `u32` for every operand width, while the LLVM
intrinsics they lower to return a value as wide as their operand.
`rustc_codegen_llvm` casts the result back; this backend did not.

The oversized value was then stored into the 4-byte result slot. For
`u64`/`u128` that store is out of bounds for the slot and is discarded, the
intrinsic call becomes dead, and the device silently computes `0` -- the
emitted PTX contains no `popc`/`clz` at all. For `u8`/`u16` the store is in
bounds, so the high bytes of the result are left uninitialized. Only width
32 was correct, because there the cast is a no-op.

Wrap the whole arm in a single `intcast` to the intrinsic's result layout
rather than casting in each of the five arms. Casting to an identical type
is a no-op, so the intrinsics in this arm that do return the operand width
(`bswap`, `bitreverse`, `rotate_*`, `saturating_*`) are unaffected, and any
intrinsic added here later is covered automatically.

Add `dis` compiletests pinning the emitted PTX for `u64::count_ones`
(`popc.b64`), `u64::leading_zeros` (`clz.b64`), and the emulated
`u128::count_ones` (two `popc.b64` plus the combining `add`). Before this
change all three kernels compiled down to a bare `ret`.

Also correct the 128-bit row in the feature table, which claimed the
bit-manipulation intrinsics were unsupported.

Fixes Rust-GPU#401

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

ctpop/ctlz/cttz results are never cast back to the operand width — u64::count_ones() returns 0 on device

1 participant