Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make rapids_cpm_cccl() disable CUB detail namespace #673

Open
wants to merge 1 commit into
base: branch-24.10
Choose a base branch
from

Conversation

trxcllnt
Copy link
Contributor

Description

Define CUB_DISABLE_NAMESPACE_MAGIC and CUB_IGNORE_NAMESPACE_MAGIC_ERROR so CUB doesn't include the __CUDA_ARCH_LIST__ in its ABI.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@trxcllnt trxcllnt requested a review from a team as a code owner August 13, 2024 20:41
@trxcllnt trxcllnt changed the title Make rapids_cpm_ccl() disable CUB detail namespace Make rapids_cpm_cccl() disable CUB detail namespace Aug 13, 2024
@trxcllnt trxcllnt added improvement Improves an existing functionality non-breaking Introduces a non-breaking change 3 - Ready for Review Ready for review by team labels Aug 13, 2024
@vyasr
Copy link
Contributor

vyasr commented Aug 19, 2024

@robertmaynard can you take a look here? I've lost track of what decisions we've made where about keeping or removing this particular ABI mangling. I know it's caused headaches with differences between host and device compilation of the same headers.

@robertmaynard
Copy link
Contributor

We know that for sure that projects the build with -fvisibility=hidden will have no impact since the CUB mangling is a fix for clashes in the global symbol table lookup.

My major concern is that by fully disabling the CUB detail magic logic we also drop the CUB_VERSION from the namespace name. That I think is important as it means that we could clash with DSO built with older ( 1.X ) versions of CUB which didn't have any name mangling. This issue currently exists with the Thrust types but we sorta ignore it for now.

In a perfect world we could tell CCCL to drop the CUDA_ARCH mangling but leave the VERSION mangling. That would give us reproducible builds and also ensure we don't clash at runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants