diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index c1979280..1f439970 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -131,6 +131,21 @@ namespace MobileGL::MG_Backend::DirectVulkan { } namespace { +#if !MOBILEGL_BUILD_DISAGGREGATED + // P7 wave 2 package C, OQ-8: THE PULL BUILD'S HALF, VERBATIM. A disaggregated build + // answers GetShaderStorageBlock{Index,Binding} out of LinkArtifacts::storageBlocks + // instead (see those functions below), which retires this whole cache - the map, the + // SPIRV-Reflect rebuild, the teardown clear and the rehash hazard. It cannot be + // retired HERE because the archive member it reads is itself disaggregated-only, for + // G1: one more Vector in LinkArtifacts moves the pull build's .text, which is pinned + // byte-for-byte at 0xa52203. + // + // NOTHING OUTSIDE THIS FILE READS ANY OF IT (measured: BufferVariableResource, + // bufferVariables, activeVariables and StorageBlockResource have no reference anywhere + // else in the tree, and `dataSize` is written and never read - GL_BUFFER_VARIABLE and + // GL_ACTIVE_VARIABLES are answered by MG_Impl/GLImpl/Program/ProgramInterface.cpp out + // of glslang's own reflection, which is a different index space). So the two arms + // below differ in exactly the two functions their callers use, and in nothing else. struct BufferVariableResource { String name; GLuint blockIndex = 0; @@ -162,6 +177,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Vector storageBlocks; Vector bufferVariables; }; +#endif // !MOBILEGL_BUILD_DISAGGREGATED struct DrawElementsIndirectCommand { Uint32 count = 0; @@ -178,12 +194,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 baseInstance = 0; }; +#if !MOBILEGL_BUILD_DISAGGREGATED // Keyed by GL program name so the freed-name reuse in IndexGenerator bounds the // map at the peak-simultaneous-program high-water mark; each slot's ownership is // checked against the program's lifetime id before it is served (see // GetProgramResourceCache). Cleared wholesale at EGL teardown via // ClearProgramResourceCaches. UnorderedMap g_programResourceCaches; +#endif void ClearReadPixelsOutput(GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels) { if (!pixels || width <= 0 || height <= 0) { @@ -198,6 +216,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } +#if !MOBILEGL_BUILD_DISAGGREGATED String NormalizeDescriptorName(const SpvReflectDescriptorBinding& binding) { const char* rawName = binding.name; if (binding.type_description != nullptr && binding.type_description->type_name != nullptr) { @@ -348,6 +367,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } return cache; } +#endif // !MOBILEGL_BUILD_DISAGGREGATED #if MOBILEGL_BUILD_DISAGGREGATED // The verb's handles identify server stores. Readback orders GPU-produced @@ -448,6 +468,67 @@ namespace MobileGL::MG_Backend::DirectVulkan { } // namespace +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 wave 2 package C, OQ-8 (CONTRACT-P7 §5.3): THE MONOLITH CONSUMER READS THE ARCHIVE + // TOO, so one published list serves both of this backend's arms. + // + // THE TWO ARMS BELOW ARE NOT TWO ANSWERS. ProgramTest's + // TheArchivesStorageBlockOrderIsTheOneSpirvReflectProduces runs the OLD arm's algorithm - + // SPIRV-Reflect over the real modules, sorted and deduplicated exactly as + // GetProgramResourceCache does it - over a multi-stage program with an arrayed SSBO and an + // atomic-counter block, and asserts element-for-element equality with what the archive + // published. That equivalence is what makes an #if here a build-time SELECTION rather than + // a behavioural fork, and it is the reason the case exists. + // + // WHY AN #if AT ALL, AND WHAT IT COSTS. LinkArtifacts::storageBlocks is + // MOBILEGL_BUILD_DISAGGREGATED-only, because one more Vector member changes the struct's + // size, its implicit destructor and its move constructor in the PULL build whose .text G1 + // pins byte-for-byte (0xa52203). So the pull build keeps `g_programResourceCaches`, the + // SPIRV-Reflect rebuild and `ClearProgramResourceCaches`, verbatim; only this build drops + // them - and with them the rehash hazard the ordering comment below the block-binding + // setter documents. Retiring the pull half is the day the member stops needing its guard, + // which is a G1/P13 decision and not a wave-2 one; recorded in notes/p7/magma-c.md. + void ClearProgramResourceCaches() { + // Nothing to clear: there is no cache in this build. Kept as an entry point so + // BackendObject_DirectVulkan's two EGL-teardown call sites stay statement for + // statement what they are in the pull build. + } + + GLuint GetShaderStorageBlockIndex(const MG_State::GLState::ProgramObject& program, const String& name) { + const auto& blocks = program.GetLinkReflection().storageBlocks; + const auto find = [&blocks](const String& key) { + return std::find_if(blocks.begin(), blocks.end(), + [&](const MG_State::GLState::StorageBlockReflection& block) { return block.name == key; }); + }; + auto it = find(name); + if (it == blocks.end()) { + // Archive names are normalised, so an arrayed block that GL enumerates per element + // - "B[0]", "B[1]" - is one entry here, spelled "B". Same second chance the cache + // gave, for the same callers. + const auto bracket = name.rfind('['); + if (bracket == String::npos || name.empty() || name.back() != ']') return GL_INVALID_INDEX; + it = find(name.substr(0, bracket)); + if (it == blocks.end()) return GL_INVALID_INDEX; + } + return static_cast(std::distance(blocks.begin(), it)); + } + + GLuint GetShaderStorageBlockBinding(const MG_State::GLState::ProgramObject& program, GLuint blockIndex) { + const auto& blocks = program.GetLinkReflection().storageBlocks; + if (blockIndex >= blocks.size()) { + return 0; + } + // THE OVERRIDE IS APPLIED ON READ, where the cache used to apply it on rebuild and + // patch it in place on rebind. Same answer by a shorter route: the program is the + // authoritative record of a rebound block (it is what GL_BUFFER_BINDING reports), the + // archive carries the DECLARED binding only, and asking the program here means a + // rebind can no longer be lost to a cache rebuild that happened to race a state-version + // bump. + const Int rebound = program.GetShaderStorageBlockBindingOverride(blocks[blockIndex].name); + if (rebound >= 0) return static_cast(rebound); + return blocks[blockIndex].binding; + } +#else void ClearProgramResourceCaches() { // Called from EGL teardown while the backend's m_eglStateMutex is held; GL // calls are serialized in this codebase (contexts migrate threads but never @@ -483,6 +564,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } return cache.storageBlocks[blockIndex].binding; } +#endif void ClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) { MOBILEGL_ASSERT(pVulkanRenderer, "DirectVulkan::ClearBufferfi called with null VulkanRenderer"); @@ -949,6 +1031,27 @@ namespace MobileGL::MG_Backend::DirectVulkan { MakeUnique("DirectVulkan", __func__, "Shader storage binding is out of range.")); return; } +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 wave 2 package C, OQ-8: NOTHING LEFT TO PATCH, and the hazard goes with it. + // + // The frontend has already recorded the new binding on the program, and that record is + // now what GetShaderStorageBlockBinding reads (it asks + // GetShaderStorageBlockBindingOverride on every call, over an IMMUTABLE archive list). + // What stood here was the other half of a mutable cache: resolve the index, take a + // reference into g_programResourceCaches and patch `binding` in place so an + // already-built entry need not be thrown away. + // + // THE HAZARD THAT CAME WITH IT IS GONE TOO, and it is worth naming because it was a + // real crash rather than a theoretical one: GetShaderStorageBlockIndex re-entered + // GetProgramResourceCache, which indexes an OPEN-ADDRESSED map and can therefore + // insert, and a rehash moves entries - so a reference taken before that call dangled. + // Binding one program's storage block while another program's entry was still absent + // from the cache was a reproducible segfault (ProgramPipelineScenario's two + // storage-block cases, in one process). The ordering above was the fix; having no + // cache is the retirement. The pull build below keeps both. + (void)storageBlockBinding; + } +#else // The frontend already validated that the name denotes an active block, and has // already recorded the new binding on the program - which is what reseeds this cache // whenever it is rebuilt. Writing the entry here as well keeps an ALREADY-BUILT cache @@ -967,6 +1070,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (blockIndex >= cache.storageBlocks.size()) return; cache.storageBlocks[blockIndex].binding = storageBlockBinding; } +#endif void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels) { MOBILEGL_ASSERT(pVulkanRenderer, "DirectVulkan::ReadPixels called with null VulkanRenderer"); MOBILEGL_ASSERT(MGB_CTX_LIVE, "DirectVulkan::ReadPixels called with null GL context"); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaProgramSource.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaProgramSource.h index 9b671cbd..e1ba417f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaProgramSource.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaProgramSource.h @@ -7,7 +7,11 @@ #include #include #include "../DirectVulkanResourceState.h" -#include +// P7 wave 2 package C, OQ-8: spirv_reflect.h is GONE from this header, and its absence is the +// gate. Everything this class needed it for - the storage-block index space - now travels in +// the archive (LinkArtifacts::storageBlocks), so the SPIR-V reflector is off the Magma wire +// draw path entirely. A future edit that re-adds the include is re-adding a per-draw reflect, +// because a MagmaProgramSource is constructed once per draw and can memoise nothing. #endif namespace MobileGL::MG_Backend::DirectVulkan { @@ -153,30 +157,49 @@ namespace MobileGL::MG_Backend::DirectVulkan { GLenum GetTransformFeedbackBufferMode() const { return IsWire() ? Link().xfbBufferMode : m_frontend->GetTransformFeedbackBufferMode(); } + // P7 wave 2 package C, OQ-8 (CONTRACT-P7 §5.3): READ OUT OF THE ARCHIVE. + // + // What stood here re-ran spvReflectCreateShaderModule over EVERY stage module of the + // program, and it did so PER DRAW: a MagmaProgramSource is a borrowed view that + // WireDraw.inc's WireProgramSource constructs afresh for each wire draw, so the lazy + // `m_storageBlocksReady` latch could never outlive one. The list it rebuilt is a pure + // function of the linked program, so it is now built once at link time and travels in + // the archive (LinkArtifacts::storageBlocks), which takes the SPIR-V reflector off the + // wire draw path entirely - this header no longer includes spirv_reflect.h at all. Uint GetShaderStorageBlockIndex(const String& name) const { if (!IsWire()) return DirectVulkan::GetShaderStorageBlockIndex(*m_frontend, name); - EnsureStorageBlocks(); - String key = name; - auto find = [&] { - return std::find_if(m_storageBlocks.begin(), m_storageBlocks.end(), - [&](const StorageBlock& block) { return block.Name == key; }); + const auto& blocks = Link().storageBlocks; + // The archive carries NORMALISED names (the array subscript stripped), because + // that is the spelling SPIRV-Reflect's type_name gives and the spelling + // ProgramFactory::ReflectLayout looks up. A caller that still holds a GL-style + // "Foo[2]" gets the same second chance it always had. + const auto find = [&blocks](const String& key) { + return std::find_if(blocks.begin(), blocks.end(), + [&](const MG_State::GLState::StorageBlockReflection& block) { + return block.name == key; + }); }; - auto found = find(); - if (found == m_storageBlocks.end() && !name.empty() && name.back() == ']') { + auto found = find(name); + if (found == blocks.end() && !name.empty() && name.back() == ']') { const auto bracket = name.rfind('['); - if (bracket != String::npos) { key = name.substr(0, bracket); found = find(); } + if (bracket != String::npos) found = find(name.substr(0, bracket)); } - return found == m_storageBlocks.end() ? GL_INVALID_INDEX : static_cast(found - m_storageBlocks.begin()); + return found == blocks.end() ? GL_INVALID_INDEX + : static_cast(found - blocks.begin()); } Uint GetShaderStorageBlockBinding(Uint index) const { if (!IsWire()) return DirectVulkan::GetShaderStorageBlockBinding(*m_frontend, index); - EnsureStorageBlocks(); - if (index >= m_storageBlocks.size()) return 0; - const auto& block = m_storageBlocks[index]; + const auto& blocks = Link().storageBlocks; + if (index >= blocks.size()) return 0; + const auto& block = blocks[index]; + // THE OVERRIDE STILL WINS, and it still comes off the RECORD rather than the + // archive: glShaderStorageBlockBinding is a post-link mutable and the archive + // carries link-time defaults only (rule on MagmaProgramSource, and the reason + // StorageBlockReflection::binding is documented as the DECLARED binding). for (const auto& override : m_record->StorageOverrides) { - if (override.Name == block.Name) return override.Binding; + if (override.Name == block.name) return override.Binding; } - return block.Binding; + return block.binding; } private: @@ -187,48 +210,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { return tIndex >= 0 && static_cast(tIndex) < Link().blockReflection.size() ? &Link().blockReflection[tIndex] : nullptr; } - struct StorageBlock { String Name; Uint Binding; }; - void EnsureStorageBlocks() const { - if (m_storageBlocksReady) return; - m_storageBlocksReady = true; - // Match the legacy resource-query index order: stage order, then - // normalized descriptor name/binding, deduplicated across stages. - for (const auto& spirv : GetGeneratedSpirv()) { - if (spirv.empty()) continue; - SpvReflectShaderModule module{}; - if (spvReflectCreateShaderModule(spirv.size() * sizeof(Uint), spirv.data(), &module) != SPV_REFLECT_RESULT_SUCCESS) continue; - Uint32 count = 0; - if (spvReflectEnumerateDescriptorBindings(&module, &count, nullptr) == SPV_REFLECT_RESULT_SUCCESS) { - Vector bindings(count); - if (spvReflectEnumerateDescriptorBindings(&module, &count, bindings.data()) == SPV_REFLECT_RESULT_SUCCESS) { - auto name = [](const SpvReflectDescriptorBinding* binding) { - const char* raw = binding->name; - if (binding->type_description && binding->type_description->type_name) raw = binding->type_description->type_name; - String result = raw ? raw : ""; - const auto suffix = result.find("[0]"); - return suffix == String::npos ? result : result.substr(0, suffix); - }; - std::sort(bindings.begin(), bindings.end(), [&](const auto* a, const auto* b) { - const String an = a ? name(a) : String(), bn = b ? name(b) : String(); - return an != bn ? an < bn : (a && b && a->binding < b->binding); - }); - for (const auto* binding : bindings) { - if (!binding || binding->descriptor_type != SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_BUFFER) continue; - const String key = name(binding); - if (key.empty() || std::any_of(m_storageBlocks.begin(), m_storageBlocks.end(), - [&](const StorageBlock& block) { return block.Name == key; })) continue; - m_storageBlocks.push_back({key, binding->binding}); - } - } - } - spvReflectDestroyShaderModule(&module); - } - } const Program* m_frontend = nullptr; MG_Pipe::MGPipeHandle m_handle = MG_Pipe::kMGPipeNullHandle; const MG_Pipe::MGPipeShaderCsoRecord* m_record = nullptr; - mutable Bool m_storageBlocksReady = false; - mutable Vector m_storageBlocks; + // P7 OQ-8: the two mutables that memoised the per-draw reflect are gone with it. They + // were `mutable` precisely because this class is a borrowed const view - and that is + // also why they never helped: a view lives for one draw, so the latch was cold every + // time it was read. }; #else using MagmaProgramSource = MG_State::GLState::ProgramObject; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifacts.h b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifacts.h index 6aa681e0..4126f3ff 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifacts.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifacts.h @@ -90,6 +90,47 @@ namespace MobileGL::MG_State::GLState { using PipeInputReflection = ResourceReflection; using PipeOutputReflection = ResourceReflection; +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 wave 2 package C, OQ-8 (CONTRACT-P7 §5.3): THE SHADER STORAGE BLOCKS AS DIRECTVULKAN + // INDEXES THEM, published once at link time instead of re-derived per draw. + // + // WHY A SECOND VIEW OF SOMETHING blockReflection ALREADY HOLDS. `blockReflection` is + // glslang's list, in TProgram index order, carrying uniform blocks, storage blocks and the + // synthesized atomic-counter blocks together, one entry per ARRAY ELEMENT. DirectVulkan's + // resource queries speak a different index space entirely - the one its SPIRV-Reflect walk + // produces: storage descriptors only, normalised block name with the array subscript + // stripped, deduplicated across stages. `GetShaderStorageBlockIndex(name)` hands that index + // out and `GetShaderStorageBlockBinding(index)` reads it back, so the two have to agree + // with each other and with the names ProgramFactory::ReflectLayout looks up (which come + // from its OWN reflect pass over the same modules). This vector IS that space, built once. + // + // WHAT IT COSTS AND WHAT IT BUYS. MagmaProgramSource is a BORROWED VIEW constructed afresh + // for every wire draw (WireDraw.inc's WireProgramSource), so its lazy + // `EnsureStorageBlocks` re-ran spvReflectCreateShaderModule over every stage module of + // every program on every draw that touched an SSBO. Reading it out of the archive instead + // takes the SPIR-V reflector off the wire draw path entirely. + // + // `binding` IS THE DECLARED (or IO-mapper-assigned) BINDING AND NOT THE CURRENT ONE. + // glShaderStorageBlockBinding travels separately - as the record's StorageOverrides on the + // wire and as ProgramObject::GetShaderStorageBlockBindingOverride in the monolith - and + // both consumers apply it on read, exactly as they did when this list was rebuilt per + // draw. Baking the current binding in would freeze a rebind at link time. + // + // DISAGGREGATED-ONLY, and that guard is load-bearing for G1: LinkArtifacts is a pull-build + // type too, and one more Vector member would change its size, its implicit destructor and + // its move constructor in a build whose .text must stay byte-identical. + struct StorageBlockReflection { + // The NORMALISED name: the SPIR-V block type name with a trailing array subscript + // stripped, which is what SpvReflectDescriptorBinding's type_name answers and what + // ProgramFactory::ReflectLayout looks blocks up by. + String name; + Uint32 binding = 0; + // GL_BUFFER_DATA_SIZE. Carried because the monolith resource-query consumer reports + // it; the wire consumer does not read it today. + Int32 dataSize = 0; + }; +#endif + // Transform feedback (GL 3.0 core: glTransformFeedbackVaryings applies on // the NEXT link; the linked snapshot below is what draws and queries see). struct XfbVarying { @@ -167,6 +208,12 @@ namespace MobileGL::MG_State::GLState { // The owned reflection snapshot. Indexed by TProgram index; see the structs above. Vector uniformReflection; Vector blockReflection; +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8: DirectVulkan's storage-block index space, filled beside blockReflection in + // SnapshotGlslangReflection. See StorageBlockReflection above for what the order is + // and why it is not blockReflection's. + Vector storageBlocks; +#endif Vector pipeInputReflection; Vector pipeOutputReflection; // Program-level scalars glslang answers off the linked intermediates. @@ -479,14 +526,32 @@ namespace MobileGL::MG_State::GLState { v("blockMemberElement", a.blockMemberElement); } // 11 fields +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8. Three plain fields, so the generic codec arms carry it with no new arm of their + // own - which is the whole reason it is a table rather than hand-written bytes. + template + requires std::same_as, StorageBlockReflection> + void VisitFields(Self& a, V&& v) { + v("name", a.name); + v("binding", a.binding); + v("dataSize", a.dataSize); + } // 3 fields +#endif + // Every member EXCEPT `program`: it is null for every archived instance by construction // (ProgramTranslationCache.h asserts that at insert) and must never be serialized - it is - // the live glslang TProgram that only DoReflection may touch. 57 of the 58 members. + // the live glslang TProgram that only DoReflection may touch. 57 of the 58 members in a + // pull/monolith build, 58 of 59 in a disaggregated one (P7 OQ-8's storageBlocks). template requires std::same_as, LinkArtifacts> void VisitFields(Self& a, V&& v) { v("uniformReflection", a.uniformReflection); v("blockReflection", a.blockReflection); +#if MOBILEGL_BUILD_DISAGGREGATED + // Beside blockReflection because that is where it is FILLED, and a visitor that walks + // the table in declaration order then reads the two together. + v("storageBlocks", a.storageBlocks); +#endif v("pipeInputReflection", a.pipeInputReflection); v("pipeOutputReflection", a.pipeOutputReflection); v("lastStageIsFragment", a.lastStageIsFragment); @@ -569,7 +634,13 @@ namespace MobileGL::MG_State::GLState { #if defined(__GLIBCXX__) && !defined(_GLIBCXX_DEBUG) && (SIZE_MAX == UINT64_MAX) #define MGL_RESOURCEREFLECTION_SIZE 128 #define MGL_XFBVARYING_SIZE 128 +#if MOBILEGL_BUILD_DISAGGREGATED +// P7 OQ-8 added storageBlocks, one more Vector, in the disaggregated build ONLY. The pull +// number below is the one G1 measures and it has not moved. +#define MGL_LINKARTIFACTS_SIZE 1080 +#else #define MGL_LINKARTIFACTS_SIZE 1056 +#endif #define MGL_SPIRVARTIFACTS_SIZE 88 #elif defined(_LIBCPP_VERSION) && (SIZE_MAX == UINT64_MAX) && defined(MGL_ARTIFACT_SIZES_LIBCXX_PINNED) // The integrator pins these from the NDK build (brief C.4); until then this branch is inert. diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h index 3391cc45..5fd7fe95 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.h @@ -61,7 +61,14 @@ namespace MobileGL::MG_State::GLState { // Bumped whenever the byte format changes in a way a previous reader would misread. A // reader that sees a different word REFUSES; it never tries to guess a layout. #if MOBILEGL_BUILD_DISAGGREGATED - inline constexpr Uint32 kProgramArtifactsCodecVersion = 2; + // 3 since P7 wave 2 package C (OQ-8): LinkArtifacts gained `storageBlocks`, so a v2 reader + // would run out of bytes in the middle of the stream rather than notice. The schema + // fingerprint beside the version would catch it on its own - it is derived from the + // VisitFields tables and therefore moved with the new row - but the version is what a + // reader checks FIRST and what the refusal names, and CONTRACT-P7 §5.3 asks for the bump + // explicitly. `wireFingerprint` moves with both, which is expected and is what makes a + // mixed-version pair refuse at the handshake instead of at the first program. + inline constexpr Uint32 kProgramArtifactsCodecVersion = 3; // Derived from the actual VisitFields order/names, container element schemas and scalar // representations. No native container size/offset is included. Also checked in Hello. Uint64 ProgramArtifactsSchemaFingerprint(); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 5522ccba..5afe23f3 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -106,6 +106,29 @@ namespace { return bracket == MobileGL::String::npos ? name : name.substr(0, bracket); } +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8. ShaderStage -> EShLanguage, so a reflection record's `stages` mask (which is an + // EShLanguageMask) can be tested against the stage of a snapshotted shader. Written here + // rather than lifted into a shared header because this is the only caller and + // ProgramArtifacts.h's include-closure probe forbids this file's neighbours from growing a + // dependency on ShaderStage.h. EShLangCount means "no glslang stage", which the caller + // skips rather than guesses at. + static EShLanguage ShaderStageToEShLanguage(MobileGL::ShaderStage stage) { + switch (stage) { + case MobileGL::ShaderStage::Vertex: return EShLangVertex; + case MobileGL::ShaderStage::TessControl: return EShLangTessControl; + case MobileGL::ShaderStage::TessEval: return EShLangTessEvaluation; + case MobileGL::ShaderStage::Geometry: return EShLangGeometry; + case MobileGL::ShaderStage::Fragment: return EShLangFragment; + case MobileGL::ShaderStage::Compute: return EShLangCompute; + case MobileGL::ShaderStage::ShaderStageCount: + case MobileGL::ShaderStage::Unknown: + break; + } + return EShLangCount; + } +#endif + // Element index of an arrayed interface-block instance: "GOKU[3]" -> 3, "GOKU" -> 0. // Reflection spells arrayed instances exactly this way (glslang expands the instance // array into one TObjectReflection per element), and the subscript it writes is a plain @@ -1735,9 +1758,97 @@ namespace MobileGL::MG_State::GLState { } SnapshotGlslangReflection(); +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8. Guarded at the CALL as well as at the definition, which is not belt and + // braces: LinkArtifacts::storageBlocks does not exist in a pull build, so neither can + // the function that fills it, and G1 requires this translation unit to be statement + // for statement what it was there. + SnapshotStorageBlockIndexSpace(); +#endif return true; } +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 wave 2 package C, OQ-8 (CONTRACT-P7 §5.3): DIRECTVULKAN'S STORAGE-BLOCK INDEX SPACE, + // BUILT ONCE HERE INSTEAD OF PER DRAW. + // + // WHAT ORDER THIS HAS TO PRODUCE, and it is not blockReflection's. The consumers speak the + // index space DirectVulkan's SPIRV-Reflect walk produces (DirectVulkan.cpp's + // GetProgramResourceCache, and the copy of it MagmaProgramSource used to carry): + // + // for each SPIR-V module, IN LINKED-STAGE ORDER + // take its STORAGE_BUFFER descriptors + // sorted by (normalised block name, binding) + // appending any name not already present + // + // Every clause is reproducible from what phase A already owns. `in.shaders` is the linked + // shader snapshot and is documented as ALREADY STAGE-SORTED, and it is the same list, in + // the same order, that ProgramSpirvTask turns into `generatedSpirv` one module per entry - + // so iterating it iterates the modules. `blockReflection[].stages` is an EShLanguageMask, + // which is what says whether a given block is one of a given module's descriptors. The + // name normalisation is StripArrayElementSuffix, which is the same "[k]" strip + // NormalizeDescriptorName does to the SPIR-V type name: glslang expands an INSTANCE ARRAY + // into one reflection entry per element ("Foo[0]", "Foo[1]") while SPIR-V carries one + // descriptor with a count, so stripping is what makes the two agree on ONE entry named + // "Foo". + // + // ATOMIC-COUNTER BLOCKS ARE INCLUDED, deliberately, and they are the case that makes the + // filter `type.isBuffer` rather than IsStorageBlock(). The transpiler lowers every + // atomic_uint onto a synthesized gl_AtomicCounterBlock_ BUFFER block, so + // SPIRV-Reflect sees an ordinary STORAGE_BUFFER descriptor for it and DirectVulkan's index + // space has always contained it. GL's own GL_SHADER_STORAGE_BLOCK enumeration excludes + // them (ProgramInterface.cpp's ClassifyBlock does the exclusion there, in the space that + // needs it) - these are two different index spaces and this is the backend's. + // + // `binding` IS THE DECLARED ONE. A rebind travels separately and both consumers apply it + // on read; see StorageBlockReflection in ProgramArtifacts.h. + void ProgramLinkTask::SnapshotStorageBlockIndexSpace() { + artifacts.storageBlocks.clear(); + const auto alreadyPresent = [this](const String& name) { + for (const auto& block : artifacts.storageBlocks) { + if (block.name == name) return true; + } + return false; + }; + for (const LinkShaderInput& shader : in.shaders) { + const EShLanguage stage = ShaderStageToEShLanguage(shader.stage); + if (stage == EShLangCount) continue; + const Uint32 stageBit = 1u << static_cast(stage); + + // Pointers, not copies: blockReflection is stable for the rest of this function and + // a BlockReflection carries several strings. + Vector ofThisStage; + for (const ProgramObject::BlockReflection& block : artifacts.blockReflection) { + if (!block.type.isBuffer) continue; + if ((block.stages & stageBit) == 0) continue; + ofThisStage.push_back(&block); + } + // STABLE, so two entries that normalise to one name keep their declaration order + // and the FIRST (element 0, the base binding) is the one that survives the dedupe - + // which is the element SPIRV-Reflect's single descriptor reports. + std::stable_sort(ofThisStage.begin(), ofThisStage.end(), + [](const ProgramObject::BlockReflection* lhs, + const ProgramObject::BlockReflection* rhs) { + const String lhsName = StripArrayElementSuffix(lhs->name); + const String rhsName = StripArrayElementSuffix(rhs->name); + if (lhsName != rhsName) return lhsName < rhsName; + return lhs->binding < rhs->binding; + }); + for (const ProgramObject::BlockReflection* block : ofThisStage) { + const String name = StripArrayElementSuffix(block->name); + if (name.empty() || alreadyPresent(name)) continue; + ProgramObject::StorageBlockReflection record; + record.name = name; + record.binding = block->binding < 0 ? 0u : static_cast(block->binding); + record.dataSize = static_cast(block->size); + artifacts.storageBlocks.push_back(Move(record)); + } + } + MGLOG_D("ProgramObject %u: Reflection - %zu storage block(s) in the backend index space", + in.externalIndex, artifacts.storageBlocks.size()); + } +#endif + // The last thing DoReflection does, and the thing that lets everything after it stop // caring that a glslang::TProgram ever existed: copy every reflection record the GL query // surface reads into LinkArtifacts' own owned tables. diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h index 245f2665..07dc26e3 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h @@ -207,6 +207,14 @@ namespace MobileGL::MG_State::GLState { // one and overwritten the qualifier. See the definition for why the invented binding is // deliberately left in place for the backends' own use. void SeedDefaultStorageBlockBindings(); +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 wave 2 package C, OQ-8: builds LinkArtifacts::storageBlocks - DirectVulkan's own + // storage-block index space - out of the reflection snapshot the call above produced, + // so the backends stop re-deriving it with SPIRV-Reflect on every draw. Runs after + // SnapshotGlslangReflection because it reads blockReflection. See the definition for + // which order it has to reproduce and why each clause of it is derivable here. + void SnapshotStorageBlockIndexSpace(); +#endif Bool ValidateFragmentOutputLocations(); Bool ResolveTransformFeedbackVaryings(); void ResolveGsTriangleStripCapture(const glslang::TIntermediate* captureIntermediate); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 7546e5aa..33a4de73 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -48,6 +48,10 @@ namespace MobileGL::MG_State::GLState { using BlockReflection = ResourceReflection; using PipeInputReflection = ResourceReflection; using PipeOutputReflection = ResourceReflection; +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8: DirectVulkan's storage-block index space, carried in LinkArtifacts. + using StorageBlockReflection = MobileGL::MG_State::GLState::StorageBlockReflection; +#endif using XfbVarying = MobileGL::MG_State::GLState::XfbVarying; using LinkArtifacts = MobileGL::MG_State::GLState::LinkArtifacts; using SpirvArtifacts = MobileGL::MG_State::GLState::SpirvArtifacts; diff --git a/MobileGL/MG_Test/Program/ProgramArtifactsCodecTest.cpp b/MobileGL/MG_Test/Program/ProgramArtifactsCodecTest.cpp index a5f71488..3290e206 100644 --- a/MobileGL/MG_Test/Program/ProgramArtifactsCodecTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramArtifactsCodecTest.cpp @@ -362,7 +362,15 @@ TEST(ProgramArtifactsCodec, AVersionMismatchIsRefused) { // no archive owns. A codec arm for it would be a use-after-free waiting for a cache hit. TEST(ProgramArtifactsCodec, TheTablesVisitEveryMemberExceptTheLiveProgram) { #if MOBILEGL_PIPE_PUSH +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8: 59 members, 58 visited. The 59th is still the live TProgram; the 58th is + // storageBlocks, which exists only in this build (ProgramArtifacts.h guards it so the pull + // build's LinkArtifacts keeps its size, its destructor and its .text). + EXPECT_EQ(ProgramArtifactsVisitedFieldCount(), 58u); + EXPECT_EQ(ProgramArtifactsVisitedFieldCount(), 3u); +#else EXPECT_EQ(ProgramArtifactsVisitedFieldCount(), 57u); +#endif EXPECT_EQ(ProgramArtifactsVisitedFieldCount(), 8u); EXPECT_EQ(ProgramArtifactsVisitedFieldCount(), 14u); EXPECT_EQ(ProgramArtifactsVisitedFieldCount(), 11u); @@ -381,7 +389,12 @@ TEST(ProgramArtifactsCodec, PortableHeaderUsesWireSchemaRatherThanNativeContaine Uint64 schema = 0; std::memcpy(&version, bytes.data(), sizeof(version)); std::memcpy(&schema, bytes.data() + sizeof(version), sizeof(schema)); - EXPECT_EQ(version, 2u); + // 3 since P7 OQ-8 (CONTRACT-P7 §5.3): LinkArtifacts gained storageBlocks, so a v2 reader + // would run out of bytes mid-stream rather than notice. The schema word beside it moved + // too - it is derived from the VisitFields tables - and so did `wireFingerprint`, which is + // what makes a mixed-version pair refuse at the handshake instead of at the first program. + EXPECT_EQ(version, kProgramArtifactsCodecVersion); + EXPECT_EQ(version, 3u); EXPECT_EQ(schema, ProgramArtifactsSchemaFingerprint()); EXPECT_NE(schema, 0u); EXPECT_NE(schema, sizeof(LinkArtifacts)); diff --git a/MobileGL/MG_Test/Program/ProgramArtifactsTest.cpp b/MobileGL/MG_Test/Program/ProgramArtifactsTest.cpp index e1f9239b..9cf7555b 100644 --- a/MobileGL/MG_Test/Program/ProgramArtifactsTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramArtifactsTest.cpp @@ -81,7 +81,15 @@ namespace { EXPECT_EQ(CountFields(), 20u); EXPECT_EQ(CountFields(), 14u); EXPECT_EQ(CountFields(), 11u); +#if MOBILEGL_BUILD_DISAGGREGATED + // P7 OQ-8 added LinkArtifacts::storageBlocks and its own three-field table, in the + // disaggregated build only. The pull/monolith number below is the one G1 measures and + // it has not moved - which is the whole reason the member is guarded. + EXPECT_EQ(CountFields(), 3u); + EXPECT_EQ(CountFields(), 58u); +#else EXPECT_EQ(CountFields(), 57u); +#endif EXPECT_EQ(CountFields(), 8u); } diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index 5d838b82..9ea6e2da 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -380,6 +380,177 @@ void main() { EXPECT_EQ(GetError(), GL_NO_ERROR); } +#if MOBILEGL_BUILD_DISAGGREGATED +// P7 wave 2 package C, OQ-8 (CONTRACT-P7 §5.3): THE ARCHIVE'S STORAGE-BLOCK INDEX SPACE IS +// SPIRV-REFLECT'S, ELEMENT FOR ELEMENT. +// +// This is the equivalence the whole slice rests on. DirectVulkan's resource queries speak an +// index space that used to be DERIVED, by running SPIRV-Reflect over the program's modules - +// per draw on the wire path (MagmaProgramSource) and behind a mutable cache in the monolith +// one (DirectVulkan.cpp's GetProgramResourceCache). It is now PUBLISHED at link time out of +// glslang's reflection instead (ProgramLinkTask::SnapshotStorageBlockIndexSpace). Two +// different producers, one index space: if they ever disagree, +// ProgramFactory::ReflectLayout - which looks blocks up by the name ITS OWN reflect pass +// produced - starts binding storage buffers at the wrong descriptor, silently. +// +// THE REFERENCE IS COMPUTED HERE, not copied as a constant, and it is computed with the same +// algorithm the old consumer used: per module in linked-stage order, STORAGE_BUFFER +// descriptors only, sorted by (normalised name, binding), deduplicated across modules by name. +// Comparing against a hand-written expected list would only pin what the author believed. +// +// THE PROGRAM IS THE ONE THE CONTRACT ASKS FOR: multi-stage (vertex + fragment), with an +// ARRAYED SSBO and an ATOMIC-COUNTER block. Both are the cases where the two producers could +// legitimately have disagreed and neither is exotic: +// * an instance array reflects in glslang as one entry PER ELEMENT ("Blocks[0]", +// "Blocks[1]") and in SPIR-V as ONE descriptor with a count, so the archive has to strip +// the subscript and keep the first element - which is the one whose binding the single +// descriptor reports; +// * an atomic_uint is lowered by the transpiler onto a synthesized gl_AtomicCounterBlock_ +// BUFFER block, so SPIRV-Reflect sees an ordinary storage descriptor for it and the +// backend's space has always contained it - even though GL's own GL_SHADER_STORAGE_BLOCK +// enumeration excludes it. The archive must follow the BACKEND's space, not GL's. +TEST_F(ProgramTest, TheArchivesStorageBlockOrderIsTheOneSpirvReflectProduces) { + char infoLog[1024] = ""; + const char* vsSrc = R"(#version 460 core +layout(std430, binding = 3) buffer Blocks { + uint value; +} blocks[2]; +layout(binding = 0, offset = 0) uniform atomic_uint counter; +layout(std430, binding = 6) buffer VertexOnly { + uint value; +} vertexOnly; +void main() { + atomicCounterIncrement(counter); + blocks[0].value = 1u; + vertexOnly.value = 2u; + gl_Position = vec4(0.0, 0.0, 0.0, 1.0); +} +)"; + const char* fsSrc = R"(#version 460 core +layout(std430, binding = 5) buffer FragmentOnly { + uint value; +} fragmentOnly; +layout(std430, binding = 3) buffer Blocks { + uint value; +} blocks[2]; +layout(location = 0) out vec4 color; +void main() { + fragmentOnly.value = 3u; + color = vec4(float(blocks[1].value), 0.0, 0.0, 1.0); +} +)"; + + const GLuint vs = CreateShader(GL_VERTEX_SHADER); + ShaderSource(vs, 1, &vsSrc, nullptr); + CompileShader(vs); + GLint status = GL_FALSE; + GetShaderiv(vs, GL_COMPILE_STATUS, &status); + GetShaderInfoLog(vs, sizeof(infoLog), nullptr, infoLog); + ASSERT_EQ(status, GL_TRUE) << infoLog; + + const GLuint fs = CreateShader(GL_FRAGMENT_SHADER); + ShaderSource(fs, 1, &fsSrc, nullptr); + CompileShader(fs); + GetShaderiv(fs, GL_COMPILE_STATUS, &status); + GetShaderInfoLog(fs, sizeof(infoLog), nullptr, infoLog); + ASSERT_EQ(status, GL_TRUE) << infoLog; + + const GLuint program = CreateProgram(); + AttachShader(program, vs); + AttachShader(program, fs); + LinkProgram(program); + GetProgramiv(program, GL_LINK_STATUS, &status); + GetProgramInfoLog(program, sizeof(infoLog), nullptr, infoLog); + ASSERT_EQ(status, GL_TRUE) << infoLog; + + auto programObject = MobileGL::MG_State::pGLContext->GetProgramObject(program); + ASSERT_NE(programObject, nullptr); + + // ---- the reference: the old consumer's algorithm, run here over the real modules ---- + const auto normalise = [](const SpvReflectDescriptorBinding& binding) { + const char* raw = binding.name; + if (binding.type_description != nullptr && binding.type_description->type_name != nullptr) { + raw = binding.type_description->type_name; + } + String name = raw ? raw : ""; + const auto suffix = name.find("[0]"); + return suffix == String::npos ? name : name.substr(0, suffix); + }; + std::vector reference; + const auto& spirvs = programObject->GetSpirvReflection().generatedSpirv; + ASSERT_GE(spirvs.size(), 2u) << "the case needs a multi-stage program to be a real test of " + "the cross-stage dedupe"; + for (const auto& spirv : spirvs) { + if (spirv.empty()) continue; + SpvReflectShaderModule module{}; + ASSERT_EQ(spvReflectCreateShaderModule(spirv.size() * sizeof(Uint), spirv.data(), &module), + SPV_REFLECT_RESULT_SUCCESS); + uint32_t count = 0; + ASSERT_EQ(spvReflectEnumerateDescriptorBindings(&module, &count, nullptr), + SPV_REFLECT_RESULT_SUCCESS); + std::vector bindings(count); + if (count != 0) { + ASSERT_EQ(spvReflectEnumerateDescriptorBindings(&module, &count, bindings.data()), + SPV_REFLECT_RESULT_SUCCESS); + } + std::sort(bindings.begin(), bindings.end(), + [&](const SpvReflectDescriptorBinding* lhs, const SpvReflectDescriptorBinding* rhs) { + const String lhsName = lhs ? normalise(*lhs) : String(); + const String rhsName = rhs ? normalise(*rhs) : String(); + if (lhsName != rhsName) return lhsName < rhsName; + return lhs->binding < rhs->binding; + }); + for (const SpvReflectDescriptorBinding* binding : bindings) { + if (binding == nullptr || + binding->descriptor_type != SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_BUFFER) { + continue; + } + const String name = normalise(*binding); + if (name.empty()) continue; + if (std::find(reference.begin(), reference.end(), name) != reference.end()) continue; + reference.push_back(name); + } + spvReflectDestroyShaderModule(&module); + } + + // ---- what the archive published ---- + std::vector archived; + for (const auto& block : programObject->GetLinkReflection().storageBlocks) { + archived.push_back(block.name); + } + + ASSERT_FALSE(reference.empty()) + << "the reference walk found no storage descriptors at all, so the comparison below " + "would be vacuous - the shaders or the SPIR-V generation changed under this case"; + EXPECT_EQ(archived, reference) + << "LinkArtifacts::storageBlocks is not the index space SPIRV-Reflect produces. " + "GetShaderStorageBlockIndex/Binding hand these indices to ProgramFactory::ReflectLayout, " + "which resolves blocks by the name its own reflect pass produced, so a divergence " + "here binds storage buffers at the wrong descriptor rather than failing loudly."; + + // The arrayed block must appear ONCE, under its stripped name: glslang reflects it per + // element and SPIR-V carries one descriptor, and collapsing the two is the whole reason + // the archive normalises. + EXPECT_EQ(std::count(archived.begin(), archived.end(), String("Blocks")), 1) + << "the arrayed SSBO must collapse to one entry named without its subscript"; + // The atomic counter is in the BACKEND's space even though GL's own enumeration excludes it. + EXPECT_NE(std::find_if(archived.begin(), archived.end(), + [](const String& name) { + return name.rfind("gl_AtomicCounterBlock", 0) == 0; + }), + archived.end()) + << "the synthesized atomic-counter block is a STORAGE_BUFFER descriptor in SPIR-V, so " + "DirectVulkan's index space contains it and the archive has to as well"; + + // And the indices actually resolve, which is what the consumers do with them. + for (Uint i = 0; i < archived.size(); ++i) { + EXPECT_EQ(MG_Backend::DirectVulkan::GetShaderStorageBlockIndex(*programObject, archived[i]), i) + << "name " << archived[i] << " did not resolve to its own index"; + } + EXPECT_EQ(GetError(), GL_NO_ERROR); +} +#endif // MOBILEGL_BUILD_DISAGGREGATED + TEST_F(ProgramTest, DirectVulkanStorageBlockUsesShaderLayoutBinding) { char infoLog[1024] = ""; const char* csSrc = R"(#version 460 core diff --git a/docs/Disaggregated/notes/p7/magma-c.md b/docs/Disaggregated/notes/p7/magma-c.md index 0c818614..9b581d8b 100644 --- a/docs/Disaggregated/notes/p7/magma-c.md +++ b/docs/Disaggregated/notes/p7/magma-c.md @@ -232,3 +232,124 @@ divergence this slice closes. Restored: 10/10. **Lane arithmetic.** The three new `Split` entries carry `integration-magma-split`, so that gating lane grows 73 → 75 (and `-spawn` 52 → 54, `-tcp` 54 → 56). Growth only; G14's name sets are grow-only by rule. + +--- + +## Slice 3 — OQ-8: one reflection archive serves the storage-block consumers (§5.3) + +**What landed.** + +- `LinkArtifacts` gains `Vector storageBlocks` (`{name, binding, + dataSize}`), **`#if MOBILEGL_BUILD_DISAGGREGATED` only** — see G1 below. +- `ProgramLinkTask::SnapshotStorageBlockIndexSpace()` fills it in phase A, right after + `SnapshotGlslangReflection`. +- Its `VisitFields` row and its own three-field table join the codec; + `kProgramArtifactsCodecVersion` 2 → 3; `ProgramArtifactsSchemaFingerprint()` and therefore + `wireFingerprint` move with them, which is expected and is what makes a mixed-version pair + refuse at the handshake rather than at the first program. +- `MagmaProgramSource::EnsureStorageBlocks` is **deleted**, with `m_storageBlocks`, + `m_storageBlocksReady`, the local `StorageBlock` struct and `#include `. +- `DirectVulkan.cpp`'s monolith consumer reads the archive too, under `#if` symmetry. + +**Why phase A and not phase B.** The obvious home is where the SPIR-V exists, and there is +exactly one function that sees both finished halves — `ProgramSpirvTask::RunBody`'s L1 cache +insert — but its writes reach only the TRANSLATION CACHE. Phase A's `artifacts` has already +been moved into the `ProgramObject` by then (`ProgramObject.cpp`'s `JoinPendingLink`), and +phase B is structurally forbidden from touching it. A field filled there would therefore be +present on a cache HIT and absent on every cold link — the worst of both, and invisible until +a second identical program linked. So the fill is glslang-derived, in phase A, and the +equivalence below is what makes that safe. + +**The equivalence is measured, not asserted.** The consumers speak the index space +SPIRV-Reflect produces, and the archive now produces it from glslang's reflection instead. +`ProgramTest.TheArchivesStorageBlockOrderIsTheOneSpirvReflectProduces` runs the OLD algorithm — +SPIRV-Reflect over the real modules, sorted by (normalised name, binding), deduplicated across +stages — over a multi-stage program with an arrayed SSBO and an atomic-counter block, and +compares element for element. Measured reference on this tree: + +``` +{ "Blocks", "VertexOnly", "gl_AtomicCounterBlock_0", "FragmentOnly" } +``` + +which exercises all three of the cases that could have diverged: the instance array collapses +to one entry (glslang reflects per element, SPIR-V carries one descriptor with a count), the +synthesized atomic-counter block IS in the backend's space although GL's own +`GL_SHADER_STORAGE_BLOCK` enumeration excludes it, and `FragmentOnly` lands last because the +cross-stage dedupe walks stages in linked order. + +**MEASUREMENT — `spvReflectCreateShaderModule` on the Magma wire draw path, per frame.** +Same binary, same workload, same window (`frames=1 window=1 draws=1 acc=18`, identical bytes), +`DirectVulkan` × inproc, `StorageBufferRegrowScenario` + the pipeline storage-block case, with +a temporary `MGLOG_I("OQ8_REFLECT_CALL")` at the call site: + +| | reflect calls | frames | draws | +|---|---|---|---| +| BEFORE (`HEAD`'s per-draw `EnsureStorageBlocks`) | **4** | 1 | 1 | +| AFTER (reads the archive) | **0** | 1 | 1 | + +The AFTER number is 0 structurally, not incidentally: the call site is gone and +`MagmaProgramSource.h` no longer includes `spirv_reflect.h` at all, which is the gate a future +edit would have to break on purpose. (A `MagmaProgramSource` is a borrowed view constructed +once per wire draw, so its `mutable` memo was cold every time it was read — that is why the +per-draw cost existed at all.) + +**The monolith half: DONE, under `#if` symmetry — with one caveat the integrator should see.** +The task allowed deferring this if it could not be done without moving pull `.text`. It can, +and it is: `GetShaderStorageBlockIndex` / `GetShaderStorageBlockBinding` / +`ClearProgramResourceCaches` and the block-binding setter each have a disaggregated arm that +reads the archive and a `#else` arm that is the previous code verbatim. In the disaggregated +build that retires `g_programResourceCaches`, `ProgramResourceCache`, `StorageBlockResource`, +`BufferVariableResource`, `AddBufferVariablesRecursive`, `NormalizeDescriptorName`, +`GetProgramResourceCache` **and the rehash hazard** — the open-addressed-map dangling reference +that was a reproducible segfault in `ProgramPipelineScenario`'s two storage-block cases. + +Two things are worth stating plainly: + +1. **The pull build keeps all of it.** The archive member is disaggregated-only because one + more `Vector` in `LinkArtifacts` changes its size, its implicit destructor and its move + constructor, and G1 pins the pull build's `.text` byte for byte. So §5.3's "`g_programResource + Caches` 随之删" is achieved in the disaggregated build and NOT in the pull one. Retiring the + pull half is a one-line change (drop the guard on the member) the day that pin is retired or + the member is moved — a P13 / integrator call, not a wave-2 one. +2. **Nothing outside `DirectVulkan.cpp` read any of the retired machinery.** Measured: + `BufferVariableResource`, `bufferVariables`, `activeVariables` and `StorageBlockResource` + have no reference anywhere else in the tree, and `dataSize` was written and never read — + `GL_BUFFER_VARIABLE` and `GL_ACTIVE_VARIABLES` are answered by + `MG_Impl/GLImpl/Program/ProgramInterface.cpp` out of glslang's own reflection, which is a + different index space. So the two arms differ in exactly the two functions their callers + use and in nothing else. + +One genuine behaviour improvement rides along: the monolith `GetShaderStorageBlockBinding` now +asks `GetShaderStorageBlockBindingOverride` on every read, over an immutable list, where it +used to read a binding patched into a mutable cache. Same answer by a shorter route, and a +rebind can no longer be lost to a cache rebuild that raced a state-version bump. + +**Proof (green).** `ctest -L unit` 2403/2403 (2402 before this slice; +1 is the new case). +`DirectVulkan.*(ProgramPipeline|StorageBufferRegrow)` 60/60 — the **monolith** `DirectVulkan.` +arm and the `Split` / `Spawn` / `Tcp` arms together, which is what says both consumers were +switched. + +**G1 after the archive change:** `.text` `0xa52203` = 10822147 → 10822147 (+0), 27837 → 27837 +defined symbols, **0 added / 0 removed / 0 resized / 0 renamed**, and the file is byte-identical +on disk (19143824 bytes both sides). The `#if` guards did their job; an unguarded call site +caught it once mid-slice (the pull build failed to compile, which is the loud version). + +**RED-ONCE (executed).** `SnapshotStorageBlockIndexSpace` made to publish nothing: + +``` +ProgramTest.cpp:525: Failure + Which is: {} + Which is: { "Blocks", "VertexOnly", "gl_AtomicCounterBlock_0", "FragmentOnly" } +... +58% tests passed, 25 tests failed out of 60 + 3091 DirectVulkan.ProgramPipelineScenario.ComputeAndGraphicsStagesShareOnePipeline (SEGFAULT) + 3092 DirectVulkan.ProgramPipelineScenario.AStageProgramsStorageBlockBindingReaches... (Failed) + 3282 DirectVulkan.StorageBufferRegrowScenario... (Failed) + 4179 DirectVulkan.Split.Buffers.StorageBufferRegrowScenario... (Failed) + 4198 DirectVulkan.Spawn.Buffers.StorageBufferRegrowScenario... (Failed) + 4217 DirectVulkan.Tcp.Buffers.StorageBufferRegrowScenario... (Failed) + 4469 DirectVulkan.Split.ProgramPipelineScenario... (Failed) +``` + +The monolith arm reds beside the three transports, which is the evidence that the monolith +consumer really was switched and not merely edited. Restored: 61/61.