gcc, clang: preprocess multi-arch compilations once per -arch by francoisgirinon-netizen · Pull Request #2871 · mozilla/sccache

francoisgirinon-netizen · 2026-09-24T14:40:15Z

With SCCACHE_CACHE_MULTIARCH set, a universal build (clang -arch x86_64 -arch arm64) was preprocessed once, with the -arch flags replaced by -D__x86_64__=1 -D__arm64__=1, and that output went into the cache key. The real target macros (__SSE2__, __ARM_NEON, __aarch64__...) come from the host triple in that pass, so code guarded by them is invisible to the key: editing only one slice's code gives the same key and a hit hands back a stale fat object. We hit this in CI as an arm64 slice with an empty symbol table and Undefined symbols for architecture arm64 at link time.

The preprocessor now runs once per distinct -arch, in command line order, the way ccache does it, and every pass's output goes into the key behind a #pragma sccache arch <arch> <len> separator. Single-arch and arch-less compilations are untouched, so their keys don't move and CACHE_VERSION stays put. The depfile ends up as the last arch's, which is what clang itself writes for a multi-arch compile.

Multi-arch compilations are no longer distributed: the dist command never forwarded arch_args, so a remote multi-arch compile already produced a single-arch object.

Preprocessor cache entries recorded by the old single-pass code would still point at stale objects after an upgrade, so their key now carries a marker for multi-arch compilations only; single-arch entries stay valid.

This implements the per-arch preprocessing asked for in #847; caching multi-arch compilations stays opt-in through SCCACHE_CACHE_MULTIARCH.

sylvestre · 2026-09-24T15:14:18Z

    }
+
+    /// The distinct architectures given with `-arch`, in command line order.
+    pub fn archs(&self) -> Vec<&OsString> {


could be .filter(..).unique().collect() with itertools, no?

Done, archs() now uses .unique(), and it moved to the commit that first uses it.

sylvestre · 2026-09-24T15:14:30Z

+        archs
+    }
+
+    pub fn is_multiarch(&self) -> bool {


this allocates a vec every time and it's called several times per compile
maybe archs().len() > 1 once and store it?

Done, is_multiarch() no longer allocates: it compares the remaining archs against the first one.

sylvestre · 2026-09-24T15:14:39Z

+            output.stdout.push(b'\n');
+        }
+        output.status = pass.status;
+        output.stderr = pass.stderr;


this drops the stderr of the earlier passes, so x86_64 warnings are lost, no?
could you please append instead?

Good catch, the stderr of every pass is now kept, including the earlier passes' when a later one fails.

sylvestre · 2026-09-24T15:14:46Z

