Repository navigation
Conversation
The first five steps of the population-count reduction relied on implicit warp lock-step with only volatile shared memory and no synchronization. On the targeted Volta/Turing+ architectures (Independent Thread Scheduling) lock-step is not guaranteed, making the read-after-write across lanes a data race that can corrupt the partial sums and thus the candidate list. Separate each step with __syncwarp(), and add one after the initial per-lane counts are stored, before the first step reads them (as in NVIDIA's warp-synchronous examples). Register use of the kernels is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MarkRose
force-pushed
the
fix/bitcount-syncwarp
branch
from
October 3, 2026 19:54
6508479 to
542474c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The population count in
create_k_deltas()(gpusieve_helper.cu) is an inclusive prefix sum over shared memory. Its first five steps stay within a warp and relied on the warp running in lock-step, with only avolatilearray and no synchronization. On Volta and newer (Independent Thread Scheduling), which covers every architecture mfaktc targets, that's no longer guaranteed. A lane can read a neighbour's partial sum before it's written, or after it's been overwritten by the next step. The result would be a wrong prefix sum: overlapping or missing candidate ranges, i.e. missed factors, or a wrong total.This adds
__syncwarp()between the steps, and one after the initial per-lane counts are stored, before the first step reads them (as in NVIDIA's warp-synchronous examples). All 32 lanes reach every barrier (256 threads per block, no early exits), so the default full mask is right.A static review of the CUDA kernels for races found this to be the only warp-synchronous code in mfaktc (no shuffles, ballots or votes are used) and judged the fix complete.
Draft until it has been run on a GPU (Turing or newer): the self-test and a GPU-sieve throughput comparison.
Testing
There's no NVIDIA GPU here; built against CUDA 13.4 (nvcc 13.4.92) for all architectures in the Makefile with no new warnings. Register use of the GPU-sieve kernels is unchanged (sm_89).
Conflicts with #133 in the same lines (the
+=rewrite); I'll rebase this after #133 merges.🤖 Generated with Claude Code