Make CUDA includes as system includes

[SunBlack]

Currently CUDA headers are not treated as system includes via CMake, but they should.

[jasjuang]

Something like

target_include_directories(${PROJECT_NAME} SYSTEM PUBLIC ${CUDA_INCLUDE_DIRS})

should do.

[SunBlack]

It's not this easy.

Reason: Include directory is already added via FindCUDA script of CMake

function(CUDA_ADD_CUDA_INCLUDE_ONCE)
  get_directory_property(_include_directories INCLUDE_DIRECTORIES)
  set(_add TRUE)
  if(_include_directories)
    foreach(dir ${_include_directories})
      if("${dir}" STREQUAL "${CUDA_INCLUDE_DIRS}")
        set(_add FALSE)
      endif()
    endforeach()
  endif()
  if(_add)
    include_directories(${CUDA_INCLUDE_DIRS})
  endif()
endfunction()

macro(CUDA_ADD_LIBRARY cuda_target)

  CUDA_ADD_CUDA_INCLUDE_ONCE()
  ...
endmacro()

macro(CUDA_ADD_EXECUTABLE cuda_target)

  CUDA_ADD_CUDA_INCLUDE_ONCE()
  ...
endmacro()

Tried already:
Variant 1:

macro(PCL_CUDA_ADD_LIBRARY _name _component)
  ...
  include_directories(SYSTEM ${CUDA_INCLUDE_DIRS})
  cuda_add_library(${_name} ${PCL_LIB_TYPE} ${ARGN})
  ...
endmacro()

Idea: If include directory is already present as system include, CUDA_ADD_CUDA_INCLUDE_ONCE will not add it again. But this does not work, because get_directory_property(_include_directories INCLUDE_DIRECTORIES) does not return SYSTEM include directories.

Variant 2:

macro(PCL_CUDA_ADD_LIBRARY _name _component)
  ...
  cuda_add_library(${_name} ${PCL_LIB_TYPE} ${ARGN})
  ...

  get_property(cuda_target_include_dirs DIRECTORY PROPERTY INCLUDE_DIRECTORIES)
  list(REMOVE_ITEM cuda_target_include_dirs ${CUDA_INCLUDE_DIRS})
  set_property(DIRECTORY PROPERTY INCLUDE_DIRECTORIES ${cuda_target_include_dirs})
  target_include_directories(${_name} SYSTEM PUBLIC ${CUDA_INCLUDE_DIRS})
  ...
endmacro()

But non system variant is already bound to target. I can read all bound include directories via
get_property(cuda_target_include_dirs2 TARGET ${_name} PROPERTY INCLUDE_DIRECTORIES), but I believe in case I call set_property with filtered include directories, I will destroy all other system includes.

Any other good idea?

[jasjuang]

The problem you are facing is because pcl is still using the old approach for CUDA in CMake, there is a modern approach to this.

The modern approach:

cmake_minimum_required(VERSION 3.11)

project(sample LANGUAGES CXX CUDA)

set(PROJECT_SRCS ${PROJECT_SOURCE_DIR}/Sample.cu)
set(PROJECT_INCS ${PROJECT_SOURCE_DIR}/Sample.cuh)

find_package(CUDA REQUIRED)

add_library(${PROJECT_NAME} ${PROJECT_SRCS} ${PROJECT_INCS})

target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_14)

target_include_directories(${PROJECT_NAME} SYSTEM PUBLIC ${CUDA_INCLUDE_DIRS})

target_link_libraries(${PROJECT_NAME} PUBLIC ${CUDA_LIBRARIES})

As you can see the modern approach treats CUDA just like any other external libraries, you have total control of it, whereas the old approach will look like

cmake_minimum_required(VERSION 3.11)

project(sample)

set(PROJECT_SRCS ${PROJECT_SOURCE_DIR}/Sample.cu)
set(PROJECT_INCS ${PROJECT_SOURCE_DIR}/Sample.cuh)

find_package(CUDA REQUIRED)

cuda_add_library(${PROJECT_NAME} ${PROJECT_SRCS} ${PROJECT_INCS})

target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_14)

as you can see the CUDA includes and libs are added implicitly, and there's no way for the downstream to treat the includes and links as PUBLIC/PRIVATE/INTERFACE, or choose to add SYSTEM or not. Hence CMake eventually revamped how CUDA should be used in CMake. Do you think it's possible to change the CMakeLists in pcl to adopt the modern approach?

[SunBlack]

@modern approach: I know this variant. We are using it in our project, but it requires CMake 3.9, but we can only raise minimum version to 3.5, because of ubuntu 16.04. Nevertheless: Your code is still to complicated. You don't even need

find_package(CUDA REQUIRED)
target_include_directories(${PROJECT_NAME} SYSTEM PUBLIC ${CUDA_INCLUDE_DIRS})
target_link_libraries(${PROJECT_NAME} PUBLIC ${CUDA_LIBRARIES})

It's enough to:

cmake_minimum_required(VERSION 3.9)

project(sample LANGUAGES CXX CUDA)

set(PROJECT_SRCS ${PROJECT_SOURCE_DIR}/Sample.cu)
set(PROJECT_INCS ${PROJECT_SOURCE_DIR}/Sample.cuh)

add_library(${PROJECT_NAME} ${PROJECT_SRCS} ${PROJECT_INCS})

target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_14)

Do you think it's possible to change the CMakeLists in pcl to adopt the modern approach?

Nope, because you need to trigger precompile CUDA files before normal compiler is running. So calling add_library will be never enough. Maybe it would be possible if we know a variant to get all targets and check if there is any cuda code,... but I don't believe it is worth.
I suggest to copy the shipped CMake code of cuda_add_library into PCL_CUDA_ADD_LIBRARY and fix include_directories call there.

I created a issue at CMake repo. Event it won't help us, in case they fix it, but maybe they have an idea for a workaround.

[jasjuang]

If we have to make it work with older CMake then I don't really know what to do at this point.

You are right, in this minimal the target_include_directories and target_link_libraries are actually not needed but I tried taking out target_include_directories and target_link_libraries in my project and it yields compilation error. The reason is typically in a project there will be a mixture of cpp and cu files.

If the cpp file has something like #include <cuda_runtime_api.h> in it, without the target_include_directories and target_link_libraries there will be compilation error like

fatal error: cuda_runtime_api.h: No such file or directory
 #include <cuda_runtime_api.h>
          ^~~~~~~~~~~~~~~~~~~~

[stale]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[kunaltyagi]

Pinging @haritha-j and @shrijitsingh99 who might run into issues due to this

[stale]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[capeie-dev]

were you able to solve this error?

[SunBlack]

I checked it short: Seems not. On MSVC CUDA is included via /I"C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.7\include" instead of /external:I "C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.7\include", during the other dependencies are correct included (checked only the depenencies of pcl_gpu_containers).