[Perf] (DirectGLES, ShaderTranspiler): fold the image-widening gate into the shared SPIR-V probe

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
+            // Probing the pre-lowering module is sound for all three gates: demoting
-            // gl_ViewportIndex neither adds nor removes multisampled image types.
+            // 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<unsigned int> 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<unsigned int> widenedImageFormatSpirv;
-            if (!g_GLESCapabilities.SupportsExtendedImageFormats &&
+            if (imageFormatWideningArmed &&
-                MG_Util::ShaderTranspiler::ShaderCompiler::DeclaresWidenableImageFormat(*effectiveSpirv) &&
+                (spirvGates.DeclaresWidenableImageFormat || bakedAWidenableImageFormat) &&
                 MG_Util::ShaderTranspiler::ShaderCompiler::WidenImageFormatsForEssl(
                     *effectiveSpirv, widenedImageFormatSpirv, enableSpirvValidation) &&
                 !widenedImageFormatSpirv.empty()) {

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;
             }
 

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<Uint32>& binary);
                 // Replaces an ARRAY vertex input with one input per element at consecutive
@@ -244,8 +245,10 @@ namespace MobileGL {
                                                      Vector<uint32_t>& 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
+                // widen, so the ~every shader that declares none pays no optimizer run. Costs its
-                // optimizer run.
+                // 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<Uint32>& 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

MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp

@@ -231,11 +231,8 @@ namespace MobileGL {
                 return ChannelsOfSpirvImageFormat(SpirvImageFormatOfGL(glInternalFormat));
             }
 
-            bool WidenImageFormatsPass::DeclaresWidenableImageFormat(const Vector<Uint32>& binary) {
+            bool WidenImageFormatsPass::DeclaresWidenableImageFormat(IRContext* context) {
-                std::unique_ptr<IRContext> context = spvtools::BuildModule(
+                if (context == nullptr) {
-                    SPV_ENV_VULKAN_1_1, [](spv_message_level_t, const char*, const spv_position_t&, const char*) {},
-                    binary.data(), binary.size());
-                if (!context) {
                     return false;
                 }
                 for (const Instruction& type : context->module()->types_values()) {
@@ -246,6 +243,13 @@ namespace MobileGL {
                 return false;
             }
 
+            bool WidenImageFormatsPass::DeclaresWidenableImageFormat(const Vector<Uint32>& binary) {
+                std::unique_ptr<IRContext> 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<uint32_t>(widenedIt->second.Carrier)});
-                    defUseMgr->AnalyzeInstUse(type);
                 }
 
                 // StorageImageExtendedFormats is deliberately left declared even though every

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<Uint32>& 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