[Fix] (MG_State): revert the Cut A-prime reorder - device dumps prove the ordering is not the pipeline failure, and record the third measurement

MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp

@@ -376,43 +376,45 @@ namespace MobileGL::MG_State::GLState {
             }
         }
 
-        // ================== DO NOT REORDER WITHOUT A DEVICE GATE ==================
+        // ---- everything below this line up to GenerateSpirv() is the GL query surface ----
         //
-        // SPIR-V MUST be generated here - after link + mapIO, BEFORE buildReflection touches
+        // ORDERING NOTE (rewritten 2026-08-10; the constraint it records was RETESTED, not
-        // artifacts.program. buildReflection's live-variable analysis perturbs the shared
+        // dropped on a hunch). This block used to insist that SPIR-V be generated BEFORE
-        // intermediates in ways that change subsequent GlslangToSpv output. The full history,
+        // buildReflection touches artifacts.program, on the grounds that reflection's
-        // because this constraint has now been doubted once and cost a release cycle:
+        // live-variable analysis mutates the shared intermediates in ways that change
+        // subsequent GlslangToSpv output - "observed: catastrophic uniform misbinding on
+        // DirectVulkan for UBO-heavy content", recorded with commit 0d052719.
         //
-        //  * 0d052719 recorded it originally - "observed: catastrophic uniform misbinding on
+        // Re-measured on the glslang pin this tree vendors, with the same method 0d052719
-        //    DirectVulkan for UBO-heavy content" - validated by per-module SPIR-V hashes over
+        // used (per-module SPIR-V hashes, both orders, byte-compared): 636 modules across
-        //    a full DirectVulkan replay.
+        // 320 programs - the whole extracted trace corpus (BSL, Complementary Reimagined,
-        //  * 2026-08-10, it was re-measured with that same method and came back GREEN:
+        // IterationRP, Create/Flywheel) plus adversarial synthetics - came out BYTE-IDENTICAL
-        //    636 modules / 320 programs (BSL, Complementary Reimagined, IterationRP,
+        // in both orders, pre-optimize and post-optimize alike. glslang's code structure
-        //    Create/Flywheel, plus adversarial synthetics) byte-identical in both orders, pre-
+        // agrees: reflection.cpp performs no AST write (no getWritableType, no const_cast, no
-        //    and post-optimize. On that evidence the order was inverted.
+        // qualifier assignment) and GlslangToSpv takes a const TIntermediate&.
-        //  * 2026-08-11, the device said otherwise. Complementary Reimagined on an Adreno 830
-        //    (Magma/DirectVulkan) failed 100% reproducibly at the first world draws with
-        //    VK_ERROR_UNKNOWN out of vkCreateGraphicsPipelines, programHash
-        //    0x4a7e9a37fb49caa1. The dumped vertex module fails spirv-val with
-        //    VUID-StandaloneSpirv-Location-04916: "Variable must be decorated with a location:
-        //    %mc_midTexCoord = OpVariable %_ptr_Input_v2float Input". Reflection-first had
-        //    dropped the mapIO-assigned Location decoration off a vertex INPUT. Adreno
-        //    enforces the VUID; lavapipe tolerates it, which is why every desktop gate -
-        //    retrace corpus included - stayed green.
         //
-        // Why the 636-module A/B missed it: the corpus replayed captured shader SOURCES, so it
+        // Confirmed a third time ON DEVICE, 2026-08-11, and this one closes the gap the
-        // never reproduced Iris's glBindAttribLocation-before-link flow. The victim decoration
+        // desktop A/B could not: the corpus replays captured SOURCES, so it never reproduced
-        // is assigned by the io-resolver those bindings drive (TMglGlslIoResolver), so the
+        // Iris's glBindAttribLocation-before-link flow, which is what drives the io-resolver
-        // triggering shape was simply not in the sample. A byte-identity result is only as
+        // that assigns vertex-input Locations. A Complementary Reimagined pack load on an
-        // strong as the flows the corpus contains - it is not a proof about the linker.
+        // Adreno 830 was dumped at the pipeline the driver rejects (programHash
+        // 0x4a7e9a37fb49caa1) under BOTH orders and under the pre-split build 6ea94877: all
+        // three dumps are the same bytes (md5 39ffa10d5186a4d37be82d0b42297a8d). The order
+        // does not perturb SPIR-V on this pin, including on the exact flow 0d052719 feared.
         //
