Skip to content

msvc: cache C++20 named modules built with clang-cl - #2876

Merged
sylvestre merged 3 commits into
mozilla:mainfrom
Kataglyphis:fix/clang-cl-cxx20-modules
Oct 17, 2026
Merged

sylvestre merged 3 commits into
mozilla:mainfrom
Kataglyphis:fix/clang-cl-cxx20-modules

Conversation

@Kataglyphis

@Kataglyphis Kataglyphis commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

CMake 4.4 builds C++20 modules with clang-cl and passes the module flags bare, in an @<target>.modmap file. On main, -x c++-module made interface units uncacheable, and -fmodule-file left the BMI out of the hash, so importers hit a stale object after their module changed. -x c++-module is cached only together with -fmodule-output=; the -clang:/-Xclang spellings stay refused.

CI has no clang-cl, so I checked it with CMake 4.4.3, Ninja and clang-cl 23.1.1: a clean rebuild hits and restores the BMI; after a module change the importer misses and the program prints the new value.

Refs #2629

🤖 Worked with Claude Code

@codecov-commenter

codecov-commenter commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.19626% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.06%. Comparing base (7bdb9bd) to head (dea2018).

Files with missing lines Patch % Lines
src/compiler/msvc.rs 97.87% 2 Missing ⚠️
src/compiler/gcc.rs 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2876      +/-   ##
==========================================
+ Coverage   78.00%   78.06%   +0.05%     
==========================================
  Files          73       73              
  Lines       43136    43233      +97     
==========================================
+ Hits        33650    33749      +99     
+ Misses       9486     9484       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Kataglyphis
Kataglyphis marked this pull request as draft September 28, 2026 17:39
@Kataglyphis
Kataglyphis force-pushed the fix/clang-cl-cxx20-modules branch 2 times, most recently from 7f2cb36 to cfb2932 Compare September 28, 2026 21:46
@Kataglyphis
Kataglyphis marked this pull request as ready for review September 28, 2026 23:08
Kataglyphis added a commit to Kataglyphis/ANTfrastructure that referenced this pull request Sep 29, 2026
A new compiler-cache-key input restores the cache into the workspace
(.ci-cache), hands the container CI_COMPILER_CACHE and saves it after
the build on pushes. sccache cannot write to a mounted directory on
Windows, so the build script moves the cache into container-local dirs
and back. Not for clang-cl module builds until mozilla/sccache#2876.

