FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Use __builtin_bswapg in cuda::std::byteswap when available by Functionhx · Pull Request #9785 · NVIDIA/cccl · GitHub

/ cccl Public

Use __builtin_bswapg in cuda::std::byteswap when available - #9785

Merged
Jacobfaib merged 1 commit into
NVIDIA:mainfrom
Functionhx:fix/bswapg-builtin
Jul 10, 2026
ConversationCommits (1)ChecksFiles changed
Merged

Use __builtin_bswapg in cuda::std::byteswap when available#9785
Jacobfaib merged 1 commit into
NVIDIA:mainfrom
Functionhx:fix/bswapg-builtin

Conversation

Functionhx commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #7778.

Functionhx requested a review from a team as a code owner July 10, 2026 10:14
Functionhx requested a review from Jacobfaib July 10, 2026 10:14
github-project-automation Bot moved this to Todo in CCCL Jul 10, 2026

copy-pr-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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.

cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Jul 10, 2026
Jacobfaib reviewed Jul 10, 2026
View reviewed changes
Comment thread libcudacxx/include/cuda/std/__bit/byteswap.h Outdated
if constexpr (sizeof(_Integer) > 1)
{
#if defined(_CCCL_BUILTIN_BSWAPG)
return static_cast<_Integer>(_CCCL_BUILTIN_BSWAPG(::cuda::std::__to_unsigned_like(__val)));

Jacobfaib Jul 10, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

__builtin_bswapg is supposed to support any integer, do we still need the cast to unsigned?

Also on this note, we should add support for the sized builtins as well, __builtin_bswapg was only added in clang 22 (not sure if gcc even has it to begin with). Before that, __builtin_bswap{16, 32, 64} and possibly __builtin_bswap128 have existed for longer.

Jacobfaib commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/ok to test

copy-pr-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/ok to test

@Jacobfaib, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

Jacobfaib commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/ok to test bb4ef02

This comment has been minimized.

Sign in to view

Functionhx commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Will do — I'll verify the unsigned cast is redundant and add the sized builtin fallback. Thanks for the review.

Functionhx force-pushed the fix/bswapg-builtin branch from bb4ef02 to 198d99a Compare July 10, 2026 16:38

Jacobfaib commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/ok to test 198d99a

Jacobfaib reviewed Jul 10, 2026
View reviewed changes
Comment thread libcudacxx/include/cuda/std/__bit/byteswap.h Outdated
#if defined(_CCCL_BUILTIN_BSWAPG)
return static_cast<_Integer>(_CCCL_BUILTIN_BSWAPG(__val));
#else // ^^^ _CCCL_BUILTIN_BSWAPG ^^^ / vvv !_CCCL_BUILTIN_BSWAPG vvv
if constexpr (sizeof(_Integer) == 2)

Jacobfaib Jul 10, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Don't hardcode the size here, I would say sizeof(_Integer) == sizeof(::cuda::std::uint16_t) since that is what we ultimately care about in the end.

Comment thread libcudacxx/include/cuda/std/__bit/byteswap.h Outdated
if constexpr (sizeof(_Integer) == 2)
{
# if defined(_CCCL_BUILTIN_BSWAP16)
return static_cast<_Integer>(_CCCL_BUILTIN_BSWAP16(static_cast<uint16_t>(__val)));

Jacobfaib Jul 10, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
return static_cast<_Integer>(_CCCL_BUILTIN_BSWAP16(static_cast<uint16_t>(__val)));
return static_cast<_Integer>(_CCCL_BUILTIN_BSWAP16(static_cast<::cuda::std::uint16_t>(__val)));

Unfortunately we must fully qualify all types and calls like this. It's not so strictly required for types, but better safe than sorry

Functionhx commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both follow-up comments in 708003e: replaced the hardcoded size checks with comparisons against the corresponding fully qualified fixed-width types, and fully qualified the cast types in all three sized-builtin branches. Local pre-commit and the targeted std/numerics/bit/byteswap.pass.cpp lit test both pass.

Jacobfaib reviewed Jul 10, 2026
View reviewed changes
Comment thread libcudacxx/include/cuda/std/__bit/byteswap.h Outdated
Use __builtin_bswapg in cuda::std::byteswap
98b083b 
Signed-off-by: Yuchen Fan <functionhx@gmail.com>
Functionhx force-pushed the fix/bswapg-builtin branch from 708003e to 98b083b Compare July 10, 2026 17:08

Jacobfaib commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/ok to test 98b083b

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 59b56197-3676-4122-a9bd-67e8adccaaa8

📥 Commits

Reviewing files that changed from the base of the PR and between 0fa6cea and 98b083b.

📒 Files selected for processing (1)
  • libcudacxx/include/cuda/std/__bit/byteswap.h
📝 Walkthrough

Summary by CodeRabbit

  • Performance
    • Improved byte-swapping for supported integral types by using an optimized compiler-provided operation when available.
    • Preserved existing fallback behavior across supported compilation environments.

Walkthrough

cuda::std::byteswap detects builtin_bswapg, disables it for NVCC device compilation, and uses it for integral types larger than one byte when available.

Changes

Byteswap builtin integration

Layer / File(s) Summary
Builtin selection and byteswap dispatch
libcudacxx/include/cuda/std/__bit/byteswap.h		 Preprocessor detection and device guards add _CCCL_BUILTIN_BSWAPG; byteswap uses the builtin for multi-byte integers and retains the existing fallback.

Assessment against linked issues

Objective Addressed Explanation
Use __builtin_bswapg in cuda::std::byteswap [#7778] ✅

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

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 1h 52m: Pass: 100%/120 | Total: 1d 21h | Max: 1h 19m | Hits: 100%/356615

See results here.

Jacobfaib merged commit a6d373c into NVIDIA:main Jul 10, 2026
139 of 140 checks passed
github-project-automation Bot moved this from In Review to Done in CCCL Jul 10, 2026

Jacobfaib commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Thanks for the patch!

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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Jacobfaib Jacobfaib approved these changes

Assignees

No one assigned

Labels

None yet

Projects

CCCL
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEA]: Use __builtin_bswapg in cuda::std::byteswap

2 participants

Back | FazBrowse Home | New Git URL