-
Notifications
You must be signed in to change notification settings - Fork 570
Support nvcc --device-debug
flag
#2384
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
cc: @robertmaynard for review |
Can you take a look into the CI tests. This might have some overlap with the CI failures in #2382 |
@drahnr the integration tests are failing due to this: https://github.blog/changelog/2025-03-20-notification-of-upcoming-breaking-changes-in-github-actions/#decommissioned-cache-service-brownouts, which is being addressed in #2385 |
@trxcllnt could you rebase your PRs since the CI fixup landed in |
…ts can run in parallel
These changes ensure cache hits for compilations which are subsets of previously cached compilations * Normalize cudafe++, ptx, and cubin names regardless of whether the compilation flag is `-c`, `-ptx`, `-cubin`, or whether there are one or many `-gencode` flags * Include the compiler `hash_key` in the output dir for internal nvcc files to guarantee stability and uniqueness * Fix cache error due to hash collision from not hashing all the PTX and cubin flags
add more multi-arch tests to ensure combining cached/new PTX and cubins doesn't produce corrupted objects
…nding deprecation
1960f9b
to
29cc48d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2384 +/- ##
==========================================
- Coverage 71.41% 69.50% -1.92%
==========================================
Files 65 45 -20
Lines 36349 26611 -9738
==========================================
- Hits 25960 18496 -7464
+ Misses 10389 8115 -2274 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
m.update(cwd.join(output).as_os_str().as_encoded_bytes()); | ||
m.finish() | ||
}); | ||
fs::create_dir_all(&out_dir).ok(); |
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.
The above line is odd. Do we not want to bail if the IO operation fails?
|
||
#[allow(unused)] | ||
impl SccacheClient { | ||
pub fn new_no_cfg() -> Self { |
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.
Could use a comment or two
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.
I have a few small questions, it's still a significant changeset to review
The changes in this PR are related to the Do you want to review and merge each of these separately, or do everything in this PR? |
4535e5a
to
63e9d72
Compare
63e9d72
to
4765772
Compare
4765772
to
9d4e3c9
Compare
I want to review them separately, that was the intent of splitting and allowing me to review given my scattered availability 🫠 |
This PR extracts the changes to support
nvcc --device-debug
from #2356 per @drahnr's request.This PR branched from/is a follow-up to #2383. See the diff of the two branches here.
This PR adds support for
nvcc -G
/nvcc --device-debug
. Presently-G
is interpreted as a GCC argument that expects a numeric value. nvcc doesn't support that, so instead it should be intercepted and passed to thenvcc --dryrun
commands.