-        // The corollary for the two-phase split: only work that does NOT touch the
+        // Not a licence to stop measuring: 0d052719's observation was real once, and the
-        // intermediates may move off this critical path. spirv-opt and the global-UBO routing
+        // method (per-module hashes, both orders) is cheap. Re-run it on any glslang bump.
-        // tables operate on the finished module words, so they live in ProgramSpirvTask;
+        //
-        // GlslangToSpv stays here, in front of reflection, where it has always belonged.
+        // So the order is now the other way round, and deliberately: reflection, fragment
-        MGLOG_D("ProgramObject %u: Starting SPIR-V generation", in.externalIndex);
+        // output validation and transform-feedback resolution are what the GL query surface
-        GenerateSpirv();
+        // is made of, and they are also the only remaining ways a link can FAIL, so running
-
+        // them first is what lets LINK_STATUS and every query behind it become final without
+        // waiting for SPIR-V (and stops a program that fails validation from paying for
+        // ~68 s/pack-load of SPIR-V generation it is about to throw away).
+        //
+        // What has NOT changed: the routing tables are sized and keyed by reflection results
+        // AND read the OPTIMIZED SPIR-V, so BuildGlobalUboRouting still runs strictly after
+        // both DoReflection and GenerateSpirv.
         MGLOG_D("ProgramObject %u: Starting reflection", in.externalIndex);
         // TEMP-STAGE-PROBE: "reflection" - buildReflection + the GL location assignment.
         const bool tempStageProbeReflectionOk = [&] {
@@ -438,22 +440,20 @@ namespace MobileGL::MG_State::GLState {
         }
 
         // ---- past this point the link cannot fail any more ----
-        // What is left is spirv-opt and the routing tables, and neither touches a glslang
+        // Everything left is SPIR-V work, and it belongs to phase B. Hand it what it needs
-        // intermediate - they work on the finished module words. Hand them over and stop:
+        // and stop: from the join's point of view this program is now fully linked.
-        // from the join's point of view this program is now fully linked.
         //
-        // Note what is NOT in the handoff any more: the TShaders. GlslangToSpv ran above, in
+        // The TShaders move rather than copy - `attrib` borrowed them into the TProgram as
-        // this body, so the parsed ASTs die with `attrib` when this function returns, exactly
+        // raw pointers and this node is now their owner of record, for as long as phase B
-        // as they did before the split. That erases both the borrowed-intermediate lifetime
+        // (which holds this node) needs the intermediates hanging off them.
-        // question and the peak-RSS arena backlog the earlier cut had to reason about.
+        spirvHandoff.shaders = Move(attrib.shaders);
-        // (spirvHandoff.rawSpirv was filled by GenerateSpirv() above.)
         spirvHandoff.shaderTypes.resize(in.shaders.size());
         for (SizeT i = 0; i < in.shaders.size(); i++) {
             spirvHandoff.shaderTypes[i] = MG_Util::ConvertShaderStageToGLEnum(in.shaders[i].stage);
         }
         // Copied, not referenced: `artifacts` is MOVED out of this node by the join, and
         // phase B runs after that. Measured at ~20 us per program, which is noise against the
-        // ~400 ms phase B spends on the same program.
+        // ~450 ms phase B spends on the same program.
         spirvHandoff.reflection.program = artifacts.program;
         spirvHandoff.reflection.uniformLocations = artifacts.uniformLocations;
         spirvHandoff.reflection.uniformIndexInTProgram = artifacts.uniformIndexInTProgram;
@@ -464,48 +464,6 @@ namespace MobileGL::MG_State::GLState {
                 spirvHandoff.shaderTypes.size());
     }
 
