gcc, clang: preprocess multi-arch compilations once per -arch

docs/Caching.md

@@ -24,12 +24,11 @@ See https://github.com/mozilla/sccache/blob/8567bbe2ba493153e76177c1f9a6f98cc7ba
 ### C/C++ compiler
 
 For C/C++, the hash is generated with a blake3 digest of the preprocessed
-file (-E with gcc/clang). For compilations that specify multiple `-arch` flags,
-these flags are rewritten to their corresponding preprocessor defines to allow
-pre-processing the file (e.g `-arch x86_64` is rewritten to `-D__X86_64__=1`),
-this can be enabled by setting the environment variable
-`SCCACHE_CACHE_MULTIARCH` but is disabled by default as it may not work in all
-cases.
+file (-E with gcc/clang). Compilations that specify several different `-arch`
+flags are only cached when the environment variable `SCCACHE_CACHE_MULTIARCH`
+is set. The file is then preprocessed once per architecture, with only that
+`-arch` flag, and every output goes into the hash, so that code only one of
+the targets sees (e.g. under `__aarch64__`) is taken into account.
 
 We also take into account in the hash:
 * Hash of the compiler binary

docs/Configuration.md

@@ -194,7 +194,7 @@ Note that some env variables may need sccache server restart to take effect.
 * `SCCACHE_NO_DAEMON` set to `1` to disable putting the server to the background
 * `SCCACHE_CLIENT_SIDE` set to `1` to run the compile in the client process and use the daemon only as a gateway to the cache storage (see [the architecture doc](Architecture.md#client-side-mode-sccache_client_side)). This is the recommended mode and is expected to become the only supported configuration in the future. Ignored when `SCCACHE_ERROR_LOG` or distributed compilation is in use.
 * `SCCACHE_SKIP_CACHE_CHECK` set to `true`, `on`, or `1` to skip remote cache capability checks. The configured backend `rw_mode` is used without reading or writing `.sccache_check`. The user is responsible for ensuring that the cache is reachable and grants the configured access.
-* `SCCACHE_CACHE_MULTIARCH` to disable caching of multi architecture builds.
+* `SCCACHE_CACHE_MULTIARCH` set (to any value, even `0`) to enable caching of builds with several different `-arch` flags, which are not cached by default. See [the caching doc](Caching.md#cc-compiler).
 * `SCCACHE_CACHE_ZSTD_LEVEL` to set zstd compression level of cache. the range is `1-22` and default is `3`.
   - For example, in `10`, it have about 0.9x size with about 1.6x time than default `3` (tested with compiling sccache code)
   - This option will only applied to newly compressed cache and don't affect existing cache.

src/compiler/c.rs

@@ -131,6 +131,25 @@ impl ParsedArguments {
             .map(|s| s.to_string_lossy())
             .unwrap_or(Cow::Borrowed("Unknown filename"))
     }
+
+    /// The distinct architectures given with `-arch`, in command line order.
+    pub fn archs(&self) -> Vec<&OsString> {
+        use itertools::Itertools as _;
+        self.arch_values().unique().collect()
+    }
+
+    pub fn is_multiarch(&self) -> bool {
+        let mut archs = self.arch_values();
+        archs
+            .next()
+            .is_some_and(|first| archs.any(|arch| arch != first))
+    }
+
+    fn arch_values(&self) -> impl Iterator<Item = &OsString> {
+        self.arch_args
+            .iter()
+            .filter(|arg| *arg != super::gcc::ARCH_FLAG)
+    }
 }
 
 /// A generic implementation of the `Compilation` trait for C/C++ compilers.
@@ -467,6 +486,7 @@ where
                 &env_vars,
                 &absolute_input_path,
                 self.compiler.plusplus(),
+                self.parsed_args.is_multiarch(),
                 preprocessor_cache_mode_config,
                 storage.basedirs(),
             )?

src/compiler/compiler.rs

@@ -2589,6 +2589,94 @@ LLVM version: 6.0",
         assert_ne!(results[0].key, results[1].key);
     }
 
+    #[test_case(true ; "with preprocessor cache")]
+    #[test_case(false ; "without preprocessor cache")]
+    fn test_multiarch_hash_covers_every_arch(preprocessor_cache_mode: bool) {
+        let f = TestFixture::new();
+        let clang = f.mk_bin("clang").unwrap();
+        let runtime = single_threaded_runtime();
+        let pool = runtime.handle();
+        let cwd = f.tempdir.path();
+        // Write a dummy input file so the preprocessor cache mode can work
+        std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap();
+
+        let key = |arguments: &[OsString], x86_64_output: &str, arm64_output: &str| {
+            let creator = new_creator();
+            next_command(
+                &creator,
+                Ok(MockChild::new(
+                    exit_status(0),
+                    "compiler_id=clang\ncompiler_version=\"16.0.0\"",
+                    "",
+                )),
+            );
+            next_assembler(&creator, "GNU assembler (GNU Binutils) 2.42", "");
+            let c = detect_compiler(
+                creator.clone(),
+                &clang,
+                f.tempdir.path(),
+                &[],
+                &[],
+                pool,
+                None,
+            )
+            .wait()
+            .unwrap()
+            .0;
+            let outputs = [
+                ("x86_64", x86_64_output.to_owned()),
+                ("arm64", arm64_output.to_owned()),
+            ];
+            for _ in 0..outputs.len() {
+                let outputs = outputs.clone();
+                next_command_calls(&creator, move |args| {
+                    let output = outputs
+                        .iter()
+                        .find(|(arch, _)| args.iter().any(|a| a == arch))
+                        .map(|(_, output)| output.clone())
+                        .unwrap_or_default();
+                    Ok(MockChild::new(exit_status(0), output, ""))
+                });
+            }
+            let mut hasher = match c.parse_arguments(arguments, ".".as_ref(), &[]) {
+                CompilerArguments::Ok(h) => h,
+                o => panic!("Bad result from parse_arguments: {:?}", o),
+            };
+            hasher
+                .generate_hash_key(
+                    &creator,
+                    cwd.to_path_buf(),
+                    vec![],
+                    false,
+                    pool,
+                    false,
+                    Arc::new(MockStorage::new(None, preprocessor_cache_mode)),
+                    CacheControl::Default,
+                )
+                .wait()
+                .unwrap()
+                .key
+        };
+
+        temp_env::with_var("SCCACHE_CACHE_MULTIARCH", Some("1"), || {
+            let fat = ovec![
+                "-arch", "x86_64", "-arch", "arm64", "-c", "foo.c", "-o", "foo.o"
+            ];
+            assert_eq!(
+                key(&fat, "x86_64 code\n", "arm64 code\n"),
+                key(&fat, "x86_64 code\n", "arm64 code\n")
+            );
+            assert_ne!(
+                key(&fat, "x86_64 code\n", "arm64 code\n"),
+                key(&fat, "x86_64 code\n", "arm64 code, changed\n")
+            );
+            assert_ne!(
+                key(&fat, "x86_64 code\n", "arm64 code\n"),
+                key(&fat, "x86_64 code, changed\n", "arm64 code\n")
+            );
+        });
+    }
+
     #[test_case(true ; "with preprocessor cache")]
     #[test_case(false ; "without preprocessor cache")]
     fn test_assembler_affects_hash(preprocessor_cache_mode: bool) {