From 93f1106ba431798930692feb0e94305813dfc15b Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 11:30:31 -0400 Subject: [PATCH] [Fix] (ShaderTranspiler): key translation cache L1 on the front-end environment only, not backend identity --- .../GLState/ProgramState/ProgramLinkTask.cpp | 5 +- .../ShaderTranspiler/TranslationCacheTest.cpp | 110 +++++++++++++++++- .../MG_Util/ShaderTranspiler/CompileEnv.cpp | 26 +++++ .../MG_Util/ShaderTranspiler/CompileEnv.h | 50 ++++++++ .../ShaderTranspiler/ShaderCompiler.cpp | 5 + .../ShaderTranspiler/TranslationCache.cpp | 2 +- .../ShaderTranspiler/TranslationCache.h | 19 ++- 7 files changed, 208 insertions(+), 9 deletions(-) diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 59096bd7..3b871a05 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -518,7 +518,10 @@ namespace MobileGL::MG_State::GLState { if (!ShaderTranslationCacheEnabled()) return {}; SpirvTranslationKeyInputs keyInputs; - keyInputs.envFingerprint = env.fingerprint; + // The FRONT-END fingerprint, not env.fingerprint: L1 must be shared by two contexts + // on different GPUs whenever glslang would produce the same thing for them. See the + // classification on CompileEnv::frontendFingerprint. + keyInputs.frontendFingerprint = env.frontendFingerprint; // Always 0 on both production parse paths (ShaderCompileTask::RunCompilePipeline and // ClaimParsedShader's re-parse). In the key regardless, so that a future non-zero // value cannot alias a module parsed without it. diff --git a/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp index 860c5d49..37a31ac4 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -120,7 +121,7 @@ void main() { SpirvTranslationKeyInputs BaselineSpirvInputs(const Vector& stages) { SpirvTranslationKeyInputs inputs; - inputs.envFingerprint = 0x1234'5678'9abc'def0ull; + inputs.frontendFingerprint = 0x1234'5678'9abc'def0ull; inputs.stages = stages; inputs.shaderCompileFlags = 0; inputs.enableSpirvValidation = false; @@ -394,8 +395,8 @@ TEST_F(TranslationCacheTest, L1KeyMovesWithEveryInputThatMovesTheSpirv) { { // the environment fingerprint (glslang resource limits, backend identity, // advertised extension set, compute limits) SpirvTranslationKeyInputs v = base; - v.envFingerprint ^= 1ull; - variants.emplace_back("envFingerprint", BuildSpirvTranslationKey(v)); + v.frontendFingerprint ^= 1ull; + variants.emplace_back("frontendFingerprint", BuildSpirvTranslationKey(v)); } { // a stage's source text const String otherFs = SwizzleLikeFragment("i"); @@ -462,6 +463,109 @@ TEST_F(TranslationCacheTest, L1KeyMovesWithEveryInputThatMovesTheSpirv) { } } +// ========================================================================================= +// L1 backend-agnosticism: the environment inputs that were REMOVED from the key +// ========================================================================================= + +namespace { + // Two environments that differ in every way that only steers a BACKEND, and in no way + // that reaches glslang. + Pair BackendOnlyDifferentEnvs() { + CompileEnv a; + CompileEnv b; + // (1) backend identity - both HAVE a backend, they are just different ones + a.backend = BackendType::DirectGLES; + b.backend = BackendType::DirectVulkan; + // (2) the advertised extension vector, including the fp64 flag's own extension + a.advertisedExtensions = {E_GL_ARB_gpu_shader_fp64, E_GL_KHR_debug}; + b.advertisedExtensions = {}; + // (3) the compute limits (ValidateComputeLocalSizeLimits only) + a.maxComputeWorkGroupSize[0] = 1024; + a.maxComputeWorkGroupSize[1] = 1024; + a.maxComputeWorkGroupSize[2] = 64; + a.maxComputeWorkGroupInvocations = 128; + b.maxComputeWorkGroupSize[0] = 2048; + b.maxComputeWorkGroupSize[1] = 2048; + b.maxComputeWorkGroupSize[2] = 1024; + b.maxComputeWorkGroupInvocations = 2048; + // (4) a spread of DynamicBackendParameters fields the front end never reads + a.params.MaxColorTextureSamples = 1; + b.params.MaxColorTextureSamples = 8; + a.params.MaxTextureSize = 4096; + b.params.MaxTextureSize = 16384; + a.params.MaxViewports = 1; + b.params.MaxViewports = 16; + a.params.MaxUniformBufferBindings = 24; + b.params.MaxUniformBufferBindings = 84; + a.params.MaxTextureImageUnits = 16; + b.params.MaxTextureImageUnits = 32; + return {a, b}; + } +} // namespace + +// THE case that pins L1's backend-agnosticism. Everything moved here is something that +// only steers a backend transpile, and L2 already keys on the ones that matter there. +// The old whole-environment fingerprint moves; the front-end one must not. +TEST_F(TranslationCacheTest, TheFrontendFingerprintIgnoresBackendOnlyDifferences) { + const auto [a, b] = BackendOnlyDifferentEnvs(); + + EXPECT_NE(ComputeCompileEnvFingerprint(a), ComputeCompileEnvFingerprint(b)) + << "the whole-environment fingerprint is supposed to notice these; if it does not, " + "this case is no longer testing anything"; + EXPECT_EQ(ComputeFrontendCompileEnvFingerprint(a), ComputeFrontendCompileEnvFingerprint(b)) + << "a backend-only difference leaked into the front-end fingerprint"; +} + +// ... and the same thing one level up: the two environments must produce ONE L1 entry. +TEST_F(TranslationCacheTest, TwoBackendsCompilingTheSameGlslShareOneL1Entry) { + const auto [a, b] = BackendOnlyDifferentEnvs(); + const String vs = kVertexSource; + const String fs = kFragmentSource; + const Vector stages{{GL_VERTEX_SHADER, vs}, + {GL_FRAGMENT_SHADER, fs}}; + + SpirvTranslationKeyInputs onA = BaselineSpirvInputs(stages); + onA.frontendFingerprint = ComputeFrontendCompileEnvFingerprint(a); + SpirvTranslationKeyInputs onB = BaselineSpirvInputs(stages); + onB.frontendFingerprint = ComputeFrontendCompileEnvFingerprint(b); + + EXPECT_TRUE(BuildSpirvTranslationKey(onA) == BuildSpirvTranslationKey(onB)); +} + +// The other direction, one case per input that was KEPT. Each is a limit the front end +// really consumes - the seven BuildTBuiltInResource copies into TBuiltInResource, plus the +// two inputs to the reflection vertex-attrib limit - so each must still split the key. +TEST_F(TranslationCacheTest, TheFrontendFingerprintMovesWithEveryFrontendLimit) { + const CompileEnv base; + const Uint64 baseline = ComputeFrontendCompileEnvFingerprint(base); + + const Vector>> mutations{ + {"params.MaxImageUnits", [](CompileEnv& e) { e.params.MaxImageUnits += 1; }}, + {"params.MaxDrawBuffers", [](CompileEnv& e) { e.params.MaxDrawBuffers += 1; }}, + {"params.MaxVertexImageUniforms", [](CompileEnv& e) { e.params.MaxVertexImageUniforms += 1; }}, + {"params.MaxGeometryImageUniforms", [](CompileEnv& e) { e.params.MaxGeometryImageUniforms += 1; }}, + {"params.MaxFragmentImageUniforms", [](CompileEnv& e) { e.params.MaxFragmentImageUniforms += 1; }}, + {"params.MaxComputeImageUniforms", [](CompileEnv& e) { e.params.MaxComputeImageUniforms += 1; }}, + {"params.MaxCombinedImageUniforms", [](CompileEnv& e) { e.params.MaxCombinedImageUniforms += 1; }}, + {"params.MaxVertexAttribs", [](CompileEnv& e) { e.params.MaxVertexAttribs += 1; }}, + // HasBackend(): with no backend the reflection attrib limit falls back to the + // storage capacity rather than the driver's number, so the bit is load-bearing. + {"HasBackend", [](CompileEnv& e) { e.backend = BackendType::DirectGLES; }}, + }; + + Vector seen{baseline}; + for (const auto& [name, mutate] : mutations) { + CompileEnv env = base; + mutate(env); + const Uint64 moved = ComputeFrontendCompileEnvFingerprint(env); + EXPECT_NE(moved, baseline) << "moving " << name << " did not move the front-end fingerprint"; + for (const Uint64 previous : seen) { + EXPECT_NE(moved, previous) << name << " collides with an earlier front-end limit"; + } + seen.push_back(moved); + } +} + // ========================================================================================= // L1 end to end, through the real GL entry points // ========================================================================================= diff --git a/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.cpp b/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.cpp index cb8e3a46..ce61f303 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.cpp @@ -41,6 +41,30 @@ namespace MobileGL::MG_Util::ShaderTranspiler { return state; } + Uint64 ComputeFrontendCompileEnvFingerprint(const CompileEnv& env) { + Uint64 state = 0xff51afd7ed558ccdull; + // The seven limits BuildTBuiltInResource copies into TBuiltInResource. Enumerated + // ONE BY ONE rather than hashed as a struct, deliberately: hashing all of + // DynamicBackendParameters would drag ~50 backend-only limits into a key that is + // supposed to be backend-agnostic, and every one of them would be a false miss. + // Keep this list in step with BuildTBuiltInResource. + HashValue(state, env.params.MaxImageUnits); + HashValue(state, env.params.MaxDrawBuffers); + HashValue(state, env.params.MaxVertexImageUniforms); + HashValue(state, env.params.MaxGeometryImageUniforms); + HashValue(state, env.params.MaxFragmentImageUniforms); + HashValue(state, env.params.MaxComputeImageUniforms); + HashValue(state, env.params.MaxCombinedImageUniforms); + // The two inputs to GetReflectionVertexAttribLimit. Hashed as inputs rather than as + // the resolved limit so this stays in one translation unit; that is coarser (two + // envs whose MaxVertexAttribs both exceed the storage capacity resolve to the same + // limit yet hash differently) but coarser means a false MISS, never a false hit. + HashValue(state, env.params.MaxVertexAttribs); + const Uint8 hasBackend = env.HasBackend() ? 1u : 0u; + HashValue(state, hasBackend); + return state; + } + SharedPtr CaptureCompileEnv() { auto env = MakeShared(); @@ -73,6 +97,7 @@ namespace MobileGL::MG_Util::ShaderTranspiler { : kFrontendMaxComputeWorkGroupInvocations; env->fingerprint = ComputeCompileEnvFingerprint(*env); + env->frontendFingerprint = ComputeFrontendCompileEnvFingerprint(*env); return env; } @@ -82,6 +107,7 @@ namespace MobileGL::MG_Util::ShaderTranspiler { static const SharedPtr kDefault = [] { auto env = MakeShared(); env->fingerprint = ComputeCompileEnvFingerprint(*env); + env->frontendFingerprint = ComputeFrontendCompileEnvFingerprint(*env); return SharedPtr(Move(env)); }(); return kDefault; diff --git a/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.h b/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.h index 2eadd65b..ac0e6b3d 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.h +++ b/MobileGL/MG_Util/ShaderTranspiler/CompileEnv.h @@ -48,6 +48,50 @@ namespace MobileGL::MG_Util::ShaderTranspiler { Uint64 fingerprint = 0; // set by CaptureCompileEnv() + // The FRONT-END half of the environment: the subset of the fields above that can + // change what glslang PRODUCES - the SPIR-V or the reflection - as opposed to what a + // BACKEND later does with the result. This, and never `fingerprint`, is what the L1 + // shader translation memo keys on, because L1 is backend-agnostic BY CONTRACT: two + // contexts on different GPUs compiling the same GLSL must share one L1 entry. + // + // WHAT IS IN IT (audited; re-audit whenever a new env read appears in the front end): + // * the seven DynamicBackendParameters fields BuildTBuiltInResource actually copies + // into TBuiltInResource - MaxImageUnits, MaxDrawBuffers, MaxVertexImageUniforms, + // MaxGeometryImageUniforms, MaxFragmentImageUniforms, MaxComputeImageUniforms, + // MaxCombinedImageUniforms. glslang enforces those at parse, so they decide + // whether a shader compiles at all and can change the link result. + // * MaxVertexAttribs and the HasBackend() bit: the two inputs to ProgramLinkTask's + // GetReflectionVertexAttribLimit, which bounds how many vertex input locations + // reflection records - so they change the REFLECTION the memo carries. + // Both are backend-DERIVED but front-end-CONSUMED. Dropping them would be a + // miscompile, not a backend leak: a driver with 16 vertex attribs and one with 32 + // genuinely reflect the same GLSL differently. + // + // WHAT IS DELIBERATELY OUT: + // * `backend` beyond the HasBackend() bit. Nothing in the parse, the link or + // GlslangToSpv branches on which backend is active - ShaderAttrib::flags is 0 on + // both production parse paths (ShaderCompileTask::RunCompilePipeline and + // ClaimParsedShader). Backend identity steers the TRANSPILE, which is L2's key. + // * `advertisedExtensions`. Its only front-end consumer is ShaderSourceProcessor's + // FilterUnsupportedGpuShaderInt64, which REWRITES THE SOURCE TEXT - and the + // preprocessed text is in the L1 key verbatim, a strictly finer discriminator + // than the extension list. (E_GL_ARB_gpu_shader_fp64 is never read by the front + // end at all: MOBILEGL_ADVERTISE_FP64 only adds it to the extension STRING the + // application queries, and DemoteFloat64Pass runs unconditionally either way, so + // fp64 GLSL translates identically with the flag on or off.) + // * the other ~50 DynamicBackendParameters fields: read by the GL getters and by + // the backends, never by the parse, the link or reflection. + // * maxComputeWorkGroupSize / maxComputeWorkGroupInvocations. Consumed ONLY by + // ValidateComputeLocalSizeLimits, a pre-parse ACCEPT/REJECT gate. A rejected + // shader fails its compile, so its program never reaches the tail of the link and + // no L1 entry is ever created under a rejecting environment; an accepted one + // produces the same SPIR-V under any limits, because BuildTBuiltInResource + // HARDCODES the compute maxima instead of reading these. + // THIS ONE IS A REACHABILITY ARGUMENT, NOT AN INDEPENDENCE ONE. If the TODO in + // BuildTBuiltInResource ("Drive glslang compute resource limits from the active + // backend") is ever done, these MUST move into this fingerprint. + Uint64 frontendFingerprint = 0; // set by CaptureCompileEnv() + Bool HasBackend() const { return backend != BackendType::Unknown; } // Matches the historical rule exactly: with no active backend every extension counts // as advertised, because the frontend then has nothing to gate against. @@ -62,6 +106,12 @@ namespace MobileGL::MG_Util::ShaderTranspiler { // different envs really do produce different P0b cache keys. Uint64 ComputeCompileEnvFingerprint(const CompileEnv& env); + // The backend-agnostic half; see CompileEnv::frontendFingerprint for the classification + // and the evidence behind each call. Public so a test can assert both directions: that a + // backend-only difference produces the SAME value (which is what pins L1's + // backend-agnosticism) and that a front-end limit produces a different one. + Uint64 ComputeFrontendCompileEnvFingerprint(const CompileEnv& env); + // GL thread only: this is where the GL_MAX_COMPUTE_WORK_GROUP_SIZE queries live now. SharedPtr CaptureCompileEnv(); diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 7cf8722a..32c136b5 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -89,6 +89,11 @@ namespace MobileGL { Resources.maxComputeWorkGroupSizeX = 1024; Resources.maxComputeWorkGroupSizeY = 1024; // TODO: Drive glslang compute resource limits from the active backend instead of this permissive cap. + // WHEN THAT IS DONE: CompileEnv::maxComputeWorkGroupSize and + // maxComputeWorkGroupInvocations must also be added to + // ComputeFrontendCompileEnvFingerprint(). They are out of the L1 memo key today + // ONLY because these maxima are hardcoded here - see the classification comment + // on CompileEnv::frontendFingerprint. Resources.maxComputeWorkGroupSizeZ = 1024; Resources.maxComputeUniformComponents = 1024; Resources.maxComputeTextureImageUnits = 16; diff --git a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp index 7a63d4a3..86154d4e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp @@ -99,7 +99,7 @@ namespace MobileGL::MG_Util::ShaderTranspiler { TranslationCacheKey BuildSpirvTranslationKey(const SpirvTranslationKeyInputs& inputs) { TranslationKeyBuilder builder; AppendCommonKeyPrefix(builder, kSpirvKeyTag); - builder.Value(inputs.envFingerprint); + builder.Value(inputs.frontendFingerprint); builder.Value(inputs.shaderCompileFlags); builder.Value(static_cast(inputs.enableSpirvValidation)); builder.Value(static_cast(inputs.stages.size())); diff --git a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h index a3d242c6..8140ab82 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h +++ b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h @@ -321,10 +321,19 @@ namespace MobileGL::MG_Util::ShaderTranspiler { // costs on top of the 40 us GlslangToSpv, and gives the backends exactly the // bytes they would have got. // + // L1 IS BACKEND-AGNOSTIC BY CONTRACT. Two contexts on different GPUs compiling + // the same GLSL share one L1 entry: nothing that merely steers a BACKEND + // transpile (backend identity, GLES/Vulkan capability bits, driver extension + // strings, GPU vendor) is allowed in this key - all of that lives in L2's key, + // where it belongs. What IS here is the subset of the environment that changes + // what glslang itself produces; see CompileEnv::frontendFingerprint for the + // field-by-field classification and the evidence behind each call. + // // WHAT IS IN THE KEY (each one is an input that can change the modules): - // * the CompileEnv fingerprint - covers the glslang resource limits - // (BuildTBuiltInResource reads env->params), the backend identity, the - // advertised extension set and the compute limits; + // * CompileEnv::frontendFingerprint - the glslang resource limits + // BuildTBuiltInResource enforces at parse, plus the two inputs to the + // reflection vertex-attrib limit. NOT CompileEnv::fingerprint, which also + // covers backend identity and the advertised extension vector; // * per stage, in link order: the GL stage enum and the FULL preprocessed // source, which is literally the text ParseShaderSource was given; // * the four link-time request maps mapIO resolves against @@ -355,7 +364,9 @@ namespace MobileGL::MG_Util::ShaderTranspiler { StringView preprocessedSource; }; - Uint64 envFingerprint = 0; + // CompileEnv::frontendFingerprint, NEVER CompileEnv::fingerprint - see the + // backend-agnosticism note above. + Uint64 frontendFingerprint = 0; Vector stages; const UnorderedMap* explicitVertexInLocations = nullptr; const UnorderedMap* explicitFragmentOutLocations = nullptr;