Also records that a cache cap below the working set is slower than no
cache (Ballett's release build: 782 s capped vs 533 s uncached).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Kataglyphis added a commit to Kataglyphis/OmniAccelerANT that referenced this pull request Sep 29, 2026
…ger says sccache stores nothing on modules

The hub pin moves 89824691 -> 425f4019: Invoke-CmakeConfigureAndBuild
-DisableSccache now overrides a preset's COMPILER_CACHE, and the hub's
sccache-on-modules page is rewritten from measurement. The other two
commits in the range (Windows ROCm torch source build, the deleted
:latest-cross tags) touch nothing this repo builds.

AGENTS.md section 2 pointed at that page as "it stores nothing there".
The released sccache does store module importers on clang-cl, without
their BMIs, and serves stale objects after an interface edit until
mozilla/sccache#2876 ships.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread src/compiler/msvc.rs Outdated
Some(Clang(s)) => clangs.push(s.clone()),
Some(ClangModuleFile(val)) if is_clang && !val.is_empty() => {
let val = val.to_string_lossy();
extra_hash_files.push(cwd.join(val.split_once('=').map_or(&*val, |(_, p)| p)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the name=path split is also done twice in gcc.rs, could it be a small shared helper?

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.

Good point. Didn't see that. I created a small helper with a proper test.

Kataglyphis and others added 2 commits October 14, 2026 19:46
CMake passes clang-cl's module flags bare: `-x c++-module` made interface
units uncacheable, and `-fmodule-file` left the BMI out of the hash, so an
importer could hit a stale object after its module changed.

Cache `-x c++-module` only together with `-fmodule-output=`, with the BMI
as an output, and hash the BMI of each `-fmodule-file`. A `/Tc` or `/Tp`
input stays uncached with `-x c++-module`, since clang-cl then drops both
module flags.

Refs mozilla#2629

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gcc.rs split `-fmodule-file`'s `name=path` value in two places and
msvc.rs in a third. Move it into one gcc::module_file_path helper,
with a unit test.

A value without `=` now keeps its original OsStr on the clang-cl path
too, as gcc.rs already did, instead of going through to_string_lossy.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Kataglyphis
Kataglyphis force-pushed the fix/clang-cl-cxx20-modules branch from 9cf612d to 87b1d31 Compare October 14, 2026 18:26
Comment thread src/compiler/msvc.rs Outdated
msvc_take_arg!("we", OsString, Concatenated, PassThroughWithSuffix),
msvc_take_arg!("winsysroot", PathBuf, CanBeSeparated, PassThroughWithPath),
msvc_take_arg!("wo", OsString, Concatenated, PassThroughWithSuffix),
msvc_take_arg!("x", OsString, Separated, ClangLanguage),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what about -xc++-module? it would pass through as an unknown flag and the BMI wouldn't be tracked
could it be CanBeSeparated, or refused?

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.

Good catch! -x is now CanBeSeparated, as in gcc.rs (didn't see that before) so -xc++-module is cached only together with -fmodule-output=, like -x c++-module. The test covers both spellings.

`-xc++-module` went through as an unknown flag, and clang-cl writes an
implicit BMI when `-fmodule-output=` is absent, so `-xc++-module m.cpp`
was cached with the object only. `-x` is CanBeSeparated, as in gcc.rs,
so both spellings are cached only together with `-fmodule-output=`.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sylvestre
sylvestre merged commit 1bd7432 into mozilla:main Oct 17, 2026
51 checks passed
@sylvestre

Copy link
Copy Markdown
Collaborator

Thanks for your PR

@Kataglyphis

Copy link
Copy Markdown
Contributor Author

Thanks for your PR

Always happy to contribute. Thanks for the review!

Kataglyphis added a commit to Kataglyphis/ANTfrastructure that referenced this pull request Oct 17, 2026
…X-only nvcc fix

No sccache release carries the two fixes that hold back caching here:
mozilla/sccache#2876 (clang-cl C++20 named modules, merged to main today) and
#2809, the open fix for #2862 (a PTX-only gencode fails with `Missing "cubin"
file output`).

- versions.env pins the source (SCCACHE_SOURCE_REV, main at #2876's merge) and
  SCCACHE_BUILD_ID; shared/upstream/sccache/patches/ holds #2809 as its author
  wrote it.
- build-sccache.sh builds one target in upstream's archive layout and stamps the
  version (`sccache 0.18.0+1bd7432.pr2809`).
- sccache-release.yml builds x86_64/aarch64/riscv64gc musl in upstream's own
  musl cross containers, pinned by digest, plus x86_64-pc-windows-msvc, and
  publishes release sccache-v<id> with SHA256SUMS.

Verified on CUDA 13.2 and an RTX 2080: unpatched main reproduces #2862. With the
patch, cold and warm (14/14 hits) objects of an arch- and define-guarded kernel
equal the bare build's host symbols, SASS and PTX, and run correctly on the GPU.
The installers still take mozilla's 0.18.0; they move once the release exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013nDuz5yv57sH4oVNMjMUNK
Kataglyphis added a commit to Kataglyphis/OmniAccelerANT that referenced this pull request Oct 17, 2026
…e's sccache keys clang-cl modules right

- windows-x64.yml's Release job restores .ci-cache, hands the container
  CI_COMPILER_CACHE and SCCACHE_CACHE_SIZE=2500M, and saves it from develop when
  the build filled it. A prune job (require-saved) keeps one entry, and both
  callers grant actions: write for it.
- Build-Windows.ps1 moves the cache into the container's SCCACHE_DIR, restarts
  the server so its disk level indexes it, and moves it back in finally.
- Test-SccacheModuleSafe gates it: the hub's build (0.18.0+1bd7432.pr2809, with
  mozilla/sccache#2876) or 0.19.0 on. :winamd64's 0.18.0 serves stale clang-cl
  module importers, so until that image is rebuilt the run says so and neither
  uses nor saves the cache.
- Hub -> 0eba4bb5 (both lanes install that build; nvcc, hipcc and Rust go
  through sccache), AccelerANTgine -> 733fbce, OxidANT -> 9ee491f.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013nDuz5yv57sH4oVNMjMUNK
Kataglyphis added a commit to Kataglyphis/ANTfrastructure that referenced this pull request Oct 17, 2026
…X-only nvcc fix

No sccache release carries the two fixes that hold back caching here:
mozilla/sccache#2876 (clang-cl C++20 named modules, merged to main today) and
#2809, the open fix for #2862 (a PTX-only gencode fails with `Missing "cubin"
file output`).

- versions.env pins the source (SCCACHE_SOURCE_REV, main at #2876's merge) and
  SCCACHE_BUILD_ID; shared/upstream/sccache/patches/ holds #2809 as its author
  wrote it.
- build-sccache.sh builds one target in upstream's archive layout and stamps the
  version (`sccache 0.18.0+1bd7432.pr2809`).
- sccache-release.yml builds x86_64/aarch64/riscv64gc musl in upstream's own
  musl cross containers, pinned by digest, plus x86_64-pc-windows-msvc, and
  publishes release sccache-v<id> with SHA256SUMS.

Verified on CUDA 13.2 and an RTX 2080: unpatched main reproduces #2862. With the
patch, cold and warm (14/14 hits) objects of an arch- and define-guarded kernel
equal the bare build's host symbols, SASS and PTX, and run correctly on the GPU.
The installers still take mozilla's 0.18.0; they move once the release exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013nDuz5yv57sH4oVNMjMUNK
Kataglyphis added a commit to Kataglyphis/OmniAccelerANT that referenced this pull request Oct 17, 2026
…e's sccache keys clang-cl modules right

- windows-x64.yml's Release job restores .ci-cache, hands the container
  CI_COMPILER_CACHE and SCCACHE_CACHE_SIZE=2500M, and saves it from develop when
  the build filled it. A prune job (require-saved) keeps one entry, and both
  callers grant actions: write for it.
- Build-Windows.ps1 moves the cache into the container's SCCACHE_DIR, restarts
  the server so its disk level indexes it, and moves it back in finally.
- Test-SccacheModuleSafe gates it: the hub's build (0.18.0+1bd7432.pr2809, with
  mozilla/sccache#2876) or 0.19.0 on. :winamd64's 0.18.0 serves stale clang-cl
  module importers, so until that image is rebuilt the run says so and neither
  uses nor saves the cache.
- Hub -> b9e451da (both lanes install that build; nvcc, hipcc and Rust go
  through sccache), AccelerANTgine -> 8ed2fcf, OxidANT -> 1f9a7b4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013nDuz5yv57sH4oVNMjMUNK
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.

3 participants