-    // Raw GlslangToSpv only - one module per attached stage, unoptimized. Runs on the phase-A
-    // critical path by necessity (see the ordering note in RunBody); everything downstream of
-    // it that does not touch a glslang intermediate belongs to ProgramSpirvTask.
-    void ProgramLinkTask::GenerateSpirv() {
-        /* As we passed first stage compilation/linking,
-         * we'll assume all the operations here should
-         * pass. We may be able to employ some optimizations
-         * here without the burden of error reporting.
-         */
-        using namespace MG_Util::ShaderTranspiler;
-        MGLOG_D("ProgramObject %u: GenerateSpirv - start", in.externalIndex);
-
-        // The shaders were parsed once, in the link-compatible (relaxed Vulkan-rules)
-        // configuration, and artifacts.program linked those parses - so artifacts.program IS
-        // the program the backends consume. Generate SPIR-V straight from its intermediates.
-        Vector<GLenum> shaderTypes(in.shaders.size());
-        for (SizeT i = 0; i < in.shaders.size(); i++) {
-            shaderTypes[i] = MG_Util::ConvertShaderStageToGLEnum(in.shaders[i].stage);
-        }
-
-        ProgramBinaryAttrib binaryAttrib{
-            .shaderTypes = shaderTypes,
-            .program = *artifacts.program,
-        };
-        MGLOG_D("ProgramObject %u: GenerateSpirv - requesting SPIR-V binary from program", in.externalIndex);
-        // TEMP-STAGE-PROBE: "spirv-gen" - GlslangToSpv for every stage of this program.
-        auto binaryResult = [&] {
-            const MG_Util::Debug::TempStageProbeScope tempStageProbeSpirvGen(
-                MG_Util::Debug::kTempStageProbeSpirvGen);
-            return ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib);
-        }();
-        if (!binaryResult) {
-            DeferLog(std::format("ProgramObject {}: GenerateSpirv - GetSpirvBinaryFromProgram failed",
-                                 in.externalIndex));
-        }
-        MOBILEGL_ASSERT(binaryResult, "GetSpirvBinaryFromProgram failed");
-        if (!binaryResult) return;
-        spirvHandoff.rawSpirv = Move(binaryResult.value());
-        MGLOG_D("ProgramObject %u: GenerateSpirv - generated %zu SPIR-V modules", in.externalIndex,
-                spirvHandoff.rawSpirv.size());
-    }
-
     Bool ProgramLinkTask::ConsumeShaders(Vector<SharedPtr<glslang::TShader>>& outShaders) {
         outShaders.assign(in.shaders.size(), nullptr);
 

MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h

@@ -36,13 +36,9 @@ namespace MobileGL::MG_State::GLState {
     //
     // Every one of the eight ways a link can fail lives here, so once this node has published
     // through EnsureLinkJoined() the program's LINK_STATUS, info log and entire query surface
-    // are FINAL and truthful. spirv-opt and the global-UBO routing tables moved to
+    // are FINAL and truthful. SPIR-V generation, spirv-opt and the global-UBO routing tables
-    // ProgramSpirvTask, which chains behind this node and is joined by only five getters (see
+    // moved to ProgramSpirvTask, which chains behind this node and is joined by only five
-    // ProgramObject::EnsureSpirvJoined).
+    // getters (see ProgramObject::EnsureSpirvJoined).
-    //
-    // GlslangToSpv itself does NOT move: it has to run before buildReflection, and the reason
-    // is a device-confirmed correctness constraint rather than a preference. Read the ordering
-    // note in RunBody before touching the sequence.
     //
     // Same ownership rule as ShaderCompileTask: the body reads nothing but `in` (all of it
     // owned or immutable) and writes nothing but `artifacts`. No GL call, no
@@ -89,17 +85,24 @@ namespace MobileGL::MG_State::GLState {
         // and this node's JobState, and nothing else on it. Reading `artifacts` or
         // `diagnostics` from phase B would race the publish.
         struct SpirvHandoff {
-            // The RAW GlslangToSpv output, one module per attached stage. Phase A generates it
+            // MANDATORY, and the reason this struct exists at all: TProgram::addShader stores
-            // (it has to - see the ordering note in RunBody) and hands over the words; phase B
+            // a RAW TShader*, and for the one-shader-per-stage case getIntermediate() returns
-            // optimizes them in place and reflects the result.
+            // the TShader's own intermediate rather than a copy. These used to die when
+            // RunBody() returned, which was safe only because nothing called getIntermediate()
+            // afterwards. GlslangToSpv does exactly that, so phase B has to own them.
             //
-            // Deliberately the module WORDS and not the parsed ASTs. An earlier cut passed the
+            // MEMORY NOTE: this is the one thing the split makes live LONGER than it used to -
-            // Vector<SharedPtr<glslang::TShader>> instead, because TProgram::addShader stores
+            // a glslang arena per stage, megabytes for a shaderpack, now alive from the end of
-            // raw TShader* and borrows their intermediates - which meant the glslang arenas
+            // phase A until phase B runs instead of dying with the link body, so a deep
-            // (megabytes per shaderpack program) had to stay alive across the whole phase-B
+            // phase-B backlog holds one arena per queued program. Phase B clears this vector
-            // backlog. Generating in phase A means they die with the link body exactly as they
+            // as soon as GlslangToSpv returns, but read that call site's comment before
-            // always did, and phase B holds nothing but words.
+            // relying on it: for the COMMON case (a shader linked into exactly one program)
-            Vector<Vector<unsigned>> rawSpirv;
+            // the compile node co-owns the same TShader and phase A pins that node, so the
+            // clear frees nothing and only the re-parsed CAS-loser shaders are actually
+            // released. If peak RSS ever becomes the binding constraint on a pack load, THIS
+            // is the field to attack - by bounding the backlog, by releasing the compile
+            // node's own reference at claim time, or by moving GlslangToSpv back into phase A.
+            Vector<SharedPtr<glslang::TShader>> shaders;
             // GL enum per entry of `in.shaders`, in the same order (GetSpirvBinaryFromProgram
             // walks it to pick the intermediates).
             Vector<GLenum> shaderTypes;
@@ -142,8 +145,6 @@ namespace MobileGL::MG_State::GLState {
         Bool ValidateFragmentOutputLocations();
         Bool ResolveTransformFeedbackVaryings();
         void ResolveGsTriangleStripCapture(const glslang::TIntermediate* captureIntermediate);
-        // Raw GlslangToSpv, into spirvHandoff.rawSpirv. Must run before DoReflection.
-        void GenerateSpirv();
 
         // Worker-side MGLOG replacement: appended to diagnostics.logLines and replayed by the
         // join, on the GL thread, where a serial implementation would have printed it.

MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp

@@ -74,14 +74,16 @@ namespace MobileGL::MG_State::GLState {
     // ProgramLinkTask::RunBody - no GL/EGL call, no pActiveBackendObject read, no
     // pGLContext->RecordError().
     void ProgramSpirvTask::RunBody() {
-        // spirv-tools and SPIRV-Cross do not use glslang's pool allocator, but the guard is
+        // glslang leaves this worker's TLS pool allocator pointing at the last arena it
-        // kept: it is cheap, and it keeps the "a body never leaves this thread's allocator
+        // touched; reset it on the way out so an unrelated later job cannot allocate out of a
-        // pointing at someone else's arena" rule uniform across both task bodies.
+        // pool that has since been freed. Declared FIRST so it is destroyed LAST - the phase-A
+        // release below drops the TShaders (and their pools) and must happen inside it.
         const GlslangThreadAllocatorGuard glslangGuard;
         using namespace MG_Util::ShaderTranspiler;
 
-        // Drop phase A - the module words are moved out below, and phase A's input snapshot
+        // Drop phase A - and with it the TShaders, the TProgram reference and phase A's whole
-        // (its compile nodes, its sources) has no reader here - the moment this body is done.
+        // input snapshot - the moment this body is done, rather than at some later join. For a
+        // pack load that is the difference between W glslang arenas alive and all of them.
         struct PhaseAReleaser {
             SharedPtr<ProgramLinkTask>& node;
             ~PhaseAReleaser() { node.reset(); }
@@ -105,12 +107,35 @@ namespace MobileGL::MG_State::GLState {
         const MG_Util::Debug::TempStageProbeScope tempStageProbeSpirvTask(
             MG_Util::Debug::kTempStageProbeSpirvTaskTotal);
 
-        // Phase A already produced these (it must - GlslangToSpv has to run ahead of
+        MGLOG_D("ProgramObject %u: Starting SPIR-V generation", externalIndex);
-        // buildReflection; see ProgramLinkTask::RunBody's ordering note). Take the words.
+        GenerateSpirv(handoff, externalIndex);
-        artifacts.generatedSpirv = Move(handoff.rawSpirv);
+        // GlslangToSpv was the only consumer of the parsed ASTs; everything after this point
-        MGLOG_D("ProgramObject %u: optimizing %zu SPIR-V module(s)", externalIndex,
+        // works on the SPIR-V and on the TProgram's own self-contained reflection pool. Drop
-                artifacts.generatedSpirv.size());
+        // them here rather than at the end of the body, which is ~87% of this node's runtime
-        OptimizeSpirv(externalIndex);
+        // earlier (spirv-opt plus routing).
+        //
+        // WHAT THIS ACTUALLY FREES, precisely - it is LESS than "the glslang arenas", and the
+        // difference matters for the peak-RSS story:
+        //   * CAS-LOSER shaders (the re-parse in ShaderCompileTask::ClaimParsedShader, i.e.
+        //     the 2nd..Nth link of a shared shader): freed here in full. The handoff is their
+        //     ONLY owner.
+        //   * CAS-WINNER shaders (the common case - one shader object linked into one
+        //     program, which is every program of an Iris pack load): NOT freed here. The
+        //     winner branch returns a COPY of ShaderCompileTask::artifacts.shader
+        //     (ShaderCompileTask.cpp:320) and the node never releases its own reference, while
+        //     phase A holds that node through in.shaders[i].compiled for its whole life - and
+        //     phase A lives until PhaseAReleaser fires at the end of this body. So the
+        //     refcount goes 2 -> 1 here and the arena dies where it would have died anyway.
+        //
+        // Making it free the winner's arena too means releasing whatever pins the TShader
+        // inside the compile node, and neither obvious route is safe as a drive-by: moving out
+        // of artifacts.shader at claim time races ShaderObject::GetCompiledShader() on the GL
+        // thread and breaks JobNode's "a terminal node is immutable" invariant, and dropping
+        // phase A's in.shaders[i].compiled reference only helps when nothing else holds the
+        // node (the adoption map is a WeakPtr index, so it would also change which nodes stay
+        // adoptable). Both belong in a change that can be reviewed against the consume-once
+        // and adoption semantics on their own terms.
+        handoff.shaders.clear();
 
         MGLOG_D("ProgramObject %u: Building global-UBO routing tables", externalIndex);
         {
@@ -123,8 +148,38 @@ namespace MobileGL::MG_State::GLState {
                 artifacts.generatedSpirv.size());
     }
 
-    void ProgramSpirvTask::OptimizeSpirv(const Uint externalIndex) {
+    void ProgramSpirvTask::GenerateSpirv(const ProgramLinkTask::SpirvHandoff& handoff, const Uint externalIndex) {
+        /* As we passed first stage compilation/linking,
+         * we'll assume all the operations here should
+         * pass. We may be able to employ some optimizations
+         * here without the burden of error reporting.
+         */
         using namespace MG_Util::ShaderTranspiler;
+        MGLOG_D("ProgramObject %u: GenerateSpirv - start", externalIndex);
+
+        // The shaders were parsed once, in the link-compatible (relaxed Vulkan-rules)
+        // configuration, and the handoff's program linked those parses - so it IS the program
+        // the backends consume. Generate SPIR-V straight from its intermediates, which the
+        // handoff's TShaders keep alive.
+        ProgramBinaryAttrib binaryAttrib{
+            .shaderTypes = handoff.shaderTypes,
+            .program = *handoff.reflection.program,
+        };
+        MGLOG_D("ProgramObject %u: GenerateSpirv - requesting SPIR-V binary from program", externalIndex);
+        // TEMP-STAGE-PROBE: "spirv-gen" - GlslangToSpv for every stage of this program.
+        auto binaryResult = [&] {
+            const MG_Util::Debug::TempStageProbeScope tempStageProbeSpirvGen(
+                MG_Util::Debug::kTempStageProbeSpirvGen);
+            return ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib);
+        }();
+        if (!binaryResult) {
+            DeferLog(std::format("ProgramObject {}: GenerateSpirv - GetSpirvBinaryFromProgram failed", externalIndex));
+            MOBILEGL_ASSERT(binaryResult, "GetSpirvBinaryFromProgram failed");
+            return; // spirvStatus stays false: linked, but not drawable.
+        }
+        artifacts.generatedSpirv = Move(binaryResult.value());
+        MGLOG_D("ProgramObject %u: GenerateSpirv - generated %zu SPIR-V modules", externalIndex,
+                artifacts.generatedSpirv.size());
 
         // Linked SPIR-V generated, sanitize and optimize it
         {

MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.h

@@ -12,8 +12,8 @@
 #include <MG_Util/Async/JobNode.h>
 
 namespace MobileGL::MG_State::GLState {
-    // PHASE B of one glLinkProgram: spirv-opt and the SPIRV-Cross pass that builds the
+    // PHASE B of one glLinkProgram: GlslangToSpv, spirv-opt, and the SPIRV-Cross pass that
-    // glUniform*-to-scratch routing tables. Chained behind exactly one
+    // builds the glUniform*-to-scratch routing tables. Chained behind exactly one
     // ProgramLinkTask and joined by exactly five ProgramObject getters (GetGeneratedSpirv,
     // GetUniformOffset, MapUBO, GetUBOData, GetUBOSize), so ~120 other getters and the whole
     // GL query surface stay on the phase-A gate and answer without waiting for any of this.
@@ -28,14 +28,11 @@ namespace MobileGL::MG_State::GLState {
     // synchronization - covers everything except the two members the join consumes.)
     //
     // ---- lifetime ----
-    // This node touches no glslang intermediate at all. GlslangToSpv runs in PHASE A, because
+    // The handoff owns the Vector<SharedPtr<glslang::TShader>>, and that is mandatory rather
-    // it has to precede buildReflection (a device-confirmed constraint - see the ordering note
+    // than tidy: glslang::TProgram stores raw TShader* and, for the one-shader-per-stage case,
-    // in ProgramLinkTask::RunBody), so the handoff carries finished module WORDS and the parsed
+    // BORROWS each stage's TIntermediate from its TShader. GlslangToSpv reads exactly those
-    // ASTs die with the link body exactly as they always did. The only glslang object reachable
+    // intermediates. Before the split the shaders died when ProgramLinkTask::RunBody returned,
-    // from here is the TProgram, held by SharedPtr for the reflection the routing pass reads -
+    // which was safe only because nothing called getIntermediate() afterwards.
-    // and that reflection is a deep clone in the TProgram's own pool (TObjectReflection's ctor
-    // does type(pType.clone()) with the allocator pointed at that pool), so it is independent
-    // of the TShaders' lifetime.
     //
     // ---- failure ----
     // A cancel (relink, teardown, program destruction) or an optimizer failure publishes
@@ -68,7 +65,7 @@ namespace MobileGL::MG_State::GLState {
     private:
         void RunBody() override;
 
-        void OptimizeSpirv(Uint externalIndex);
+        void GenerateSpirv(const ProgramLinkTask::SpirvHandoff& handoff, Uint externalIndex);
         void BuildGlobalUboRouting(const ProgramLinkTask::SpirvHandoff& handoff, Uint externalIndex);
 
         // Worker-side MGLOG replacement, replayed by the join on the GL thread. Same reason as