[CUB] Add fast constant division utility - #11304
Conversation
fbusato
left a comment
There was a problem hiding this comment.
We already have it cuda::fast_mod_div https://nvidia.github.io/cccl/unstable/libcudacxx/extended_api/math/fast_mod_div.html#libcudacxx-extended-api-math-fast-mod-div
📝 SummarySummary by CodeRabbit
WalkthroughAdds ChangesConstant Division
Suggested reviewers: Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The divider is mergeable, but large divisors may unnecessarily reduce CUDA performance by using full-width division where a comparison would suffice. The duplication concern is maintainability follow-up rather than a current runtime blocker. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cub/cub/detail/fast_modulo_division.cuh (2)
206-212: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winsuggestion: replace
mode::hardwarewith a comparison.
log2_divisor == bitsimpliesdivisor > 2^(bits-1), so the quotient is only 0 or 1. Line 259 then issues a full-width integer division, which is expensive on device for 64-bit operands.fast_div_modalready handles this case with a comparison (lines 343-347).identity, shift, multiply_shift, - hardware + compare };const int log2_divisor = ceil_log2(divisor); if (log2_divisor == bits) { magic_ = divisor; shift_ = 0; - mode_ = mode::hardware; + mode_ = mode::compare; return; }- if (mode_ == mode::hardware) + if (mode_ == mode::compare) { - return numerator / magic_; + return numerator >= magic_ ? UInt{1} : UInt{0}; }Source: Path instructions
122-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffsuggestion: the header now contains two independent magic-multiplier dividers.
fast_divide_by_constantandfast_div_modduplicate divisor classification, magic computation, and multiply-high logic. Consider buildingfast_div_modon top of the new class, or document why the two must stay separate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 35981c9c-0bd1-4d07-9c54-c4f78cb360a6
📒 Files selected for processing (2)
cub/cub/detail/fast_modulo_division.cuhcub/test/internal/catch2_test_fast_divide_by_constant.cu
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| [[nodiscard]] _CCCL_HOST_DEVICE _CCCL_FORCEINLINE static int ceil_log2(UInt divisor) noexcept | ||
| { | ||
| return divisor <= UInt{1} ? 0 : bits - ::cuda::std::countl_zero(divisor - UInt{1}); | ||
| } |
There was a problem hiding this comment.
We have cuda::ceil_ilog2
| return divisor <= UInt{1} ? 0 : bits - ::cuda::std::countl_zero(divisor - UInt{1}); | ||
| } | ||
|
|
||
| [[nodiscard]] _CCCL_HOST_DEVICE _CCCL_FORCEINLINE static UInt multiply_high(UInt lhs, UInt rhs) noexcept |
| mode_ = mode::identity; | ||
| return; | ||
| } | ||
| if ((divisor & (divisor - UInt{1})) == UInt{0}) |
There was a problem hiding this comment.
| if ((divisor & (divisor - UInt{1})) == UInt{0}) | |
| if (cuda::is_power_of_two(divisor)) |
Why
The cooperative histogram work in #10568 introduced a small precomputed divider so an invariant unsigned divisor does not require a hardware integer division for every sample. Review requested that this generally useful facility live in CUB's internal utilities and be reviewed independently of both #10568 and the benchmark work in #10555.
Keeping the divider separate also gives its arithmetic edge cases direct coverage. In particular, divisors above half of the integer range cannot use the normal magic-number setup because constructing its numerator would require shifting by the full width of the widened type.
What changed
cub::detail::fast_divide_by_constantto the existingcub/detail/fast_modulo_division.cuhinternal utility header.cuda::std::countl_zeroand fixed-widthcuda::stdinteger types. The 64-bit host path also retains a portable limb-based multiply-high and precomputation fallback for toolchains without 128-bit integers.For example, repeatedly dividing values by
3,10, or2^63 - 1uses precomputed state, while a divisor such as2^63 + 1safely selects the hardware fallback. Divisors0and1return the numerator unchanged, matching the extracted facility's disabled/identity state.Tests
Added focused host and device tests for
uint32_tanduint64_t. They compare every result against the corresponding exact quotient across:max - 1andmax;Validation was run after rebasing onto current
upstream/main(4b8e07fac3f8acc78712ee14da73eed3c99338c4):pre-commit run --files cub/cub/detail/fast_modulo_division.cuh cub/test/internal/catch2_test_fast_divide_by_constant.cualso passed.Scope
This PR intentionally does not modify DeviceHistogram implementation, dispatch, tuning, benchmarks, or tests. PR #10568 is removing its local copy of this optimization and does not depend on this PR. Any future algorithm adoption of this utility will be reviewed separately; this branch is not stacked on either #10568 or #10555.