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

Revert "Add CUDA_STATIC_MATH_LIBRARIES" #192

Conversation

KyleFromNVIDIA
Copy link
Contributor

@KyleFromNVIDIA KyleFromNVIDIA commented Jul 12, 2024

#190 was supposed to separate static CUDA math libraries from static CUDA runtime library, but accidentally pulled the runtime along with the math libraries. The way we'd normally fix this is by creating a separate variable for the runtime. However, since this project doesn't actually use any math libraries, we can just revert the whole thing.

Contributes to rapidsai/build-planning#35

@KyleFromNVIDIA KyleFromNVIDIA requested a review from a team as a code owner July 12, 2024 15:59
@KyleFromNVIDIA KyleFromNVIDIA added bug Something isn't working non-breaking Introduces a non-breaking change labels Jul 12, 2024
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I'm fine reverting since there's no use of math libs, but could you clarify what you mean by

accidentally pulled the runtime along with the math libraries.

Was there some problem caused by #190, somewhere where we lost the runtime static linking altogether?

@KyleFromNVIDIA
Copy link
Contributor Author

The problem caused by #190 was that the static linking of the runtime was suddenly controlled by CUDA_STATIC_MATH_LIBRARIES rather than CUDA_STATIC_RUNTIME. My intention at first was to have the runtime controlled by CUDA_STATIC_RUNTIME again, but when I reviewed the code and realized that nothing else was actually using CUDA_STATIC_MATH_LIBRARIES, I decided it would be best to just revert the entire change.

@vyasr
Copy link
Contributor

vyasr commented Jul 15, 2024

I was looking at the call to rapids_cuda_init_runtime (which is using the right variable) and I didn't realize that the suffix was affecting the cudart library we link to. Got it, yes this is a necessary fix. Thanks.

@KyleFromNVIDIA
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit ea6ea26 into rapidsai:branch-24.08 Jul 15, 2024
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants