From b6a44cd1e26b80df72b9ca94b867eb40479449e7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 10 Aug 2026 05:50:20 -0400 Subject: [PATCH] [Refactor] (MG_State): run reflection and link validation before SPIR-V generation - the ordering constraint retested byte-identical --- .../GLState/ProgramState/ProgramLinkTask.cpp | 61 ++++++++++++------- .../GLState/ProgramState/ProgramLinkTask.h | 12 ++-- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 32547df1..7d9c8bad 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -376,19 +376,33 @@ namespace MobileGL::MG_State::GLState { } } - // SPIR-V must be generated BEFORE buildReflection touches artifacts.program: - // reflection's live-variable analysis mutates the intermediates in ways that - // change subsequent GlslangToSpv output (observed: catastrophic uniform - // misbinding on DirectVulkan for UBO-heavy content). The old two-link pipeline - // never ran buildReflection on the SPIR-V-producing program; this order keeps - // that property with the single link. The glUniform*-to-scratch routing - // tables, in contrast, are sized and keyed by reflection results, so they are - // built strictly AFTER DoReflection. (Everything else on the reflection - // surface - locations, sampler units, block bindings/sizes - was measured - // identical in either order.) - MGLOG_D("ProgramObject %u: Starting SPIR-V generation", in.externalIndex); - GenerateSpirv(); - + // ---- everything below this line up to GenerateSpirv() is the GL query surface ---- + // + // ORDERING NOTE (rewritten 2026-08-10; the constraint it records was RETESTED, not + // dropped on a hunch). This block used to insist that SPIR-V be generated BEFORE + // buildReflection touches artifacts.program, on the grounds that reflection's + // 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. + // + // Re-measured on the glslang pin this tree vendors, with the same method 0d052719 + // used (per-module SPIR-V hashes, both orders, byte-compared): 636 modules across + // 320 programs - the whole extracted trace corpus (BSL, Complementary Reimagined, + // IterationRP, Create/Flywheel) plus adversarial synthetics - came out BYTE-IDENTICAL + // in both orders, pre-optimize and post-optimize alike. glslang's code structure + // agrees: reflection.cpp performs no AST write (no getWritableType, no const_cast, no + // qualifier assignment) and GlslangToSpv takes a const TIntermediate&. + // + // So the order is now the other way round, and deliberately: reflection, fragment + // output validation and transform-feedback resolution are what the GL query surface + // 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 = [&] { @@ -401,15 +415,8 @@ namespace MobileGL::MG_State::GLState { artifacts.infoLog)); return; } - - MGLOG_D("ProgramObject %u: Building global-UBO routing tables", in.externalIndex); - { - // TEMP-STAGE-PROBE: "spvc-routing" - the SPIRV-Cross session per SPIR-V module. - const MG_Util::Debug::TempStageProbeScope tempStageProbeSpvcRouting( - MG_Util::Debug::kTempStageProbeSpvcRouting); - BuildGlobalUboRouting(); - } MGLOG_D("ProgramObject %u: Reflection done (linkStatus=%d)", in.externalIndex, (int)artifacts.linkStatus); + if (!ValidateFragmentOutputLocations()) { return; } @@ -419,6 +426,18 @@ namespace MobileGL::MG_State::GLState { in.externalIndex, artifacts.infoLog)); return; } + + // ---- past this point the link cannot fail any more ---- + MGLOG_D("ProgramObject %u: Starting SPIR-V generation", in.externalIndex); + GenerateSpirv(); + + MGLOG_D("ProgramObject %u: Building global-UBO routing tables", in.externalIndex); + { + // TEMP-STAGE-PROBE: "spvc-routing" - the SPIRV-Cross session per SPIR-V module. + const MG_Util::Debug::TempStageProbeScope tempStageProbeSpvcRouting( + MG_Util::Debug::kTempStageProbeSpvcRouting); + BuildGlobalUboRouting(); + } MGLOG_D("ProgramObject %u: Binary generation finished (generatedSpirv size=%zu)", in.externalIndex, artifacts.generatedSpirv.size()); } diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h index 7c857a6f..4b959d11 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h @@ -40,11 +40,13 @@ namespace MobileGL::MG_State::GLState { // through the CompileEnv snapshot and diagnostics are deferred to the join. // // ONE LINK IS ONE HANDLER. RunBody() runs start to finish inside a single pool handler - // and is the only place `artifacts` is written. Do not split it across handlers to - // "pipeline" the reflection half: the intermediates that GlslangToSpv and buildReflection - // share are mutated in a strict order (see the GenerateSpirv-before-DoReflection comment - // in Run()), and a second handler would let a cancel land between them and publish a - // program whose SPIR-V and reflection describe different things. + // and is the only place `artifacts` is written. Splitting it across handlers to + // "pipeline" the reflection half would let a cancel land between the halves and publish a + // program whose SPIR-V and reflection describe different things - so any such split has + // to be structural: the first half must publish a LINK_STATUS and a query surface that + // are already final, and a lost second half must degrade to "linked but not drawable", + // never to a half-published program. (The intermediates' ordering constraint that used to + // be quoted here is retested and no longer binding; see the ordering note in RunBody.) class ProgramLinkTask final : public MG_Util::Async::JobNode { public: // ---- inputs, snapshotted on the GL thread in ProgramObject::Link()'s prologue ----