diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 0d5b0c31..3dfc63aa 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -5413,8 +5413,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // Declares* probes each cost a BuildModule per stage, and on a driver where both // gates are armed (Mali: no GL_OES_viewport_array AND integer multisample // squeezed to 1) the doubled parse made compile-heavy workloads ~10% slower. - // Probing the pre-lowering module is sound for both gates: demoting - // gl_ViewportIndex neither adds nor removes multisampled image types. + // Probing the pre-lowering module is sound for all three gates: demoting + // gl_ViewportIndex neither adds nor removes multisampled image types, and no pass + // between here and the widening changes which image FORMATS the module declares - + // Lower1DArrayImagesPass rebuilds an image type but copies its format operand across, + // and the one pass that can introduce a format (the bake) reports for itself below. // Recomputed here rather than calling GL_Getter's GetAdvertisedMaxSamples(): // this is backend code and must not reach into the GL frontend. 4 is that // translation unit's kFrontendMaxSamples, which is the source of truth - @@ -5426,8 +5429,13 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESCapabilities.MaxColorTextureSamples < advertisedMaxSamples || g_GLESCapabilities.MaxIntegerSamples < advertisedMaxSamples || g_GLESCapabilities.MaxDepthTextureSamples < advertisedMaxSamples; + // The image-format widening is armed on EVERY real device (no driver tested advertises + // GL_NV_image_formats), so its probe has to ride the shared parse rather than add one: + // it is asked of every stage of every program, and a BuildModule per stage per gate is + // exactly what cost compile-heavy CTS cases ~10% before this struct existed. + const Bool imageFormatWideningArmed = !g_GLESCapabilities.SupportsExtendedImageFormats; MG_Util::ShaderTranspiler::ShaderCompiler::SpirvGateFeatures spirvGates; - if (viewportLoweringArmed || sampleClampArmed) { + if (viewportLoweringArmed || sampleClampArmed || imageFormatWideningArmed) { spirvGates = MG_Util::ShaderTranspiler::ShaderCompiler::ProbeSpirvGateFeatures( *effectiveSpirv); } @@ -5607,6 +5615,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // whose images all declare formats, and the cheap probe keeps a program that has // an unbound format-less image from paying an optimizer round trip per stage. Vector imageFormatSpirv; + Bool bakedAWidenableImageFormat = false; if (!imageFormatBake.glFormatByUniformName.empty() && MG_Util::ShaderTranspiler::ShaderCompiler::DeclaresFormatlessStorageImage(*effectiveSpirv) && MG_Util::ShaderTranspiler::ShaderCompiler::BakeImageFormatsForEssl( @@ -5614,6 +5623,19 @@ namespace MobileGL::MG_Backend::DirectGLES { enableSpirvValidation) && !imageFormatSpirv.empty()) { effectiveSpirv = &imageFormatSpirv; + // The bake can put a format into the module that was not there when the gate + // probe above parsed it, so the probe's verdict is stale for exactly this case. + // Answered from the bake's own map rather than by re-parsing: an entry the map + // does not carry cannot have been baked. Conservative in the harmless direction - + // the pass declines individual uniforms the map names, and a widening run that + // finds nothing to widen only costs a re-serialisation of a module that already + // went through the optimizer one line above. + for (const auto& entry : imageFormatBake.glFormatByUniformName) { + if (MG_Util::ShaderTranspiler::ShaderCompiler::WidenedCoreEsslImageFormat(entry.second) != 0) { + bakedAWidenableImageFormat = true; + break; + } + } } // GL has forty image formats and GLSL ES core has thirteen; the other twenty-seven @@ -5640,8 +5662,8 @@ namespace MobileGL::MG_Backend::DirectGLES { // DirectVulkan is deliberately not given this: it takes the declared format natively // and resolves the descriptor's view format from the same bind state. Vector widenedImageFormatSpirv; - if (!g_GLESCapabilities.SupportsExtendedImageFormats && - MG_Util::ShaderTranspiler::ShaderCompiler::DeclaresWidenableImageFormat(*effectiveSpirv) && + if (imageFormatWideningArmed && + (spirvGates.DeclaresWidenableImageFormat || bakedAWidenableImageFormat) && MG_Util::ShaderTranspiler::ShaderCompiler::WidenImageFormatsForEssl( *effectiveSpirv, widenedImageFormatSpirv, enableSpirvValidation) && !widenedImageFormatSpirv.empty()) { diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 2343fcae..550bbc62 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -728,6 +728,8 @@ namespace MobileGL { LowerViewportIndexPass::DeclaresViewportIndexBuiltin(context.get()); features.DeclaresMultisampledImage = ClampMultisampleFetchPass::DeclaresMultisampledImage(context.get()); + features.DeclaresWidenableImageFormat = + WidenImageFormatsPass::DeclaresWidenableImageFormat(context.get()); return features; } diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h index 3557aeff..99497620 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -74,6 +74,7 @@ namespace MobileGL { struct SpirvGateFeatures { Bool WritesViewportIndexOutput = false; Bool DeclaresMultisampledImage = false; + Bool DeclaresWidenableImageFormat = false; }; static SpirvGateFeatures ProbeSpirvGateFeatures(const Vector& binary); // Replaces an ARRAY vertex input with one input per element at consecutive @@ -244,8 +245,10 @@ namespace MobileGL { Vector& outputBinary, bool enableSpirvValidation = false); // Whether the module declares a storage image WidenImageFormatsForEssl would - // widen. One module parse, so the ~every shader that declares none pays no - // optimizer run. + // widen, so the ~every shader that declares none pays no optimizer run. Costs its + // own module parse: the transpile path asks the same question through + // ProbeSpirvGateFeatures instead, because this gate is armed on every real driver + // and would otherwise put a BuildModule on every stage of every program. static bool DeclaresWidenableImageFormat(const Vector& binary); // The core-ESSL GL internal format that carries `glInternalFormat` exactly, or 0 // when it needs no widening or cannot be widened exactly. The single source of diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp index 07798e8d..9a47b32a 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp @@ -231,11 +231,8 @@ namespace MobileGL { return ChannelsOfSpirvImageFormat(SpirvImageFormatOfGL(glInternalFormat)); } - bool WidenImageFormatsPass::DeclaresWidenableImageFormat(const Vector& binary) { - std::unique_ptr context = spvtools::BuildModule( - SPV_ENV_VULKAN_1_1, [](spv_message_level_t, const char*, const spv_position_t&, const char*) {}, - binary.data(), binary.size()); - if (!context) { + bool WidenImageFormatsPass::DeclaresWidenableImageFormat(IRContext* context) { + if (context == nullptr) { return false; } for (const Instruction& type : context->module()->types_values()) { @@ -246,6 +243,13 @@ namespace MobileGL { return false; } + bool WidenImageFormatsPass::DeclaresWidenableImageFormat(const Vector& binary) { + std::unique_ptr context = spvtools::BuildModule( + SPV_ENV_VULKAN_1_1, [](spv_message_level_t, const char*, const spv_position_t&, const char*) {}, + binary.data(), binary.size()); + return DeclaresWidenableImageFormat(context.get()); + } + spvtools::opt::Pass::Status WidenImageFormatsPass::Process() { auto* irContext = context(); auto* defUseMgr = irContext->get_def_use_mgr(); @@ -519,8 +523,11 @@ namespace MobileGL { for (Instruction* type : imageTypes) { const auto widenedIt = widenedByTypeId.find(type->result_id()); if (widenedIt == widenedByTypeId.end()) continue; + // No def-use re-analysis: the Image Format operand is a LITERAL, so no use of + // any id moves, and the masks above already left the manager describing a + // module that has since grown instructions it was never told about. Every + // analysis is dropped below instead. type->SetInOperand(kImageFormatOperand, {static_cast(widenedIt->second.Carrier)}); - defUseMgr->AnalyzeInstUse(type); } // StorageImageExtendedFormats is deliberately left declared even though every diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h index aa280553..b9d13013 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h @@ -82,6 +82,11 @@ namespace MobileGL { // the caller can skip the optimizer run entirely - which is every shader but a // handful. static bool DeclaresWidenableImageFormat(const Vector& binary); + // The same question asked of a module the caller has ALREADY parsed, so a stage + // that has to answer several gate questions pays one BuildModule rather than one + // per gate - see ShaderCompiler::ProbeSpirvGateFeatures, and the ~10% it cost + // compile-heavy CTS cases when two gates each parsed for themselves. + static bool DeclaresWidenableImageFormat(spvtools::opt::IRContext* context); // The core-ESSL GL internal format that carries `glInternalFormat` exactly, or 0 // when the format needs no widening (it is core already) or cannot be widened