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

Change the device library to use CUDA runtime API #589

Merged
merged 5 commits into from
Apr 11, 2019

Conversation

alexrlongne
Copy link
Contributor

@alexrlongne alexrlongne commented Mar 25, 2019

Background

  • Our CUDA device helper classes used a the older CUDA driver API, which is not recommended for new projects. The new runtime API is much simpler and doesn't require explicit management of modules or contexts. It also provides some very nice helper functions to diagnose runtime errors and query the device. This PR changes the GPU_Device helper class to use these newer functions. It also demonstrates how to compile a library of CUDA functions that can be called from either the host or device. To linking to a library of CUDA device functions the target library must be static. This required a slight change to the add_component_library CMake macro.

Purpose of Pull Request

Description of changes

  • Add a simple CUDA compilation step during configure to determine the appropriate architecture flags
  • Allow add_component_library to accept a library type different than the DRACO_LIBRARY_TYPE variable
  • Change the GPU_Device class to use the newer CUDA Runtime API
  • Remove the GPU_Module class (explicit module and contexts are not necessary in the Runtime API)
  • Remove "EXTERN C" decorations (these are also no longer necessary in the Runtime API since functions do not need to be loaded explicitly)
  • Add a test that shows an example of calling code from both the host and device
  • Consolidate calls to simple GPU test kernels into a "basic_kernels" header and implementation file
  • Compile auxiliary GPU test code into a static library callable by the individual gpu tests

Status

@alexrlongne
Copy link
Contributor Author

@KineticTheory @jhchang-lanl I'm open to any idea on how to name GPU tests, libraries, files etc. Just let me know and I'll change it.

@alexrlongne alexrlongne changed the title Changes the device library to use CUDA runtime API Change the device library to use CUDA runtime API Mar 26, 2019
@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

Merging #589 into develop will decrease coverage by 1.6%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           develop    #589     +/-   ##
=========================================
- Coverage     93.7%     92%   -1.7%     
=========================================
  Files          377     364     -13     
  Lines        17665   16933    -732     
=========================================
- Hits         16554   15581    -973     
- Misses        1111    1352    +241

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

Merging #589 into develop will increase coverage by <.1%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           develop    #589     +/-   ##
=========================================
+ Coverage     93.7%   93.7%   +<.1%     
=========================================
  Files          377     380      +3     
  Lines        17665   17791    +126     
=========================================
+ Hits         16554   16673    +119     
- Misses        1111    1118      +7

+ Add a simple CUDA compilation step during configure to determine the apporiate arhcitecture flags
+ Allow add_componenet_library to accept a library type different than the DRACO_LIBRARY_TYPE variable
+ Change the GPU_Device class to use the newer CUDA Runtime API
+ Remove the GPU_Module class (explicit module and contexts are not necessary in the Runtime API)
+ Remove "EXTERN C" decorations (these are also no longer necessary in the Runtime API since functions do not need to be loaded explicity)
+ Add a test that shows an example of calling code from both the host and device
+ Consolidate calls to simple GPU test kernels into a "basic_kernels" header and implementation file
+ Compile auxillary GPU test code into a static library callable by the individual gpu tests
Copy link
Collaborator

@KineticTheory KineticTheory left a comment

Choose a reason for hiding this comment

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

Overall, this looks pretty good. In fact, we might use the planning meeting on Monday to read through this code. It will be instructive for all of us.

config/vendor_libraries.cmake Outdated Show resolved Hide resolved
config/vendor_libraries.cmake Outdated Show resolved Hide resolved
config/vendor_libraries.cmake Outdated Show resolved Hide resolved
config/vendor_libraries.cmake Outdated Show resolved Hide resolved
config/vendor_libraries.cmake Outdated Show resolved Hide resolved
src/device/test/Dual_Call.cu Show resolved Hide resolved
src/device/test/GPU_Device.cmake Show resolved Hide resolved
src/device/test/basic_kernels.hh Show resolved Hide resolved
src/device/test/gpu_device_info.cu Outdated Show resolved Hide resolved
src/device/test/gpu_device_info.cu Outdated Show resolved Hide resolved
@KineticTheory
Copy link
Collaborator

@KineticTheory @clevelam Is email from gitlab working

@KineticTheory
Copy link
Collaborator

KineticTheory commented Apr 4, 2019

Some weirdness...

Preprocessing /home/travis/Draco/src/device/GPU_Device.hh:55: \
error: included file device/test/gpu_hello_driver_api.cc is not found. \
Check your EXAMPLE_PATH (warning treated as error, aborting now)

I'll help you fix this one.

@alexrlongne
Copy link
Contributor Author

@KineticTheory I've tried to address all of your comments and the Travis CI is passing. This should be ready for merge once the CDash is clean.

@KineticTheory KineticTheory merged commit fbfef0d into lanl:develop Apr 11, 2019
@KineticTheory KineticTheory mentioned this pull request Jun 17, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants