-
Notifications
You must be signed in to change notification settings - Fork 13
Support simultaneous ptx and lib building (multiple builds in a single builder) #5
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
base: main
Are you sure you want to change the base?
Conversation
This is a different solution by making the builder reusable instead of using multiple thread pools like #3 |
|
@ivarflakstad Hi Ivar, are you able to review this PR? I believe the PR at huggingface/candle#3221 depends on this. |
| authors = ["Nicolas Patry <patry.nicolas@protonmail.com>"] | ||
| name = "bindgen_cuda" | ||
| version = "0.1.5" | ||
| version = "0.1.7" |
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.
| version = "0.1.7" | |
| version = "0.1.5" |
We can increment the version in a separate PR when creating a new release.
| true | ||
| }; | ||
| let ccbin_env = std::env::var("NVCC_CCBIN"); | ||
| let nvcc_binary = if std::path::Path::new("/usr/local/cuda/bin/nvcc").exists() { |
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.
Nit: Could create a utility ala get_nvcc_binary and avoid the repetition
The current version does not support multiple builder instances and a build instance cannot be used for another build (as also mentioned in #2 ), I have fixed this by making the builder reusable, e.g., building ptx after lib building.
The tested use case: