-
Notifications
You must be signed in to change notification settings - Fork 123
fix compile for CUDA 13 #169
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes compilation issues with CUDA 13.0.0 on Ubuntu 24.04 by addressing three specific compatibility problems in the PopSift codebase.
- Removes obsolete
texture_fetch_functions.h
include that is no longer available in CUDA 13 - Migrates from deprecated
thrust::identity
tocuda::std::identity
for Thrust >= 3.0.0 - Resolves macro conflicts with NVTX functions by renaming local macro definitions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/popsift/s_extrema.cu | Removes deprecated texture_fetch_functions.h include |
src/popsift/s_filtergrid.cu | Adds Thrust version compatibility and fixes NVTX macro conflicts |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Thanks for the PR! |
Great! I've removed the parts that conflict with and are overcome by #162. |
Merged! Thanks for your contribution! |
Description
This PR includes the changes that allow PopSift to compile on Ubuntu 24.04 with CUDA 13.0.0.
Features list
#include <texture_fetch_functions.h>
. This include files is no longer included with the latest CUDA Toolkit, and appears to be unused by PopSift.thrust::identity
tocuda::std::identity
(required for Thrust version >= 3.0.0).Fix macro conflict withnvtxRangePop()
(It looks like this fix will be overcome by First-order CUDA follow-up fix: do not use NVTX #162 or Guard calls to nvtx directly #168).Implementation remarks
Re: 'ntvxRangePushA(a)' andntvxRangePop()
: The definition of these above#include <thrust/copy.h>
wreaks havoc on the definition of these functions innvtx3.hpp
include indirectly viathrust/copy.h
. I changed their name for this file, but another approach could be to move the#define
blow thethrust
includes. It seems like it was probably placed above thethrust
includes for a reason though. The change to these macros will be overcome by #162 or #168.