Skip to content

Conversation

@trxcllnt
Copy link
Contributor

Never cache the outer CUDA compilation (because nvcc -E can't be trusted).

Always decompose via nvcc --dryrun, then cache and report the host compiler call as a CUDA compilation.

Fixes #2299.

…usted). Always decompose via `nvcc --dryrun`, then cache and report the host compiler call as a CUDA compilation
Comment on lines +464 to +477
Ok((
command,
None,
// Never assume the outer `nvcc` call is cacheable. We must decompose the nvcc call into
// its constituent subcommands with `--dryrun` and only cache the final build product.
//
// Always decomposing `nvcc --dryrun` is the only way to ensure caching nvcc invocations
// is fully sound, because the `nvcc -E` preprocessor output is not sufficient to detect
// all source code changes.
//
// Specifically, `nvcc -E` always defines __CUDA_ARCH__, which means changes to host-only
// code guarded by an `#ifndef __CUDA_ARCH__` will _not_ be captured in `nvcc -E` output.
Cacheable::No,
))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might now be able to get away with doing less in the preprocess() function, since we effectively don't care about most of what it does.

@sylvestre sylvestre merged commit 709309e into mozilla:main Dec 21, 2024
59 checks passed
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (0cc0c62) to head (a03b43c).
Report is 123 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #2300       +/-   ##
==========================================
- Coverage   30.91%       0   -30.92%     
==========================================
  Files          53       0       -53     
  Lines       20112       0    -20112     
  Branches     9755       0     -9755     
==========================================
- Hits         6217       0     -6217     
+ Misses       7922       0     -7922     
+ Partials     5973       0     -5973     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@trxcllnt trxcllnt mentioned this pull request Dec 23, 2024
wdvr added a commit to pytorch/pytorch that referenced this pull request Jan 16, 2025
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Jan 17, 2025
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Jan 17, 2025
sccache 0.9.1 should be dealing with `nvcc -E` correctly

see mozilla/sccache#2300

If this works as expected, we can get rid of this code:
https://github.com/pytorch/pytorch/pull/142813/files

Pull Request resolved: #145012
Approved by: https://github.com/malfet
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.

sccache incorrectly yields hits for code guarded by #ifndef __CUDA_ARCH__

3 participants