Skip to content

Perftest: fix compiling with CUDA 13#376

Open
Yue-ByteDance wants to merge 3 commits intolinux-rdma:masterfrom
Yue-ByteDance:yzl/fix-cuda-13
Open

Perftest: fix compiling with CUDA 13#376
Yue-ByteDance wants to merge 3 commits intolinux-rdma:masterfrom
Yue-ByteDance:yzl/fix-cuda-13

Conversation

@Yue-ByteDance
Copy link
Copy Markdown
Contributor

  • cuda.h only defines CUDA_VERSION but not CUDA_VER, change it to the correct macro.
  • Add CI for checking compiling with CUDA 13

As cuda.h only defines CUDA_VERSION but not CUDA_VER

Signed-off-by: Zelong Yue <yuezelong@bytedance.com>
Signed-off-by: Zelong Yue <yuezelong@bytedance.com>
printf("creating CUDA Ctx\n");

/* Create context */
#if CUDA_VER >= 13000
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(a) Here is another CUDA_VER that should be CUDA_VERSION (haven't searched if there are furhter ones), and (b) should your new CUDA 13 CI testing have caught this?

(I'm not an actual user -- this is just a drive-by comment; I found this PR here while researching solutions for similar CUDA 13 cuCtxCreate breakage in https://github.com/SourceryTools/nvptx-tools.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, thanks for the review! I think the CI didn't catch it because configure.ac defines CUDA_VER (extracted from CUDA_VERSION in cuda.h), so both macros end up with the same value during the CI build. But I agree that's not ideal — if the CUDA toolkit on the system is upgraded without re-running ./configure, CUDA_VER would be stale and the wrong preprocessor branch would be taken.

As for the compatibility question, I think we could use cuCtxCreate_v2 directly instead of relying on the unversioned cuCtxCreate symbol, which would let us keep compatibility between CUDA 12 and CUDA 13 without needing the #if altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants