Skip to content

Use CCCL overflow utils instead of custom overflow logic - #22593

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
PointKernel:use-cccl-overflow
May 21, 2026
Merged

Use CCCL overflow utils instead of custom overflow logic#22593
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
PointKernel:use-cccl-overflow

Conversation

@PointKernel

@PointKernel PointKernel commented May 19, 2026

Copy link
Copy Markdown
Member

Description

Uncovered this cleanup opportunity while working on #22465.

This PR replaces cudf's custom overflow handling logic with the overflow utilities provided by CCCL. Note that although these utilities are under the cuda:: namespace, they are available on both host and device.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@PointKernel PointKernel added the libcudf Affects libcudf (C++/CUDA) code. label May 19, 2026
@PointKernel
PointKernel requested a review from a team as a code owner May 19, 2026 23:49
@PointKernel PointKernel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 19, 2026
@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors overflow detection across cuDF by replacing manual numeric_limits-based comparisons with CUDA's native overflow intrinsics. Five files now include <cuda/numeric> and use cuda::add_overflow, cuda::sub_overflow, cuda::div_overflow, and cuda::mul_overflow to detect arithmetic overflow with their .overflow result member.

Changes

Overflow Detection Refactoring

Layer / File(s) Summary
Fixed-point integer overflow helpers
cpp/include/cudf/fixed_point/fixed_point.hpp
Copyright year updated to 2026. Added <cuda/numeric> include. Four overflow helper functions (addition_overflow, subtraction_overflow, division_overflow, multiplication_overflow) refactored to use CUDA intrinsics' .overflow member instead of manual numeric_limits range checks.
Device aggregators SUM_WITH_OVERFLOW overflow detection
cpp/include/cudf/detail/aggregation/device_aggregators.cuh
Header include changed from cuda/std/limits to cuda/numeric. Removed type_max/type_min constants from update_target_element<Source, aggregation::SUM_WITH_OVERFLOW> specialization. Overflow detection after atomic add now uses cuda::add_overflow<DeviceType>(...).overflow.
Bitmask null_mask cumulative overflow check
cpp/src/bitmask/null_mask.cu
Added <cuda/numeric> include. In set_null_masks, cumulative word overflow detection during pinned-vector tabulation replaced with cuda::add_overflow-based check.
ORC decompression output size overflow check
cpp/src/io/orc/orc.cpp
Added <cuda/numeric> include. In orc_decompressor::decompress_blocks, worst-case output size computation now uses cuda::add_overflow<size_t> instead of manual numeric_limits-based bounds check.
Parquet byte bounds filtering overflow check
cpp/src/io/parquet/reader_impl_helpers.cpp
Added <cuda/numeric> include. In aggregate_reader_metadata::apply_byte_bounds_filter, upper-bound overflow check for bytes_to_skip + bytes_to_read changed to cuda::add_overflow<size_t>(...) instead of comparing against numeric_limits::max().

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: replacing custom overflow handling with CCCL utilities.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly describes the changeset: replacing custom overflow handling with CCCL overflow utilities across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

auto const is_within_byte_range =
std::cmp_greater_equal(row_group_file_offset, bytes_to_skip) and
(not bytes_to_read.has_value() or
cuda::add_overflow<size_t>(bytes_to_skip, bytes_to_read.value()).overflow or

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signpost for the next reviewer, this line is the meaningful change in this PR. The rest is just clang-format.

@mhaseeb123 mhaseeb123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Also TIL cuda::xx_overflow<>().overflow cccl utils

@PointKernel

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 0d8d49b into NVIDIA:main May 21, 2026
115 checks passed
@PointKernel
PointKernel deleted the use-cccl-overflow branch May 21, 2026 21:35
@coderabbitai coderabbitai Bot mentioned this pull request Jun 9, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants