refactor: extract GetCompileCommand and StdPcmDir out of Build

The clang invocation was assembled inline across four regions of Build,
interleaved with the dependency-graph walk, so nothing else could ask "what
flags does this Configuration compile with". The linter's AST layer needs
exactly that, and it cannot approximate it: a precompiled module is rejected
outright by a translation unit whose target features differ from the one that
wrote it. Dropping just -march=native produces hundreds of "compiled with the
target feature '+avx512bw' but the current translation unit is not" errors and
no usable parse, so reconstructed flags fail hard rather than degrade.

GetCompileCommand is the config-pure part: target, arch, standard,
configuration defines, module search paths, includes, user compileFlags,
optimisation and LTO. Build appends only what depends on work having happened
— dependency public flags and external dependency flags. The sub-strings it
also needs on their own (includes, defines, user flags, LTO) come back as
struct members, so the .c compile path is unchanged.

Verified by probing `command` at the equivalent point before and after and
diffing: byte-identical across all 23 configurations exercised by a full build
plus the test suite.

Two incidental simplifications fell out. pcmDir was recomputing what
Configuration::PcmDir() already returns, and cmakeBuildType is now a
one-liner. GetCompileCommand is also most of what a compile_commands.json
would need, which this repo lacks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jorijn van der Graaf 2026-07-30 23:15:20 +02:00
commit 7bb0a19ed0
2 changed files with 193 additions and 141 deletions

View file

@ -252,6 +252,145 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
}
}
fs::path Crafter::StdPcmDir(const Configuration& config) {
// The std PCM is cached per target+march, but a wasm variant pass compiles
// it with extra codegen flags (e.g. -mrelaxed-simd) — its BMI must not be
// shared with the baseline's, or the consuming TUs see a target-feature
// mismatch. Suffix the cache dir with the variant flags when present.
std::string stdPcmKey = std::format("{}-{}", config.target, config.march);
for (const std::string& f : config.wasmVariantFlags) {
stdPcmKey += "+";
for (char c : f) stdPcmKey += (c == '/' || c == '\\') ? '_' : c;
}
return GetCacheDir()/stdPcmKey;
}
CompileCommand Crafter::GetCompileCommand(const Configuration& config) {
CompileCommand out;
out.stdPcmDir = StdPcmDir(config);
out.pcmDir = config.PcmDir();
std::string editedTarget = config.target;
std::replace(editedTarget.begin(), editedTarget.end(), '-', '_');
// wasm32 targets reject -march and silently ignore -mtune (clang errors on
// the former). Skip both for any wasm32-* triple.
bool isWasm = config.target.starts_with("wasm32");
std::string archFlags = isWasm
? std::string()
: std::format(" -march={} -mtune={}", config.march, config.mtune);
out.command = std::format("{} --target={}{} -std=c++26 -D CRAFTER_BUILD_CONFIGURATION_TARGET=\\\"{}\\\" -D CRAFTER_BUILD_CONFIGURATION_TARGET_{} -fprebuilt-module-path={} -fprebuilt-module-path={}", GetBaseCommand(config), config.target, archFlags, editedTarget, editedTarget, out.stdPcmDir.string(), out.pcmDir.string());
if (!config.sysroot.empty()) {
out.command += std::format(" --sysroot={}", config.sysroot);
}
if (isWasm) {
// -mllvm is consumed by codegen but not the link driver, which is the
// same command line; quiet the unused-flag warning rather than split
// compile and link commands.
out.command += " -fno-exceptions -msimd128 -fno-c++-static-destructors -mllvm -wasm-enable-sjlj -D_WASI_EMULATED_SIGNAL -Wno-unused-command-line-argument";
// Active variant pass (see the variant driver at the end of Build):
// extra codegen flags (e.g. -mrelaxed-simd) applied to every TU. Empty
// for the baseline build. Part of VariantId, so these objects/PCMs
// land in their own dir.
for (const std::string& f : config.wasmVariantFlags) {
out.command += std::format(" {}", f);
}
}
if (config.target == "x86_64-w64-mingw32") {
// mingw libstdc++ defines TLS via __emutls_v.* (emulated TLS); without
// -femulated-tls clang generates native-TLS references that don't
// match. Symptom: undefined std::__once_callable / __once_call at
// link time. Also -Wno-unused… because -femulated-tls is a codegen
// flag the link driver doesn't consume.
out.command += " -femulated-tls -Wno-unused-command-line-argument";
}
if(config.type == ConfigurationType::LibraryDynamic) {
#ifdef CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_linux_gnu
out.command += " -fPIC -D CRAFTER_BUILD_CONFIGURATION_TYPE_SHARED_LIBRARY";
#endif
#if defined(CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_windows_msvc) || defined(CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_w64_mingw32)
out.command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_SHARED_LIBRARY";
#endif
} else if(config.type == ConfigurationType::Executable) {
out.command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_EXECUTABLE";
// On Windows targets the API uses __declspec(dllimport) when consuming
// a DLL. Set the macro for executables so CRAFTER_API resolves to
// dllimport in their PCM cache (separate from the lib's PCM cache,
// which gets dllexport). Harmless if the exe doesn't actually link a
// crafter DLL — CRAFTER_API only matters at API call sites.
if (config.target == "x86_64-w64-mingw32" || config.target == "x86_64-pc-windows-msvc") {
out.command += " -D CRAFTER_BUILD_DLL_IMPORT";
}
} else {
out.command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_LIBRARY";
}
// -I propagation that's valid for both C and C++ compiles. Module-only
// bits (-fprebuilt-module-path) stay on `command` only.
{
std::unordered_set<Configuration*> seen;
std::function<void(Configuration*)> addFlags = [&](Configuration* dep) {
if (!seen.insert(dep).second) return;
for (const auto& entry : fs::recursive_directory_iterator(dep->path)) {
if (entry.is_directory() && entry.path().filename() == "include") {
out.includeFlags += std::format(" -I{}", entry.path().string());
}
}
out.includeFlags += std::format(" -I{}", dep->path.string());
out.command += std::format(" -fprebuilt-module-path={}", dep->PcmDir().string());
for (Configuration* sub : dep->dependencies) {
addFlags(sub);
}
};
for (Configuration* dep : config.dependencies) {
addFlags(dep);
}
}
out.command += out.includeFlags;
// Defines belong on both C and C++ compiles so vendored C dependencies
// can see configuration-level macros consistently with module sources.
for(const Define& define : config.defines) {
if(define.value.empty()) {
out.defineFlags += std::format(" -D {}", define.name);
} else {
out.defineFlags += std::format(" -D {}={}", define.name, define.value);
}
}
out.command += out.defineFlags;
// Track caller-provided compileFlags separately so the .c compile can
// pick them up too (vendored C deps usually need -I from this set).
for(const std::string& flag : config.compileFlags) {
out.userFlags += std::format(" {}", flag);
}
out.command += out.userFlags;
// Behaviour-neutral release performance for the binaries we emit: ThinLTO
// (cross-TU inlining) plus dead-section GC and safe identical-code folding.
// Default-on in Release, never Debug (keeps builds fast and debuggable).
// Excluded for wasm32 — its -mllvm/sjlj codegen flags don't compose with
// LTO here — and for nvcc .cu objects below (they can't emit LLVM bitcode,
// so they link in as ordinary objects alongside the bitcode ones). ThinLTO
// rather than monolithic -flto so link time and memory scale with arbitrary
// user project sizes. Compile-side flags (-flto, -ffunction/-fdata-sections)
// ride on `command`, which is reused as the link base; the link-only -Wl
// flags go on linkExtras so they don't warn on each -c compile.
out.useLto = !config.debug && !isWasm;
out.ltoCompileFlags = out.useLto ? " -flto=thin -ffunction-sections -fdata-sections" : "";
out.ltoLinkFlags = out.useLto ? " -flto=thin -Wl,--gc-sections -Wl,--icf=safe" : "";
if(config.debug) {
out.command += " -g -D CRAFTER_BUILD_CONFIGURATION_DEBUG";
} else {
out.command += " -O3";
}
out.command += out.ltoCompileFlags;
return out;
}
BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, std::shared_future<BuildResult>>& depResults, std::mutex& depMutex) {
// Reset per-build cached state on every Module/ModulePartition so that
// successive Build() calls on the same Configuration re-evaluate mtimes
@ -499,16 +638,7 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
});
}
// The std PCM is cached per target+march, but a wasm variant pass compiles
// it with extra codegen flags (e.g. -mrelaxed-simd) — its BMI must not be
// shared with the baseline's, or the consuming TUs see a target-feature
// mismatch. Suffix the cache dir with the variant flags when present.
std::string stdPcmKey = std::format("{}-{}", config.target, config.march);
for (const std::string& f : config.wasmVariantFlags) {
stdPcmKey += "+";
for (char c : f) stdPcmKey += (c == '/' || c == '\\') ? '_' : c;
}
fs::path stdPcmDir = GetCacheDir()/stdPcmKey;
fs::path stdPcmDir = StdPcmDir(config);
if (!fs::exists(stdPcmDir)) {
fs::create_directories(stdPcmDir);
@ -526,72 +656,25 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
return {stdPcmResult, false, {}};
}
fs::path pcmDir;
if(config.type != ConfigurationType::Executable) {
pcmDir = outputDir;
} else {
pcmDir = buildDir;
}
fs::path pcmDir = config.PcmDir();
fs::copy_file(stdPcmDir/"std.pcm", pcmDir/"std.pcm", fs::copy_options::update_existing);
std::string editedTarget = config.target;
std::replace(editedTarget.begin(), editedTarget.end(), '-', '_');
// wasm32 targets reject -march and silently ignore -mtune (clang errors on
// the former). Skip both for any wasm32-* triple.
bool isWasm = config.target.starts_with("wasm32");
std::string archFlags = isWasm
? std::string()
: std::format(" -march={} -mtune={}", config.march, config.mtune);
std::string command = std::format("{} --target={}{} -std=c++26 -D CRAFTER_BUILD_CONFIGURATION_TARGET=\\\"{}\\\" -D CRAFTER_BUILD_CONFIGURATION_TARGET_{} -fprebuilt-module-path={} -fprebuilt-module-path={}", GetBaseCommand(config), config.target, archFlags, editedTarget, editedTarget, stdPcmDir.string(), pcmDir.string());
if (!config.sysroot.empty()) {
command += std::format(" --sysroot={}", config.sysroot);
}
if (config.target.starts_with("wasm32")) {
// -mllvm is consumed by codegen but not the link driver, which is the
// same command line; quiet the unused-flag warning rather than split
// compile and link commands.
command += " -fno-exceptions -msimd128 -fno-c++-static-destructors -mllvm -wasm-enable-sjlj -D_WASI_EMULATED_SIGNAL -Wno-unused-command-line-argument";
// Active variant pass (see the variant driver at the end of Build):
// extra codegen flags (e.g. -mrelaxed-simd) applied to every TU. Empty
// for the baseline build. Part of VariantId, so these objects/PCMs
// land in their own dir.
for (const std::string& f : config.wasmVariantFlags) {
command += std::format(" {}", f);
}
}
if (config.target == "x86_64-w64-mingw32") {
// mingw libstdc++ defines TLS via __emutls_v.* (emulated TLS); without
// -femulated-tls clang generates native-TLS references that don't
// match. Symptom: undefined std::__once_callable / __once_call at
// link time. Also -Wno-unused… because -femulated-tls is a codegen
// flag the link driver doesn't consume.
command += " -femulated-tls -Wno-unused-command-line-argument";
}
if(config.type == ConfigurationType::LibraryDynamic) {
#ifdef CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_linux_gnu
command += " -fPIC -D CRAFTER_BUILD_CONFIGURATION_TYPE_SHARED_LIBRARY";
#endif
#if defined(CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_windows_msvc) || defined(CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_w64_mingw32)
command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_SHARED_LIBRARY";
#endif
} else if(config.type == ConfigurationType::Executable) {
command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_EXECUTABLE";
// On Windows targets the API uses __declspec(dllimport) when consuming
// a DLL. Set the macro for executables so CRAFTER_API resolves to
// dllimport in their PCM cache (separate from the lib's PCM cache,
// which gets dllexport). Harmless if the exe doesn't actually link a
// crafter DLL — CRAFTER_API only matters at API call sites.
if (config.target == "x86_64-w64-mingw32" || config.target == "x86_64-pc-windows-msvc") {
command += " -D CRAFTER_BUILD_DLL_IMPORT";
}
} else {
command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_LIBRARY";
}
// Everything about the compile command that is a pure function of the
// Configuration is assembled by GetCompileCommand, so that anything which
// needs to PARSE these sources with the same flags — the linter's AST
// layer — cannot drift from what actually built the PCMs. Build appends
// only what depends on work having happened: dependency public flags and
// external dependency flags, further down.
CompileCommand compile = GetCompileCommand(config);
std::string command = compile.command;
const std::string& includeFlags = compile.includeFlags;
const std::string& defineFlags = compile.defineFlags;
const std::string& userFlags = compile.userFlags;
const std::string& ltoCompileFlags = compile.ltoCompileFlags;
const std::string& ltoLinkFlags = compile.ltoLinkFlags;
const bool isWasm = config.target.starts_with("wasm32");
const bool useLto = compile.useLto;
std::string files;
std::unordered_set<std::string> libSet;
@ -601,30 +684,6 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
depThreads.reserve(config.dependencies.size());
std::atomic<bool> repack(false);
// -I propagation that's valid for both C and C++ compiles. Module-only
// bits (-fprebuilt-module-path) stay on `command` only.
std::string includeFlags;
{
std::unordered_set<Configuration*> seen;
std::function<void(Configuration*)> addFlags = [&](Configuration* dep) {
if (!seen.insert(dep).second) return;
for (const auto& entry : fs::recursive_directory_iterator(dep->path)) {
if (entry.is_directory() && entry.path().filename() == "include") {
includeFlags += std::format(" -I{}", entry.path().string());
}
}
includeFlags += std::format(" -I{}", dep->path.string());
command += std::format(" -fprebuilt-module-path={}", dep->PcmDir().string());
for (Configuration* sub : dep->dependencies) {
addFlags(sub);
}
};
for (Configuration* dep : config.dependencies) {
addFlags(dep);
}
}
command += includeFlags;
for(Configuration* dep : config.dependencies) {
depThreads.emplace_back([&, dep](){
try {
@ -681,50 +740,9 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
});
}
// Defines belong on both C and C++ compiles so vendored C dependencies
// can see configuration-level macros consistently with module sources.
std::string defineFlags;
for(const Define& define : config.defines) {
if(define.value.empty()) {
defineFlags += std::format(" -D {}", define.name);
} else {
defineFlags += std::format(" -D {}={}", define.name, define.value);
}
}
command += defineFlags;
// Track caller-provided compileFlags separately so the .c compile can
// pick them up too (vendored C deps usually need -I from this set).
std::string userFlags;
for(const std::string& flag : config.compileFlags) {
userFlags += std::format(" {}", flag);
}
command += userFlags;
std::string cmakeBuildType;
// Behaviour-neutral release performance for the binaries we emit: ThinLTO
// (cross-TU inlining) plus dead-section GC and safe identical-code folding.
// Default-on in Release, never Debug (keeps builds fast and debuggable).
// Excluded for wasm32 — its -mllvm/sjlj codegen flags don't compose with
// LTO here — and for nvcc .cu objects below (they can't emit LLVM bitcode,
// so they link in as ordinary objects alongside the bitcode ones). ThinLTO
// rather than monolithic -flto so link time and memory scale with arbitrary
// user project sizes. Compile-side flags (-flto, -ffunction/-fdata-sections)
// ride on `command`, which is reused as the link base; the link-only -Wl
// flags go on linkExtras so they don't warn on each -c compile.
const bool useLto = !config.debug && !isWasm;
const std::string ltoCompileFlags = useLto ? " -flto=thin -ffunction-sections -fdata-sections" : "";
const std::string ltoLinkFlags = useLto ? " -flto=thin -Wl,--gc-sections -Wl,--icf=safe" : "";
if(config.debug) {
cmakeBuildType = "Debug";
command += " -g -D CRAFTER_BUILD_CONFIGURATION_DEBUG";
} else {
cmakeBuildType = "Release";
command += " -O3";
}
command += ltoCompileFlags;
// Only the CMake build type is still derived here; the compile flags it
// used to sit alongside now come from GetCompileCommand.
const std::string cmakeBuildType = config.debug ? "Debug" : "Release";
// Same target-aware setup as the C++ compile path (line 459-): wasm32
// rejects -march, silently ignores -mtune, and needs --sysroot to find