Skip to content

Use size_t for byte count in device attributes#6151

Merged
davebayer merged 1 commit intoNVIDIA:mainfrom
davebayer:use_size_t_for_num_bytes_dev_attrs
Oct 8, 2025
Merged

Use size_t for byte count in device attributes#6151
davebayer merged 1 commit intoNVIDIA:mainfrom
davebayer:use_size_t_for_num_bytes_dev_attrs

Conversation

@davebayer
Copy link
Contributor

No description provided.

@davebayer davebayer self-assigned this Oct 8, 2025
@davebayer davebayer requested a review from a team as a code owner October 8, 2025 11:23
@davebayer davebayer requested a review from griwes October 8, 2025 11:23
@github-project-automation github-project-automation bot moved this to Todo in CCCL Oct 8, 2025
@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Review in CCCL Oct 8, 2025
@davebayer davebayer requested review from pciolkosz and removed request for griwes October 8, 2025 11:23
@fbusato
Copy link
Contributor

fbusato commented Oct 8, 2025

I'm not against it, but I would like to understand the motivation behind using size_t. The shared memory space is 32-bit; I'm not sure it's worth moving the attributes to 64-bit. I'm thinking of use cases where users experience worse performance (on the device) due to implicit promotion.

@fbusato fbusato self-requested a review October 8, 2025 15:56
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2025

🥳 CI Workflow Results

🟩 Finished in 4h 31m: Pass: 100%/239 | Total: 8d 07h | Max: 4h 30m | Hits: 80%/302280

See results here.

@davebayer
Copy link
Contributor Author

I'm not against it, but I would like to understand the motivation behind using size_t. The shared memory space is 32-bit; I'm not sure it's worth moving the attributes to 64-bit. I'm thinking of use cases where users experience worse performance (on the device) due to implicit promotion.

Whenever you have number of bytes in C++, it's data type is size_t, the type returned by sizeof operator. I believe we should be consistent with what's commonly used.

I understand the point about performance, but I don't believe anyone would use any of these parameters in performance critical path, but tell me if I'm wrong :)

And the returned values are known at compile time so I don't believe there will be any overhead in the end

Copy link
Contributor

@fbusato fbusato left a comment

Choose a reason for hiding this comment

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

the fact that they are compile-time values mitigates the issue. In the worst case, the user can do int{attribute}.

@davebayer davebayer merged commit 050b57c into NVIDIA:main Oct 8, 2025
251 of 252 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Oct 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2025

Backport failed for branch/3.1.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin branch/3.1.x
git worktree add -d .worktree/backport-6151-to-branch/3.1.x origin/branch/3.1.x
cd .worktree/backport-6151-to-branch/3.1.x
git switch --create backport-6151-to-branch/3.1.x
git cherry-pick -x 050b57c3199088674e0d7ddabdcdd32001a4c032

davebayer added a commit to davebayer/cccl that referenced this pull request Oct 14, 2025
davebayer added a commit to davebayer/cccl that referenced this pull request Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants