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

[FEA] Make it easier to mark import system includes as non-system includes #95

Open
robertmaynard opened this issue Oct 13, 2021 · 3 comments
Labels
0 - Backlog In queue waiting for assignment feature request New feature or request

Comments

@robertmaynard
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Projects such as cudf, and rmm require newer versions of Thrust, libcuc++ than can be found in the oldest supported CUDA toolkit. This requires these components to install/packaged so that consumers use the same version.

This becomes challenging with CMake+NVCC due to the rules around import targets, and system includes and the default install layout packages like Thrust use.

For example when a projects installs Thrust to a conda env you end up with a layout that looks like:

conda/env/include/rmm
conda/env/include/thrust
conda/env/include/cub 

This means that any project that specifies conda/env/include as a system path will capture thrust as a system path. Since CMake treats any include path specified on an import target as SYSTEM this causes the thrust inside conda to be a system path as a side-effect.

So why does that matter? Well compilers have specific rules on the order that includes are searched for headers. In general user includes are searched before any system includes going in the order provided. The issue is that NVCC injects the CUDA toollkit includes into the system include search order as the first entry. Therefore stopping discovery of another system included thrust.

Describe the solution you'd like

rapids-cmake should provide a general way to mark a targets includes as non-system with some INCLUDE|EXCLUDE regex filter

@robertmaynard
Copy link
Contributor Author

Correction:

NVCC implicitly adds the CUDA toolkit include directory as the last user include, not the first system include.
This is important as NVCC / GCC have specific rules that if the same include is specified as system and user, it is treated as system.

That means if we can get CMake to specify the CUDA include as the last system include on the compile line we can ensure that a conda installed Thrust / libcuc++ will be selected instead.

@robertmaynard
Copy link
Contributor Author

Currently CMake 3.23 is on track to provide a feature to make it easier for config.cmake files to mark a targets include as non-system ( https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6627 ).

This would allow us to hang the thrust include on an INTERFACE target and mark that as non-system, while having the projects normal includes still be system. This won't solve the problem of the common conda layout and we would still need to install thrust into something like:

  conda/env/include/thrust/thrust/
  conda/env/lib/cmake/thrust/  

@robertmaynard robertmaynard added 0 - Backlog In queue waiting for assignment and removed ? - Needs Triage Need team to review and classify labels Oct 15, 2021
@robertmaynard
Copy link
Contributor Author

robertmaynard commented Oct 15, 2021

Currently CMake 3.23 is on track to provide a feature to make it easier for config.cmake files to mark a targets include as non-system ( https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6627 ).

I think adding a wrapper around this is the best approach and should be doable for 22.02

Since we have #97 and #98 I don't think we have a need for a general solution for 21.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Backlog In queue waiting for assignment feature request New feature or request
Projects
Status: No status
Development

No branches or pull requests

1 participant