+        );
+        let creator = new_creator();
+        let seen = Arc::new(Mutex::new(vec![]));
+        for _ in 0..4 {


why 4? please use a constant or add a short comment

Done, the test now queues exactly one mocked pass per expected arch.

sylvestre · 2026-09-24T20:19:44Z

+        #[cfg(target_os = "windows")]
+        let expected = "c06235f5ae05c5382ae4c327977f24a976392abd40e422f08c680082e1703adf";
+        #[cfg(not(target_os = "windows"))]
+        let expected = "9f994bb68ace8b63bb62e9eae84e12c1d7d59207f328608f1190e5487264662c";


a hardcoded hash will break on every FORMAT_VERSION bump
is it really worth it?

Agreed, dropped the golden value; the test only checks that the marker changes multi-arch keys.

The dist command never passes the -arch flags on to the remote compiler, so a compilation with SCCACHE_CACHE_MULTIARCH and several -arch values came back from the build server as a single-arch object instead of a universal one. Run those compilations locally.

codecov-commenter · 2026-09-25T09:17:35Z

Codecov Report

❌ Patch coverage is 98.13665% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.73%. Comparing base (8396f02) to head (d78784f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/compiler/gcc.rs	98.08%	7 Missing ⚠️
tests/system.rs	97.35%	4 Missing ⚠️
src/compiler/compiler.rs	98.68%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2871      +/-   ##
==========================================
+ Coverage   76.35%   76.73%   +0.37%     
==========================================
  Files          72       72              
  Lines       40219    40774     +555     
==========================================
+ Hits        30709    31286     +577     
+ Misses       9510     9488      -22

☔ 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.

sylvestre · 2026-09-25T13:14:23Z

+                ("x86_64", x86_64_output.to_owned()),
+                ("arm64", arm64_output.to_owned()),
+            ];
+            for _ in 0..2 {


could be outputs.len() instead of 2, no?

Done, it now uses outputs.len().

sylvestre · 2026-09-25T13:14:35Z

    }
-    run_input_output(cmd, None).await
+
+    // Like ccache, one pass per architecture so each output sees the macros that


the passes are independent, could they run in parallel (join_all)?
that would roughly halve the preprocessing time for universal builds.

Done, the passes now run concurrently with join_all. They would all write the same depfile, so only the last pass gets the dependency flags, which leaves the same .d as clang does for a multi-arch compile (covered by test_multiarch_depfile_matches_compiler).

sylvestre · 2026-09-25T13:14:48Z

+            format!(
+                "#pragma sccache arch {} {}\n",
+                arch.to_string_lossy(),
+                pass.stdout.len()


this length is taken before strip_basedirs, so with basedirs the key changes with the checkout path, no?
the same fat build in /a/src and /b/longer/src won't share entries anymore.
could we drop the length, or hash each pass separately instead?

Right, good catch. Dropped the length from the separator, so stripping basedirs now gives the same key from any checkout path; added a test with two basedirs of different lengths.

sylvestre · 2026-09-25T13:15:05Z

+#[test_case(false ; "without preprocessor cache")]
+#[serial]
+#[cfg(target_os = "macos")]
+fn test_multiarch_slice_specific_header_affects_cache(preprocessor_cache_mode: bool) {


almost the same test as test_multiarch_slice_specific_code_affects_cache, could they be merged?

Done, merged into one test parameterized on whether the arch-specific code is in the source or in headers.

With SCCACHE_CACHE_MULTIARCH, a compilation with several -arch values was preprocessed once, with the -arch flags replaced by -D__<arch>__=1. Those defines are not what the compiler sees for each target: code guarded by __aarch64__, __ARM_NEON, __SSE2__ and the like did not reach the hash, so changing it could hand back a stale universal object. Like ccache, run the preprocessor once per architecture, with only that -arch flag, and hash every output, each preceded by its architecture and line count. Lines rather than bytes, so that the key doesn't depend on the basedirs stripped from the output afterwards. The passes run concurrently. They would all write the same depfile and serialized diagnostics, so only the last one keeps the arguments that write files, which leaves the same depfile as clang does. The stderr of every pass is kept. Include files are collected from every pass for the preprocessor cache. Since those compilations are never distributed, the passes don't keep the line markers dist needs.

Preprocessor cache entries recorded before multi-arch compilations were preprocessed once per -arch point at object keys computed from the old, incomplete preprocessor output, and their include files alone can't tell them apart. Mix a marker into the entry key of multi-arch compilations so those entries are no longer found. Bumping FORMAT_VERSION would have done the same, but would also have thrown away the entries of every single-arch compilation, which are still valid.

Build universal objects with -arch x86_64 -arch arm64 and check that editing code, or a header, that only one architecture sees is a cache miss and lands in the right slice of the object, with and without the preprocessor cache. Also check that the dependency file matches clang's: clang writes it once per architecture, so it lists the include files of the last -arch only, and sccache must hand back the same file on a miss and on a hit.

It was described as disabling it. Setting it to any value, even 0, enables caching of compilations with several different -arch flags.

sylvestre · 2026-09-28T11:15:26Z

Thanks for your PR

sylvestre reviewed Sep 24, 2026

View reviewed changes

francoisgirinon-netizen force-pushed the fix/multiarch-per-arch-preprocess branch from 18b65dd to 1b51bf5 Compare September 25, 2026 08:17

sylvestre reviewed Sep 25, 2026

View reviewed changes

francoisgirinon-netizen force-pushed the fix/multiarch-per-arch-preprocess branch from 1b51bf5 to fcb5bd6 Compare September 25, 2026 13:54

francoisgirinon-netizen added 4 commits September 25, 2026 16:04

docs: SCCACHE_CACHE_MULTIARCH enables multi-arch caching

d78784f

It was described as disabling it. Setting it to any value, even 0, enables caching of compilations with several different -arch flags.

francoisgirinon-netizen force-pushed the fix/multiarch-per-arch-preprocess branch from fcb5bd6 to d78784f Compare September 25, 2026 14:10

sylvestre merged commit 26b6b13 into mozilla:main Sep 28, 2026
51 checks passed

Conversation

francoisgirinon-netizen commented Sep 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sylvestre commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Sep 25, 2026 •

edited

Loading