Skip to content

Remove fast_int inheritance from dynamic valid extents - #845

Open
MaybeWilli wants to merge 7 commits into
NVIDIA:devfrom
MaybeWilli:Remove-fastint-from-extent
Open

MaybeWilli wants to merge 7 commits into
NVIDIA:devfrom
MaybeWilli:Remove-fastint-from-extent

Conversation

@MaybeWilli

@MaybeWilli MaybeWilli commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Fixes #838.

Removed dynamic valid_extent specialization, because as of #836 / #837, fast_int's modulo utility is no longer needed by valid_extent. Made valid_extent inherit from extent, allowing valid_extent to reuse extent's existing static/dynamic behavior. valid_extent.value() is inlined with no additional instructions (verified by PTX analysis). Changed struct valid_extent to class valid_extent.

@copy-pr-bot

copy-pr-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Comment thread include/cuco/detail/extent/extent.inl Outdated
struct valid_extent<SizeType, dynamic_extent> : cuco::utility::fast_int<SizeType> {
using value_type =
typename cuco::utility::fast_int<SizeType>::fast_int::value_type; ///< Extent value type
struct valid_extent<SizeType, dynamic_extent> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the cleanup! Could we also remove the unused fast_int.cuh include and the stale TODO use fast_int operator in probing_scheme_impl.inl? The PR description should reference #836 / #837 instead of #827.

@PointKernel PointKernel added the type: improvement Improvement / enhancement to an existing function label Sep 16, 2026
@MaybeWilli MaybeWilli changed the title [WIP] Remove fast_int inheritance from dynamic valid extents Remove fast_int inheritance from dynamic valid extents Sep 17, 2026
@MaybeWilli
MaybeWilli marked this pull request as ready for review September 17, 2026 02:32
@PointKernel

Copy link
Copy Markdown
Member

/ok to test 974d6a5

Comment thread include/cuco/detail/extent/extent.inl Outdated
struct valid_extent<SizeType, dynamic_extent> : cuco::utility::fast_int<SizeType> {
using value_type =
typename cuco::utility::fast_int<SizeType>::fast_int::value_type; ///< Extent value type
struct valid_extent<SizeType, dynamic_extent> {

@sleeepyjack sleeepyjack Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we can simplify the implementation even more:
Now that the fast_int member has been removed, valid_extent is functionally the same as extent, but only allows for private construction through friend factories (make_valid_extent). There's also no longer a need for disallowing implicit conversions to SizeType.

The cleanest option is valid_extent inheriting from the corresponding extentdirectly:

template <typename SizeType, std::size_t N>
class valid_extent : public extent<SizeType, N> {
  using base_type = extent<SizeType, N>;

 public:
  using value_type = typename base_type::value_type;

  constexpr value_type value() const noexcept
  {
    return base_type::operator value_type();
  }

 private:
  explicit constexpr valid_extent(value_type value = {}) noexcept
    : base_type{value}
  {
  }

  // Factory friends
};

This would:

  • Delete the dynamic valid_extent specialization.
  • Reuse extent’s static/dynamic storage behavior.
  • Preserve private construction and explicit conversion.
  • Preserve empty-base optimization for static extents.
  • Keep the shared arithmetic operators unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: improvement Improvement / enhancement to an existing function

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT]: Remove fast_int storage from dynamic valid extents

3 participants